From 678ddf8a4e5193dd78230368f7fd0f833d1e4587 Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Wed, 28 Oct 2026 13:53:13 -0700 Subject: [PATCH 01/14] Island user program modules end to end A file the island decision put in the set is no longer compiled natively: it is keyed by its absolute path, its declarations type the program as foreign, its diagnostics do not fail the program the build does not compile, and every import or require of it lowers to an island load. `--node-island=` therefore takes effect for a user module, not only for a package. The recovery stays one round: when the lowering refuses a user JavaScript module, the reload adds every such module at once and each one is reported once as an SC8001 note naming the module and its reason. The build reports the islands it ran (packages and modules) with its result, and an executable cache hit reports them again, so a runner can tell an island result from a native one. Co-Authored-By: Claude Opus 5.5 --- packages/compiler/src/compile-types.ts | 4 + .../compiler/src/diagnostics/diagnostic.ts | 16 ++ .../src/executable/executable-cache.ts | 16 ++ packages/compiler/src/executable/prepare.ts | 157 ++++++++++++++++-- .../src/frontend/lowering/lower-modules.ts | 4 + .../frontend/lowering/lower-node-island.ts | 52 +++++- .../frontend/lowering/lower-shared-modules.ts | 4 + packages/compiler/src/frontend/pipeline.ts | 43 ++++- packages/compiler/src/index.ts | 38 ++++- .../compiler/src/node-islands/boundary.ts | 43 ++++- packages/compiler/src/node-islands/policy.ts | 5 +- 11 files changed, 346 insertions(+), 36 deletions(-) diff --git a/packages/compiler/src/compile-types.ts b/packages/compiler/src/compile-types.ts index 404648051..29a34660d 100644 --- a/packages/compiler/src/compile-types.ts +++ b/packages/compiler/src/compile-types.ts @@ -165,6 +165,10 @@ export type CompileFailure = { export interface CompileWarnings { warnings?: ScrDiagnostic[]; sourceTexts?: Map; + /** The Node islands the build ran: the npm packages and the program + * modules (absolute file paths). Reported again on an executable cache + * hit, so a runner can tell an island result from a native one. */ + islands?: { packages: string[]; modules: string[] }; } export type CompileSourceResult = diff --git a/packages/compiler/src/diagnostics/diagnostic.ts b/packages/compiler/src/diagnostics/diagnostic.ts index a5f4fdbe2..a1b2f071f 100644 --- a/packages/compiler/src/diagnostics/diagnostic.ts +++ b/packages/compiler/src/diagnostics/diagnostic.ts @@ -453,6 +453,22 @@ export function nodeIslandNoteDiag( }; } +/** SC8001 — a program module runs in a Node island (a note, once per island + * module in the build output; DESIGN.md note 1 §3.2). */ +export function nodeIslandModuleNoteDiag( + file: string, + klass: string, + reason: string, + loc: SrcLoc, +): ScrDiagnostic { + return { + code: "SC8001", + message: `module '${file}' runs in a Node island: ${klass}: ${reason}`, + loc, + hint: "sloppy-mode code Node runs; --node-islands=off compiles this module natively instead", + }; +} + /** SC8010 — a value crosses an island boundary in a way NI-4 cannot keep * exact yet (callbacks, native objects, remote promises). */ export function nodeIslandBoundaryDiag(what: string, loc: SrcLoc): ScrDiagnostic { diff --git a/packages/compiler/src/executable/executable-cache.ts b/packages/compiler/src/executable/executable-cache.ts index 604af05b9..491cc5fa2 100644 --- a/packages/compiler/src/executable/executable-cache.ts +++ b/packages/compiler/src/executable/executable-cache.ts @@ -147,6 +147,8 @@ interface EarlyExecutableCacheStamp { native: EarlyExecutableNativeFeatures; /** The build's performance warnings (SC61xx), which a cache hit repeats. */ performance?: ScrDiagnostic[]; + /** The Node islands the build ran, which a cache hit reports again. */ + islands?: { packages: string[]; modules: string[] }; /** Programs that call into Node.js: the libnode pack identity the build * recorded (EarlyExecutableCacheOptions.libnodeIdentity). */ libnode?: string; @@ -259,6 +261,8 @@ export interface EarlyExecutableCacheHit { frontend: FrontendInputSnapshot; /** The performance warnings the build reported (SC61xx), repeated on a hit. */ performance?: ScrDiagnostic[]; + /** The Node islands the build ran, repeated on a hit. */ + islands?: { packages: string[]; modules: string[] }; /** How a --profile-use build's profile matched, reported again on a hit. */ profileMatch?: NativeProfileMatch; } @@ -267,6 +271,16 @@ export interface EarlyExecutableCachePublish extends EarlyExecutableCacheHit { nativeDependencies?: NativeArtifactDependency[]; } +function isIslands(value: unknown): value is { packages: string[]; modules: string[] } { + if (typeof value !== "object" || value === null) return false; + const islands = value as Record; + return ( + Array.isArray(islands.packages) && + Array.isArray(islands.modules) && + [...islands.packages, ...islands.modules].every((item) => typeof item === "string") + ); +} + function isProfileMatch(value: unknown): value is NativeProfileMatch { if (typeof value !== "object" || value === null) return false; const match = value as Record; @@ -745,6 +759,7 @@ async function readExecutableCache( ...(Array.isArray(stamp.performance) && stamp.performance.length > 0 ? { performance: stamp.performance } : {}), + ...(isIslands(stamp.islands) ? { islands: stamp.islands } : {}), ...(isProfileMatch(stamp.profileMatch) ? { profileMatch: stamp.profileMatch } : {}), ...(ir === null ? {} : { irPath: paths.irPath }), ...(llvmSourcePath === undefined ? {} : { llvmSourcePath }), @@ -936,6 +951,7 @@ export async function publishEarlyExecutableCache( ...(result.performance === undefined || result.performance.length === 0 ? {} : { performance: result.performance }), + ...(result.islands === undefined ? {} : { islands: result.islands }), ...(result.native.nodeIsland === true ? { libnode: options.libnodeIdentity?.() ?? "" } : {}), ...(result.profileMatch === undefined ? {} : { profileMatch: result.profileMatch }), }; diff --git a/packages/compiler/src/executable/prepare.ts b/packages/compiler/src/executable/prepare.ts index a85e17f37..fc6484438 100644 --- a/packages/compiler/src/executable/prepare.ts +++ b/packages/compiler/src/executable/prepare.ts @@ -1,8 +1,10 @@ +import { realpathSync } from "node:fs"; import type { CompileFailure, CompileRequestOptions } from "../compile-types.js"; import type { FfiProfile } from "../ffi/ffi-manifest.js"; import { frontendDisposed } from "../frontend/dispose-hook.js"; import { checkerPanicDiag, + nodeIslandModuleNoteDiag, nodeIslandNoteDiag, nodeIslandRetryFailedDiag, npmStaticRefusalDiag, @@ -24,15 +26,20 @@ import type { FrontendFactory } from "../frontend/pipeline.js"; import type { UnitStore } from "../frontend/lowering/unit-reuse.js"; import type { CompilationTiming } from "../timing.js"; import { readProjectConfig } from "../project-config.js"; -import { nodeIslandPolicy } from "../node-islands/policy.js"; +import { + isMarkedUserModule, + nodeIslandPolicy, + type NodeIslandPolicy, +} from "../node-islands/policy.js"; import { buildNodeIslandReport } from "../node-islands/report.js"; -import type { IslandPackage, NodeIslandSet } from "../node-islands/boundary.js"; +import type { IslandModule, IslandPackage, NodeIslandSet } from "../node-islands/boundary.js"; import { classifyDiagnostic, isIslandReasonCode } from "../node-islands/classify.js"; import { packageDirOf, packageVersionOf } from "../node-islands/origin.js"; import { describe } from "../node-islands/summary.js"; import { resolveBareModule } from "../frontend/resolve.js"; import { isEsmFile } from "../frontend/lowering/lower-node-island.js"; -import type { PackageEdge } from "../node-islands/scan.js"; +import type { NodeIslandScan, PackageEdge } from "../node-islands/scan.js"; +import { isJsSourceFileName } from "../frontend/tsc-codes.js"; export interface PreparedExecutableModule { ok: true; @@ -42,6 +49,10 @@ export interface PreparedExecutableModule { /** Divergence warnings (SC6xxx): the build succeeds; these sites can * behave differently from Node. */ warnings: ScrDiagnostic[]; + /** The Node islands the build ran (the SC8001 notes name them with their + * reasons): packages by name, modules by file. A cached build repeats + * them. */ + islands?: { packages: string[]; modules: string[] }; } /** Release the parser/checker before native object generation begins. @@ -57,6 +68,7 @@ export function prepareExecutableModule( lowerIslands: ReadonlyMap = new Map(), retryFences = true, unitStore: UnitStore | null = null, + lowerModules: ReadonlyMap = new Map(), ): PreparedExecutableModule | CompileFailure { // The island decision reads the detector's scan unless islands are off. const islandScan = @@ -66,11 +78,14 @@ export function prepareExecutableModule( // Node islands (scope-B NI-4): packages refused natively run in Node, // unless the policy says otherwise. The frontend reloads without them, // so their declarations type the program. - const islandDecision = decideNodeIslands(entryPath, opts, fe, lowerIslands); + const islandDecision = decideNodeIslands(entryPath, opts, fe, lowerIslands, lowerModules); if (islandDecision !== null) { fe.dispose(); fe = createFrontend(entryPath, opts.npmStatic, undefined, undefined, { islandPackages: new Set(islandDecision.packages.keys()), + ...(islandDecision.modules.size > 0 + ? { islandModules: new Set(islandDecision.modules.keys()) } + : {}), islandScan, }); timing("frontend-island-reload"); @@ -121,6 +136,7 @@ export function prepareExecutableModule( grown, retryFences, unitStore, + lowerModules, ); } return fail(fe.preflight); @@ -159,7 +175,11 @@ export function prepareExecutableModule( lowerIslands, fe.nodeIslandScan?.({ userJs: false }).edges, ); - if (grown !== null) { + // Sloppy user modules the lowering refused join the same retry + // (DESIGN.md note 1 §3.2): one reload with every island at once, + // never one round per module. + const grownModules = loweringModules(entryPath, opts, lowered.diagnostics, lowerModules); + if (grown !== null || grownModules !== null) { fe.dispose(); frontendDisposed(); disposed = true; @@ -170,9 +190,10 @@ export function prepareExecutableModule( buildPlatform, createFrontend, timing, - grown, + grown ?? lowerIslands, retryFences, unitStore, + grownModules ?? lowerModules, ); } // A failed program whose compiled packages need the runtime or a @@ -198,6 +219,8 @@ export function prepareExecutableModule( timing, marked, false, + undefined, + lowerModules, ); } return fail(lowered.diagnostics); @@ -258,6 +281,14 @@ export function prepareExecutableModule( ...(lowered.divergences ?? []), ...(lowered.performance ?? []), ], + ...(islandDecision === null + ? {} + : { + islands: { + packages: [...islandDecision.packages.keys()].sort(), + modules: [...islandDecision.modules.keys()].sort(), + }, + }), }; // Packages compiled with runtime fences that need the JavaScript // runtime or a mutable object model (fenceIslands): build again with @@ -284,6 +315,7 @@ export function prepareExecutableModule( islands, false, unitStore, + lowerModules, ); if (islanded.ok) return islanded; firstError ??= islanded.diagnostics[0] ?? null; @@ -298,16 +330,63 @@ export function prepareExecutableModule( return prepared; } -/** The packages a build runs in Node islands, or null for none: under - * `--node-islands=auto` (the default) every npm package the native - * pipeline refused, plus packages marked `node`; under `explicit` only the - * marked ones; under `off`, none (refusals stay SC2013). Each island is - * reported once as an SC8001 note. */ +/** Whether two spellings name the same file (the checker resolves through + * symlinks; a CLI argument may not). */ +function sameFile(a: string, b: string): boolean { + if (samePath(a, b)) return true; + try { + const real = realpathSync(a); + const other = realpathSync(b); + return samePath(real, other); + } catch { + return false; + } +} + +function samePath(a: string, b: string): boolean { + return a.replaceAll("\\", "/") === b.replaceAll("\\", "/"); +} + +/** The program modules that run in Node islands, by file: modules marked + * with `--node-island=` or `scriptc.nodeIslands.modules`. The entry + * stays native: a program whose entry is an island runs the whole file in + * Node, which is a different feature. */ +function moduleIslands( + entryPath: string, + policy: NodeIslandPolicy, + scan: NodeIslandScan, +): Map { + const modules = new Map(); + for (const m of scan.modules) { + if (m.origin !== "user") continue; + if (!isJsSourceFileName(m.file)) continue; + if (sameFile(m.file, entryPath)) continue; + const marked = isMarkedUserModule(policy, m.file); + if (marked === null) continue; + modules.set(m.file, { + file: m.file, + class: "marked", + reason: + marked === "cli" + ? "marked with --node-island" + : 'marked "node" in scriptc.nodeIslands.modules', + decision: "marked", + }); + } + return modules; +} + +/** The packages and modules a build runs in Node islands, or null for none: + * under `--node-islands=auto` (the default) every npm package the native + * pipeline refused, plus the packages and modules marked `node`; under + * `explicit` only the marked ones; under `off`, none (refusals stay + * SC2013). Each island is reported once as an SC8001 note. */ function decideNodeIslands( entryPath: string, opts: CompileRequestOptions, fe: ReturnType, lowerIslands: ReadonlyMap, + lowerModules: ReadonlyMap, ): (NodeIslandSet & { notes: ScrDiagnostic[] }) | null { const policy = nodeIslandPolicy(readProjectConfig(entryPath), opts.nodeIslands); if (policy.mode === "off" || fe.nodeIslandScan === undefined) return null; @@ -321,10 +400,15 @@ function decideNodeIslands( : [], ); for (const name of lowerIslands.keys()) refused.add(name); - if (refused.size === 0 && policy.forcedNode.size === 0) return null; + const scan = fe.nodeIslandScan({ userJs: false }); + const modules = moduleIslands(entryPath, policy, scan); + // Modules the lowering refused keep their first reason (the reload no + // longer compiles them). + for (const [file, island] of lowerModules) if (!modules.has(file)) modules.set(file, island); + if (refused.size === 0 && policy.forcedNode.size === 0 && modules.size === 0) return null; const report = buildNodeIslandReport({ entry: entryPath, - scan: fe.nodeIslandScan({ userJs: false }), + scan, npm: fe.npmStatic, diagnostics: [], fences: [], @@ -397,7 +481,17 @@ function decideNodeIslands( ), ); } - return packages.size === 0 ? null : { packages, notes }; + for (const island of [...modules.values()].sort((a, b) => + a.file < b.file ? -1 : a.file > b.file ? 1 : 0, + )) + notes.push( + nodeIslandModuleNoteDiag(island.file, island.class, island.reason, { + file: island.file, + start: 0, + end: 0, + }), + ); + return packages.size === 0 && modules.size === 0 ? null : { packages, modules, notes }; } /** The island set grown by the npm packages whose own modules refused to @@ -478,6 +572,41 @@ function loweringIslands( return grown.size === known.size ? null : grown; } +/** The island modules grown by the lowering's refusals in user JavaScript + * (`--node-islands=auto` only), or null when there are none: a sloppy + * module whose semantics the native pipeline refuses (an implicit global, + * a mapped `arguments`, a `with` the pre-scan could not decide) runs in + * Node. One retry adds every one of them at once. */ +function loweringModules( + entryPath: string, + opts: CompileRequestOptions, + diagnostics: readonly ScrDiagnostic[], + known: ReadonlyMap, +): Map | null { + const policy = nodeIslandPolicy(readProjectConfig(entryPath), opts.nodeIslands); + if (policy.mode !== "auto" || known.size > 16) return null; + const grown = new Map(known); + for (const d of diagnostics) { + if (!isIslandReasonCode(d.code)) continue; + const file = d.loc.file; + // Packages run in Node through their own decision (loweringIslands). + if (file.includes("/node_modules/") || packageDirOf(file) !== null) continue; + if (!isJsSourceFileName(file) || sameFile(file, entryPath)) continue; + if (grown.has(file)) continue; + const reason = classifyDiagnostic(d, "refusal"); + // A gap is a construct scriptc does not implement, not a sloppy + // semantic: running it in Node would hide a compiler limitation. + if (reason.class === "gap") continue; + grown.set(file, { + file, + class: reason.class, + reason: describe(reason), + decision: "auto", + }); + } + return grown.size === known.size ? null : grown; +} + /** Preflight refusals at import statements of natively compiled npm * packages (`--node-islands=auto` only): those packages become islands. */ function preflightIslands( diff --git a/packages/compiler/src/frontend/lowering/lower-modules.ts b/packages/compiler/src/frontend/lowering/lower-modules.ts index 00299ce58..6091b8ce4 100644 --- a/packages/compiler/src/frontend/lowering/lower-modules.ts +++ b/packages/compiler/src/frontend/lowering/lower-modules.ts @@ -665,6 +665,10 @@ export function collectNpmImports(lowerer: Lowerer, parts: FileParts[]): void { // graph: resolveImport follows its node_modules link to the loaded // TypeScript entry. Its internal `.js` spellings have already been // resolved to `.ts` by that graph. + // A relative import of a program module the pre-scan put in a Node + // island (DESIGN.md note 1 §3.2): the island loads at this module's + // position and its bindings are island values. + if (lowerer.nodeIslands !== null && collectIslandImport(lowerer, fp.sf, stmt, spec)) continue; const projectDep = isRelativeSpecifier(spec) ? null : resolveImport(lowerer.program, fp.sf, spec); diff --git a/packages/compiler/src/frontend/lowering/lower-node-island.ts b/packages/compiler/src/frontend/lowering/lower-node-island.ts index dc49bb99f..742b47d4b 100644 --- a/packages/compiler/src/frontend/lowering/lower-node-island.ts +++ b/packages/compiler/src/frontend/lowering/lower-node-island.ts @@ -16,11 +16,12 @@ * checked against the declared result type, like `JSON.parse(s) as T`; a * thrown Error crosses as its nearest intrinsic class with its message and * string `code`. */ -import { readFileSync, realpathSync } from "node:fs"; +import { readFileSync, realpathSync, statSync } from "node:fs"; import { resolveFrom } from "../../node-islands/resolve.js"; -import { dirname, isAbsolute, join } from "node:path"; +import { dirname, isAbsolute, join, resolve } from "node:path"; import * as ts from "../ts7/adapter.js"; import { isJsSourceFile, locOf } from "../program.js"; +import { isRelativeSpecifier } from "../workspace-registry.js"; import type { NodeApiMember } from "../node-api-import.js"; import { PoisonError, newFnCtx, type Lowerer } from "./lowerer.js"; import { @@ -49,7 +50,7 @@ import { isForeignType, isIslandSubclassDecl, markIslandSubclass, - islandPackageOfFile, + islandOwnerOfFile, type NodeIslandSet, } from "../../node-islands/boundary.js"; import { stripTypeWrappers } from "./typed-arguments.js"; @@ -288,21 +289,54 @@ export function isEsmFile(file: string): boolean { } } +/** The file a relative (or absolute) specifier names from `fromFile`, as + * Node's CommonJS/ESM resolution would find it, or null. Extension fallback + * and directory indexes only: an island module is a file the program + * already loaded, and `exports` maps stay Node's job. */ +function relativeIslandFile(fromFile: string, spec: string): string | null { + const base = isAbsolute(spec) ? spec : resolve(dirname(fromFile), spec); + const candidates = [base]; + if (!/\.[cm]?js$/.test(base)) { + for (const ext of [".js", ".cjs", ".mjs"]) candidates.push(base + ext); + } + for (const ext of [".js", ".cjs", ".mjs"]) candidates.push(join(base, `index${ext}`)); + for (const candidate of candidates) { + try { + if (statSync(candidate).isFile()) return candidate; + } catch { + /* the next candidate */ + } + } + return null; +} + /** The island module file `spec` names from `fromFile`, or null when the - * specifier does not resolve into an island package. */ + * specifier does not resolve into an island package or module. */ export function islandModuleFile( islands: NodeIslandSet, fromFile: string, spec: string, kind: "import" | "require", ): string | null { - const js = resolveBareModule(fromFile, spec, "js-only", kind); - if (js === null) return null; - if (islandPackageOfFile(islands, js.typesFile) === null) return null; + let file: string; + if (isRelativeSpecifier(spec) || isAbsolute(spec)) { + // A program module the pre-scan put in a Node island (DESIGN.md note 1 + // §3.2): the file itself is the island, resolved as Node would. + if (islands.modules.size === 0) return null; + const resolved = relativeIslandFile(fromFile, spec); + if (resolved === null) return null; + if (islandOwnerOfFile(islands, resolved) === null) return null; + file = resolved; + } else { + const js = resolveBareModule(fromFile, spec, "js-only", kind); + if (js === null) return null; + if (islandOwnerOfFile(islands, js.typesFile) === null) return null; + file = js.typesFile; + } try { - return realpathSync(js.typesFile); + return realpathSync(file); } catch { - return js.typesFile; + return file; } } diff --git a/packages/compiler/src/frontend/lowering/lower-shared-modules.ts b/packages/compiler/src/frontend/lowering/lower-shared-modules.ts index ea0cfb702..ecae17894 100644 --- a/packages/compiler/src/frontend/lowering/lower-shared-modules.ts +++ b/packages/compiler/src/frontend/lowering/lower-shared-modules.ts @@ -415,6 +415,10 @@ export function sharedModulePlans(lowerer: Lowerer): SharedModulePlan[] { } for (const island of lowerer.nodeIslands?.packages.values() ?? []) packageJsFiles(island.dir, queue); + // A program module the build runs in a Node island is loaded by Node, so + // the program modules IT imports are shared with it like an island + // package's (DESIGN.md note 1 §3.2). + for (const module of lowerer.nodeIslands?.modules.values() ?? []) queue.push(real(module.file)); if (queue.length === 0 && candidates.size === 0) return plans; const seen = new Set(); // Classes island code extends, by the compiled module that exports them. diff --git a/packages/compiler/src/frontend/pipeline.ts b/packages/compiler/src/frontend/pipeline.ts index ab3e5cc83..cbd1fb238 100644 --- a/packages/compiler/src/frontend/pipeline.ts +++ b/packages/compiler/src/frontend/pipeline.ts @@ -146,6 +146,10 @@ export interface FrontendOptions { /** Packages that run in Node islands (scope-B NI-4): never compiled, * typed by their declarations. */ islandPackages?: ReadonlySet; + /** Program modules that run in Node islands (DESIGN.md note 1 §3.2): + * their bodies are never compiled; their imports lower to island loads. + * Absolute file paths. */ + islandModules?: ReadonlySet; /** The Node island report (coverage): the project's scriptc settings and * the detector's scan of the program and its npm packages. Builds do not * read either. */ @@ -565,6 +569,11 @@ export function runFrontend( } } +/** A file path as the frontend and the island decision spell it. */ +function normalizeFile(file: string): string { + return file.replaceAll("\\", "/"); +} + /** Rounds of dropping single refused overlay annotations before the * fallback drops whole files' overlays. */ const OVERLAY_ANNOTATION_ROUNDS = 4; @@ -586,6 +595,18 @@ function loadFrontend( beginShapeOverlayBuild(); const statuses: NpmStaticStatus[] = []; const npmSites = new Map(); + // Program modules that run in Node islands (DESIGN.md note 1 §3.2): their + // bodies are not lowered, so their sloppy semantics stay Node's, and a + // diagnostic anchored in one does not fail the program the build does not + // compile. + const islandModules = + options.islandModules === undefined || options.islandModules.size === 0 + ? null + : new Set([...options.islandModules].map(normalizeFile)); + const inIslandModule = (loc: SrcLoc | undefined): boolean => + islandModules !== null && loc !== undefined && islandModules.has(normalizeFile(loc.file)); + const withoutIslandModules = (diags: ScrDiagnostic[]): ScrDiagnostic[] => + islandModules === null ? diags : diags.filter((d) => !inIslandModule(d.loc)); // Node island detection (the report, or a build that may run islands): // how each package was refused, and the modules of packages removed from // the program. @@ -848,8 +869,9 @@ function loadFrontend( const patchDiagnostics = typePatchDiagnostics()?.diagnostics ?? []; const buildWarnings = [...(report ? [] : configDiagnostics), ...patchDiagnostics]; return { - preflight: + preflight: withoutIslandModules( report && configDiagnostics.length > 0 ? [...configDiagnostics, ...preflight] : preflight, + ), ...(buildWarnings.length > 0 ? { warnings: buildWarnings } : {}), entryText: () => finalLoad.entry.text, entryExports: () => entryFunctionExports(finalLoad.entry), @@ -896,9 +918,22 @@ function loadFrontend( return texts; }, lower: (opts, valid) => { + // The module order without the island modules, computed once per load: + // a reload keeps the array the lowering appends its dynamic import + // modules to. + const islandFiltered = new WeakMap(); + const lowerOrder = (l: typeof finalLoad): SourceFile[] => { + if (islandModules === null) return l.moduleOrder; + let kept = islandFiltered.get(l.moduleOrder); + if (kept === undefined) { + kept = l.moduleOrder.filter((sf) => !islandModules.has(normalizeFile(sf.fileName))); + islandFiltered.set(l.moduleOrder, kept); + } + return kept; + }; const lowerLoad = (l: typeof finalLoad): LowerResult => { if (l.shapeOverlayState !== undefined) restoreShapeOverlayState(l.shapeOverlayState); - return lowerToIr(l.program, l.entry, l.moduleOrder, { + return lowerToIr(l.program, l.entry, lowerOrder(l), { ...opts, frontendServices: l.services, startupCrash: l.startupCrash ?? null, @@ -964,7 +999,7 @@ function loadFrontend( const t1 = performance.now(); // The preflight fills the module order; the tsc world reads the // files as written, so it answers as the first load did. - const reloadPreflight = checkPreflight(finalLoad); + const reloadPreflight = withoutIslandModules(checkPreflight(finalLoad)); if (reloadPreflight.length > 0) return { ...lowered, module: null, diagnostics: reloadPreflight }; const t2 = performance.now(); @@ -1066,7 +1101,7 @@ function loadFrontend( setShapeOverlayDropped(true); finalLoad.dispose(); finalLoad = loadProgram(entryPath, { npmStatic: finalNpmStatic, externalTypes }); - const reloadPreflight = checkPreflight(finalLoad); + const reloadPreflight = withoutIslandModules(checkPreflight(finalLoad)); if (reloadPreflight.length > 0) return { ...lowered, module: null, diagnostics: reloadPreflight }; lowered = lowerOnce(); diff --git a/packages/compiler/src/index.ts b/packages/compiler/src/index.ts index 34ea20b28..927b56e68 100644 --- a/packages/compiler/src/index.ts +++ b/packages/compiler/src/index.ts @@ -539,10 +539,11 @@ export async function compile( } /** Divergence warnings collected while a compile lowers (none when a cache - * hit skips lowering). */ + * hit skips lowering), and the Node islands the build ran. */ interface CompileWarningSink { warnings: ScrDiagnostic[]; sourceTexts: Map | null; + islands?: { packages: string[]; modules: string[] }; } /** The package.json texts type patch warnings point into, re-read for a @@ -560,6 +561,15 @@ function cachedWarningSources(warnings: readonly ScrDiagnostic[]): Map( result: T, sink: CompileWarningSink, ): T { - if (!result.ok || sink.warnings.length === 0) return result; - return { ...result, warnings: sink.warnings, sourceTexts: sink.sourceTexts ?? new Map() }; + if (!result.ok) return result; + const islands = sink.islands === undefined ? {} : { islands: sink.islands }; + if (sink.warnings.length === 0) return { ...result, ...islands }; + return { + ...result, + warnings: sink.warnings, + sourceTexts: sink.sourceTexts ?? new Map(), + ...islands, + }; } /** Build-time API compatibility fence: a caller's existing CompileOptions @@ -1026,6 +1043,9 @@ interface PreparedExecutable { nativeFeatures: EarlyExecutableNativeFeatures; programSplit: ReturnType; sourceTexts: Map; + /** The Node islands the build ran, reported with its result (a runner + * reads them to tell an island result from a native one). */ + islands?: { packages: string[]; modules: string[] }; } /** Finish frontend work in a separate scope so the AST, checker caches and @@ -1103,6 +1123,8 @@ async function prepareExecutableInput( warningSink.warnings = prepared.warnings; warningSink.sourceTexts = sourceTexts; } + if (warningSink !== undefined && prepared.islands !== undefined) + warningSink.islands = prepared.islands; const stem = basename(entryPath).replace(/\.(ts|mts|cts|js|mjs|cjs)$/, ""); const defaultSourcePaths = { @@ -1262,6 +1284,7 @@ async function prepareExecutableInput( nativeFeatures, programSplit, sourceTexts, + ...(prepared.islands === undefined ? {} : { islands: prepared.islands }), }; } @@ -1447,6 +1470,7 @@ type FrontendProcessResult = nativeFeatures: EarlyExecutableNativeFeatures; sourceTexts: [string, string][]; warnings: ScrDiagnostic[]; + islands?: { packages: string[]; modules: string[] }; frontend: FrontendInputSnapshot; } | { @@ -1507,6 +1531,8 @@ async function prepareExecutableInputInChild( warningSink.warnings = result.warnings; warningSink.sourceTexts = sourceTexts; } + if (warningSink !== undefined && result.kind === "prepared" && result.islands !== undefined) + warningSink.islands = result.islands; if (result.kind === "diagnostics") return { ok: false, diagnostics: result.diagnostics, sourceTexts }; return { @@ -1518,6 +1544,7 @@ async function prepareExecutableInputInChild( nativeFeatures: result.nativeFeatures, programSplit: null, sourceTexts, + ...(result.islands === undefined ? {} : { islands: result.islands }), }; } @@ -1595,6 +1622,7 @@ export async function runFrontendProcess(requestPath: string, resultPath: string nativeFeatures: prepared.nativeFeatures, sourceTexts: [...prepared.sourceTexts], warnings: warnings.warnings, + ...(prepared.islands === undefined ? {} : { islands: prepared.islands }), frontend, }; } else if (!prepared.ok) { @@ -2007,6 +2035,8 @@ async function compileTracked( warningSink.warnings = earlyHit.performance; warningSink.sourceTexts = cachedWarningSources(earlyHit.performance); } + if (warningSink !== undefined && earlyHit.islands !== undefined) + warningSink.islands = earlyHit.islands; const executableCacheOptions = earlyCacheOptions; // Route/proof metadata is independently evictable. A full-compiler // fallback that still finds the validated payload repairs that lightweight @@ -2253,6 +2283,7 @@ async function compileTracked( frontend: frontendInputs.snapshot(), ...(irSourcePath === undefined ? {} : { irPath: irSourcePath }), ...performanceWarnings(warningSink), + ...islandFields(warningSink), ...profileMatchField(nativeProgramObject), }); publishedExecutable = true; @@ -2275,6 +2306,7 @@ async function compileTracked( frontend: frontendInputs.snapshot(), ...(irSourcePath === undefined ? {} : { irPath: irSourcePath }), ...performanceWarnings(warningSink), + ...islandFields(warningSink), ...profileMatchField(nativeProgramObject), }).catch(() => undefined); } diff --git a/packages/compiler/src/node-islands/boundary.ts b/packages/compiler/src/node-islands/boundary.ts index be4339391..bd317e23f 100644 --- a/packages/compiler/src/node-islands/boundary.ts +++ b/packages/compiler/src/node-islands/boundary.ts @@ -28,9 +28,45 @@ export interface IslandPackage { reason: string; } -/** The program's island packages, by package name. */ +/** One program module the build runs in a Node island (DESIGN.md + * note 1 §3.2): a user JavaScript file whose semantics need the JavaScript + * engine, keyed by its absolute path. */ +export interface IslandModule { + /** The module file (absolute, as the checker spells it). */ + file: string; + /** The island class (runtime, object-model, gap, packaging) and the first + * reason, for the SC8001 note. */ + class: string; + reason: string; + /** Marked by the user (`--node-island`, `nodeIslands.modules`) or + * detected by the pre-scan under `--node-islands=auto`. */ + decision: "marked" | "auto"; +} + +/** The program's island packages, by package name, and island modules, by + * absolute file path. */ export interface NodeIslandSet { packages: ReadonlyMap; + modules: ReadonlyMap; +} + +function normalizePath(file: string): string { + return file.replaceAll("\\", "/"); +} + +/** The island module at `file`, or null. */ +export function islandModuleOfFile(islands: NodeIslandSet, file: string): IslandModule | null { + if (islands.modules.size === 0) return null; + return islands.modules.get(file) ?? islands.modules.get(normalizePath(file)) ?? null; +} + +/** The island (package or module) that owns a declaration or JavaScript + * file, or null. Modules first: a user module is never inside a package. */ +export function islandOwnerOfFile( + islands: NodeIslandSet, + file: string, +): IslandPackage | IslandModule | null { + return islandModuleOfFile(islands, file) ?? islandPackageOfFile(islands, file); } /** The island package that owns a declaration or JavaScript file, or null. @@ -65,7 +101,7 @@ function declaredInIsland(islands: NodeIslandSet, symbol: ts.Symbol | null | und if (decls.length === 0) return false; if (islandSubclassDecls.size > 0 && decls.some((d) => islandSubclassDecls.has(d))) return true; return decls.every( - (d) => islandPackageOfFile(islands, ts.getSourceFileOfNode(d).fileName) !== null, + (d) => islandOwnerOfFile(islands, ts.getSourceFileOfNode(d).fileName) !== null, ); } @@ -150,7 +186,8 @@ export function crossingOf( let active: NodeIslandSet | null = null; export function setActiveNodeIslands(islands: NodeIslandSet | null): void { - active = islands !== null && islands.packages.size > 0 ? islands : null; + active = + islands !== null && (islands.packages.size > 0 || islands.modules.size > 0) ? islands : null; } export function activeNodeIslands(): NodeIslandSet | null { diff --git a/packages/compiler/src/node-islands/policy.ts b/packages/compiler/src/node-islands/policy.ts index 72dbf3cd0..945bdfd22 100644 --- a/packages/compiler/src/node-islands/policy.ts +++ b/packages/compiler/src/node-islands/policy.ts @@ -4,9 +4,8 @@ * become islands only when marked (`nodeIslands.modules`, * `--node-island=`); `"native"` (`--native=`) makes one * dependency strict; `--node-islands=off` makes the whole program strict. - * - * This version is report-only: the policy decides what WOULD run in Node, - * and coverage reports it, but code generation does not change. */ + * The decision drives code generation: an island package or module is not + * compiled natively but loaded by the program's Node island. */ import { isAbsolute, join, resolve, dirname } from "node:path"; import type { ProjectConfig } from "../project-config.js"; From 9aaccac8dca7e6e97ae99ec38f0d02074a605a32 Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Wed, 28 Oct 2026 13:53:24 -0700 Subject: [PATCH 02/14] Detect the sloppy-only constructs a native build cannot express MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three pre-scan markers name JavaScript whose semantics need the engine: a direct call of `eval` (it reads and writes the caller's scope), a function declaration in a block in sloppy code (Annex B.3.2 also binds the name as a `var` in the enclosing function), and `this` in a sloppy plain function (a plain call gets the global object). `with` now marks only sloppy code, where strict code is an early error Node refuses anyway. The `this` marker is conservative by design: a file cannot see its call sites, so every `this` in a sloppy plain function marks the module, except the shapes the file itself proves are receiver calls — a `new`-only constructor and an `X.prototype.name = function` method, neither of which is also called bare (decision S5). Markers narrow, never move, the native passes: inside a function body a block-level declaration is ordinary, the comma and `globalThis` spellings of `eval` stay indirect, and a file that does not parse as sloppy JavaScript keeps none of the sloppy markers, so a parse-negative test stays the compile refusal it is. Co-Authored-By: Claude Opus 5.5 --- .../compiler/src/node-islands/detect.test.ts | 80 +++++++++- packages/compiler/src/node-islands/detect.ts | 142 ++++++++++++++++-- packages/compiler/src/node-islands/rules.ts | 3 + packages/compiler/src/node-islands/scan.ts | 41 ++++- 4 files changed, 254 insertions(+), 12 deletions(-) diff --git a/packages/compiler/src/node-islands/detect.test.ts b/packages/compiler/src/node-islands/detect.test.ts index 2a21395da..72cbae9c7 100644 --- a/packages/compiler/src/node-islands/detect.test.ts +++ b/packages/compiler/src/node-islands/detect.test.ts @@ -13,7 +13,8 @@ function markers(source: string, ext = "cjs"): string[] { describe("R1 runtime markers", () => { test("R1.1 code from strings", () => { - expect(markers("eval(code);")).toEqual(["R1.1 code-from-string"]); + // A bare eval() is also a sloppy-mode construct (R2.4 direct-eval). + expect(markers("eval(code);")).toEqual(["R1.1 code-from-string", "R2.4 direct-eval"]); expect(markers("(0, eval)(code);")).toEqual(["R1.1 code-from-string"]); expect(markers("const f = new Function('a', 'return a');")).toEqual(["R1.1 code-from-string"]); expect(markers("const f = Function('a', body);")).toEqual(["R1.1 code-from-string"]); @@ -80,13 +81,16 @@ describe("R2 object-model markers", () => { expect(markers("const util = require('util'); util.inherits(A, B);")).toEqual([ "R2.1 constructor-function", ]); + // `this` in a plain function is also the conservative sloppy-this marker + // (decision S2): the file cannot see a call site in another module. expect(markers("function Child() { Parent.call(this, 1); }")).toEqual([ "R2.1 constructor-function", + "R2.4 sloppy-this", ]); expect(markers("class C {} const c = new C();")).toEqual([]); expect( markers("function wrap(fn) { return function () { return fn.apply(this, arguments); }; }"), - ).toEqual([]); + ).toEqual(["R2.4 sloppy-this"]); }); test("R2.2 prototype mutation", () => { @@ -133,6 +137,77 @@ describe("R2 object-model markers", () => { expect(markers("class C { m(a) { a = 2; return arguments[0]; } }")).toEqual([]); }); + test("R2.4 direct eval in sloppy mode", () => { + expect(markers("eval(code);")).toEqual(["R1.1 code-from-string", "R2.4 direct-eval"]); + expect(markers("function f() { return eval('x'); }")).toEqual([ + "R1.1 code-from-string", + "R2.4 direct-eval", + ]); + // Indirect, aliased and builtin-object calls read no local scope. + expect(markers("(0, eval)(code);")).toEqual(["R1.1 code-from-string"]); + // An aliased eval is not a call the scan recognizes as code-from-string. + expect(markers("const e = eval; e(code);")).toEqual([]); + expect(markers("globalThis.eval(code);")).toEqual(["R1.1 code-from-string"]); + expect(markers("function eval(code) { return code; } eval('x');")).toEqual([]); + // Strict code's direct eval cannot see the caller's scope: no marker. + expect(markers("'use strict'; eval(code);")).toEqual(["R1.1 code-from-string"]); + expect(markers("export function f() { return eval('x'); }", "mjs")).toEqual([ + "R1.1 code-from-string", + ]); + }); + + test("R2.4 Annex B block functions in sloppy mode", () => { + // A block-level function declaration also creates a var-like binding in + // the enclosing function (Annex B.3.2). + expect(markers("if (x) { function f() {} f(); }")).toEqual(["R2.4 annexb-block-function"]); + expect(markers("{ function f() {} }")).toEqual(["R2.4 annexb-block-function"]); + expect(markers("function g() { { function f() {} } }")).toEqual(["R2.4 annexb-block-function"]); + // A declaration in a function body or at the top level is ordinary, and + // a strict block declaration is block-scoped. + expect(markers("function f() {}")).toEqual([]); + expect(markers("function g() { function f() {} }")).toEqual([]); + expect(markers("'use strict'; if (x) { function f() {} }")).toEqual([]); + expect(markers("export function g() { if (x) { function f() {} } }", "mjs")).toEqual([]); + }); + + test("R2.4 'this' in a plain sloppy call (decision S2)", () => { + // A plain call's `this` is the global object in sloppy mode and + // undefined in strict mode: the file cannot see its call sites, so any + // plain function's `this` marks the module. + expect(markers("function f() { return this; }")).toEqual(["R2.4 sloppy-this"]); + expect(markers("const f = function () { return this; };")).toEqual(["R2.4 sloppy-this"]); + expect(markers("function f() { const g = () => this; return g(); }")).toEqual([ + "R2.4 sloppy-this", + ]); + expect(markers("function f() { return { m() { return this; } }; }")).toEqual([]); + // A receiver-carrying call, a class body, a class member initializer, + // top-level `this` (the exports object) and strict code carry no marker. + expect(markers("const o = { m() { return this; } }; o.m();")).toEqual([]); + // A property-assigned function expression is indistinguishable from a + // bare-call site in another module: conservative marking (S2). + expect(markers("const o = { m: function () { return this; } };")).toEqual(["R2.4 sloppy-this"]); + expect(markers("class C { m() { return this; } static s = this; }")).toEqual([]); + expect(markers("this.x = 1;")).toEqual([]); + expect(markers("'use strict'; function f() { return this; }")).toEqual([]); + expect(markers("export function f() { return this; }", "mjs")).toEqual([]); + }); + + test("R2.4 receiver-proven functions stay native (decision S5)", () => { + const sloppy = (source: string) => markers(source).filter((m) => m.startsWith("R2.4")); + // Native passes the receiver for both shapes: `new X()` and a + // `X.prototype.name` method. Marking them would move native passes. + expect(sloppy("function Ctor() { this.x = 1; } new Ctor();")).toEqual([]); + expect(sloppy("const Ctor = function () { this.x = 1; }; new Ctor();")).toEqual([]); + expect(sloppy("function M() {} M.prototype.m = function () { return this; };")).toEqual([]); + // The name is also called plainly, so the receiver is not proven. + expect(sloppy("function Ctor() { return this; } new Ctor(); Ctor();")).toEqual([ + "R2.4 sloppy-this", + ]); + // A callback's receiver belongs to its callee, which the file cannot + // see: conservative marking (S2). + expect(sloppy("function f() { return this; } [1].map(f);")).toEqual(["R2.4 sloppy-this"]); + }); + test("R2.5 builtin and global patching", () => { expect(markers("Array.prototype.last = function () {};")).toEqual(["R2.5 builtin-write"]); expect(markers("Promise = Bluebird;")).toEqual(["R2.5 builtin-write"]); @@ -156,6 +231,7 @@ test("markers come back in source order with their locations", () => { expect(found.map((m) => [m.rule, source.slice(m.loc.start, m.loc.end)])).toEqual([ ["R2.2", "Object.setPrototypeOf(a, b)"], ["R1.1", "eval(x)"], + ["R2.4", "eval(x)"], ]); }); diff --git a/packages/compiler/src/node-islands/detect.ts b/packages/compiler/src/node-islands/detect.ts index f612d58e5..e0c341bd5 100644 --- a/packages/compiler/src/node-islands/detect.ts +++ b/packages/compiler/src/node-islands/detect.ts @@ -9,8 +9,10 @@ * declare it at all, and a builtin module object when the file binds it to * require("") or imports it from one. Checker-dependent parts of * R2.3 (writes the declared type does not cover, freeze followed by writes) - * and R2.4 (assignments to undeclared identifiers, `this` as the global - * object) are left to the lowering. */ + * and R2.4 (assignments to undeclared identifiers) are left to the lowering; + * `this` in a plain call is marked conservatively (decision S2): a file + * cannot see its call sites, so every `this` in a sloppy plain function + * marks the module. */ import type { SrcLoc } from "../ir/ir.js"; import * as ts from "../frontend/ts7/adapter.js"; import { SUPPORTED_BUILTIN_MODULES } from "../frontend/builtin-modules.js"; @@ -236,6 +238,29 @@ function isFunctionBoundary(node: ts.Node): boolean { ); } +/** The name a plain function value is bound to in the same file + * (`function f`, `var f = function`), for the constructor refinement. */ +function boundFunctionName(fn: ts.FunctionDeclaration | ts.FunctionExpression): string | null { + if (ts.isFunctionDeclaration(fn)) return fn.name?.text ?? null; + const parent = fn.parent; + if (parent === undefined) return null; + if (ts.isVariableDeclaration(parent) && ts.isIdentifier(parent.name)) return parent.name.text; + if (ts.isPropertyAssignment(parent)) return propertyName(parent.name); + return null; +} + +/** `X.prototype.name = function () {}`: a method assignment, whose every + * call through the same spelling has a receiver. */ +function isPrototypeFunctionAssignment(left: ts.PropertyAccessExpression, right: ts.Node): boolean { + let value = right; + while (ts.isParenthesizedExpression(value)) value = value.expression; + return ( + ts.isFunctionExpression(value) && + ts.isPropertyAccessExpression(left.expression) && + propertyName(left.expression) === "prototype" + ); +} + /** One module's pre-scan. */ class ModuleScanner { readonly markers: NodeIslandMarker[] = []; @@ -250,6 +275,15 @@ class ModuleScanner { private readonly builtinBindings = new Map(); /** Local names bound to a createRequire(...) result. */ private readonly createdRequires = new Set(); + /** Names the file constructs with `new`: a constructor's `this` is never + * the global object, so its `this` uses need no island (decision S5). */ + private readonly constructed = new Set(); + /** Names the file calls as a bare identifier, `f(...)`: a name used both + * ways is a constructor and a plain function, so it still marks. */ + private readonly bareCalled = new Set(); + /** Function expressions the file assigns to `X.prototype.name`: every + * same-file call of them is a receiver call (decision S5). */ + private readonly prototypeFunctions = new Set(); private readonly strictness = new Map(); private readonly moduleStrict: boolean; @@ -362,6 +396,17 @@ class ModuleScanner { this.bindName(node.name, node.initializer); } else if (ts.isParameter(node)) { this.bindName(node.name, undefined); + } else if (ts.isNewExpression(node) && ts.isIdentifier(node.expression)) { + this.constructed.add(node.expression.text); + } else if (ts.isCallExpression(node) && ts.isIdentifier(node.expression)) { + this.bareCalled.add(node.expression.text); + } else if ( + ts.isBinaryExpression(node) && + node.operatorToken.kind === ts.SyntaxKind.EqualsToken && + ts.isPropertyAccessExpression(node.left) && + isPrototypeFunctionAssignment(node.left, node.right) + ) { + this.prototypeFunctions.add(node.right); } else if (ts.isImportDeclaration(node)) { const spec = literalText(node.moduleSpecifier); const module = spec === null ? null : builtinModuleName(spec); @@ -427,6 +472,67 @@ class ModuleScanner { return BUILTIN_GLOBALS.has(name) && !this.declared.has(name); } + /** A function declaration whose parent is a block that is not a function + * body: Annex B.3.2's block-level declaration, which in sloppy code also + * creates a `var`-like binding in the enclosing function (DESIGN.md note 1 + * §3.1, construct 5). A declaration directly in a function body or at the + * top level of a script is ordinary. */ + private isAnnexBBlockFunction(fn: ts.FunctionDeclaration): boolean { + const parent = fn.parent; + if (parent === undefined || !ts.isBlock(parent)) return false; + const owner = parent.parent; + if (owner === undefined) return false; + // A function-like body, a class static block or a class member + // initializer block is not a bare block statement. + return (owner as ts.FunctionLikeDeclaration).body !== parent; + } + + /** Whether the file shows the function is only ever called with a + * receiver, so native's missing sloppy `this` cannot be observed: it is + * constructed with `new`, or it is a `X.prototype.name` method (decision + * S5 — the marker islands what native gets wrong, and native passes the + * receiver for both shapes). */ + private receiverSafe(fn: ts.FunctionDeclaration | ts.FunctionExpression): boolean { + if (this.prototypeFunctions.has(fn)) return true; + const name = boundFunctionName(fn); + return name !== null && this.constructed.has(name) && !this.bareCalled.has(name); + } + + /** `this` in a plain function call: a sloppy function that is not a + * method, constructor, accessor or class member, and is not an arrow + * function (whose `this` is its enclosing function's). Conservative + * (decision S2): a file-level scan cannot see cross-file call sites, so + * any `this` in such a function marks the module, except the shapes the + * file itself proves are receiver calls (decision S5). Top-level `this` + * is not this marker: it is the module's exports object, and the test262 + * profile's script-environment exclusion covers the script form. */ + private visitThis(node: ts.Node): void { + if (this.isStrict(node)) return; + for (let cur: ts.Node | undefined = node.parent; cur !== undefined; cur = cur.parent) { + // An arrow function has no `this` of its own: keep looking outward. + if (ts.isArrowFunction(cur)) continue; + if (ts.isFunctionDeclaration(cur) || ts.isFunctionExpression(cur)) { + if (this.receiverSafe(cur)) return; + this.add("R2.4", "sloppy-this", "'this' in a plain call in sloppy mode", node); + return; + } + // A method, constructor or accessor has a receiver; a class member + // initializer and a class static block have their own `this`; a + // property assignment's function is commonly a method too. + if ( + isFunctionBoundary(cur) || + ts.isClassDeclaration(cur) || + ts.isClassExpression(cur) || + ts.isClassStaticBlockDeclaration(cur) || + ts.isPropertyDeclaration(cur) || + ts.isPropertyAssignment(cur) + ) + return; + // Top-level `this` is the module's exports object, not a call site. + if (ts.isSourceFile(cur)) return; + } + } + private visit(node: ts.Node): void { if (ts.isCallExpression(node)) this.visitCall(node); else if (ts.isNewExpression(node)) this.visitNew(node); @@ -440,7 +546,9 @@ class ModuleScanner { ) this.visitWrite(node.operand, node, "write"); } else if (ts.isWithStatement(node)) { - this.add("R2.4", "with", "a 'with' statement", node); + // A `with` in strict code is an early error (Node refuses the file + // too), so only sloppy code carries the sloppy-semantics marker. + if (!this.isStrict(node)) this.add("R2.4", "with", "a 'with' statement", node); } else if (ts.isImportDeclaration(node)) { const spec = literalText(node.moduleSpecifier); if (spec !== null) this.visitSpecifier(spec, node); @@ -454,6 +562,8 @@ class ModuleScanner { const text = chainText(node); if ((text === "arguments.callee" || text === "arguments.caller") && !this.isStrict(node)) this.add("R2.4", "sloppy-arguments", `'${text}'`, node); + } else if (node.kind === ts.SyntaxKind.ThisKeyword) { + this.visitThis(node); } else if (ts.isObjectLiteralExpression(node)) { for (const prop of node.properties.nodes) { if (!ts.isPropertyAssignment(prop)) continue; @@ -506,16 +616,23 @@ class ModuleScanner { // eval, Function let target = callee; while (ts.isParenthesizedExpression(target)) target = target.expression; + // A bare `eval(...)` is the intrinsic's DIRECT call: it reads and + // creates bindings in the caller's scope, and in sloppy mode creates + // globals (DESIGN.md note 1 §3.1). `globalThis.eval(...)`, the + // comma-operator idiom `(0, eval)(...)` and the other spellings are + // indirect: they observe no caller scope, so only R1.1. + const direct = ts.isIdentifier(target) && target.text === "eval" && !this.declared.has("eval"); if (ts.isBinaryExpression(target) && target.operatorToken.kind === ts.SyntaxKind.CommaToken) target = target.right; const text = chainText(target); if (text === null) return; - if ( - (text === "eval" && !this.declared.has("eval")) || - text === "globalThis.eval" || - text === "global.eval" || - text === "window.eval" - ) { + if (text === "eval" && !this.declared.has("eval")) { + this.add("R1.1", "code-from-string", "eval()", call); + if (direct && !this.isStrict(call)) + this.add("R2.4", "direct-eval", "a direct call of eval() in sloppy mode", call); + return; + } + if (text === "globalThis.eval" || text === "global.eval" || text === "window.eval") { this.add("R1.1", "code-from-string", "eval()", call); return; } @@ -789,6 +906,13 @@ class ModuleScanner { /** Observable `arguments` aliasing in a sloppy function: writes to * arguments[i], or to a parameter while `arguments` is read. */ private visitFunction(fn: ts.FunctionLikeDeclaration): void { + if (ts.isFunctionDeclaration(fn) && this.isAnnexBBlockFunction(fn) && !this.isStrict(fn)) + this.add( + "R2.4", + "annexb-block-function", + "a function declaration in a block in sloppy mode", + fn, + ); const body = fn.body; if (body === undefined || !ts.isBlock(body)) return; if (this.isStrict(body.statements.nodes[0] ?? body)) return; diff --git a/packages/compiler/src/node-islands/rules.ts b/packages/compiler/src/node-islands/rules.ts index 25f686314..b074ac5bc 100644 --- a/packages/compiler/src/node-islands/rules.ts +++ b/packages/compiler/src/node-islands/rules.ts @@ -69,6 +69,9 @@ const REASON_RULES: Record = { with: "R2.4", "sloppy-arguments": "R2.4", "sloppy-mode": "R2.4", + "direct-eval": "R2.4", + "annexb-block-function": "R2.4", + "sloppy-this": "R2.4", "builtin-write": "R2.5", proxy: "R2.5", }; diff --git a/packages/compiler/src/node-islands/scan.ts b/packages/compiler/src/node-islands/scan.ts index 0ac082e73..70d7355fc 100644 --- a/packages/compiler/src/node-islands/scan.ts +++ b/packages/compiler/src/node-islands/scan.ts @@ -94,6 +94,44 @@ function specifiersOf(program: ts.Program, sf: ts.SourceFile): { spec: string; n return out; } +/** The diagnostics of a construct that sloppy-mode Node accepts: the + * frontend compiles every file under the project's `strict` options (with + * `alwaysStrict`), so a `with`, an octal literal, or a strict-only reserved + * word reports here even though the file itself is a CommonJS script that + * Node runs sloppy. JavaScript grammar errors arrive in the bind pass + * (TS1101 among them), so both passes filter by this set. They cannot hide + * a real syntax error: they exist only for code the engine parses in sloppy + * mode. */ +const SLOPPY_LEGAL_CODES: ReadonlySet = new Set([ + 1100, // invalid use of a strict-mode reserved word + 1101, // 'with' statements are not allowed in strict mode + 1102, // 'delete' of an identifier in strict mode + 1121, // an octal literal + 1487, // an octal escape sequence + 1210, // a class body's strict-mode restriction + 1212, // a strict-mode reserved word + 1213, // a strict-mode reserved word in a class + 1214, // a strict-mode reserved word in a module + 1215, // invalid use of a strict-mode-only construct in a module +]); + +/** Whether the file parses as the sloppy JavaScript the island would run: + * a file with a genuine syntax or early error cannot run in an island as + * written (Node refuses it too, at load), so its pre-scan markers must not + * put it there — a parse-negative test stays the compile refusal it is + * (decision S5: the markers narrow, never move, the native passes). */ +function sloppyParsable(program: ts.Program, sf: ts.SourceFile): boolean { + try { + const sloppyLegal = (d: ts.Diagnostic): boolean => SLOPPY_LEGAL_CODES.has(d.code); + return ( + program.getSyntacticDiagnostics(sf).every(sloppyLegal) && + program.getBindDiagnostics(sf).every(sloppyLegal) + ); + } catch { + return true; + } +} + /** Scans `files`: markers for npm JavaScript (and for user JavaScript when * `userJs`), and package edges for every file when `edges` (a program with * no npm package has none to find). */ @@ -112,7 +150,8 @@ export function scanFiles( if (origin === "builtin" || origin === "shim") continue; const at = origin === "npm" ? packageDirOf(sf.fileName) : null; const js = isJsSourceFileName(sf.fileName); - const markers = js && (origin === "npm" || opts.userJs) ? scanModule(sf) : []; + let markers = js && (origin === "npm" || opts.userJs) ? scanModule(sf) : []; + if (markers.length > 0 && !sloppyParsable(program, sf)) markers = []; modules.push({ file: sf.fileName, origin, From 1a1b5bbce2a535ff98d84e7eb9c017a05bc00135 Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Wed, 28 Oct 2026 13:53:32 -0700 Subject: [PATCH 03/14] Island sloppy user modules automatically under --node-islands=auto Under the default `auto` mode a user JavaScript module whose pre-scan carries a sloppy-only construct no longer stays a refusal: it joins the island set, so the program runs it in Node at the position it is required (decision S3). The marked forms (`--node-island=`, `scriptc.nodeIslands.modules`) keep working unchanged, as does the existing lowering-driven recovery. The entry stays native: a program whose entry is an island would run the whole file in Node, which is a different feature and not this slice. Co-Authored-By: Claude Opus 5.5 --- packages/compiler/src/executable/prepare.ts | 52 +++++++++++++++------ 1 file changed, 38 insertions(+), 14 deletions(-) diff --git a/packages/compiler/src/executable/prepare.ts b/packages/compiler/src/executable/prepare.ts index fc6484438..2d1e30fbc 100644 --- a/packages/compiler/src/executable/prepare.ts +++ b/packages/compiler/src/executable/prepare.ts @@ -33,7 +33,11 @@ import { } from "../node-islands/policy.js"; import { buildNodeIslandReport } from "../node-islands/report.js"; import type { IslandModule, IslandPackage, NodeIslandSet } from "../node-islands/boundary.js"; -import { classifyDiagnostic, isIslandReasonCode } from "../node-islands/classify.js"; +import { + classifyDiagnostic, + classifyMarker, + isIslandReasonCode, +} from "../node-islands/classify.js"; import { packageDirOf, packageVersionOf } from "../node-islands/origin.js"; import { describe } from "../node-islands/summary.js"; import { resolveBareModule } from "../frontend/resolve.js"; @@ -347,10 +351,22 @@ function samePath(a: string, b: string): boolean { return a.replaceAll("\\", "/") === b.replaceAll("\\", "/"); } +/** The pre-scan reasons that make a user module a sloppy-semantics island + * by themselves (DESIGN.md note 1 §3.1): the constructs whose behavior + * needs the JavaScript engine and that a syntax scan decides exactly, plus + * the conservative `this` marker (decision S2). */ +const SLOPPY_REASONS: ReadonlySet = new Set([ + "with", + "direct-eval", + "annexb-block-function", + "sloppy-this", +]); + /** The program modules that run in Node islands, by file: modules marked - * with `--node-island=` or `scriptc.nodeIslands.modules`. The entry - * stays native: a program whose entry is an island runs the whole file in - * Node, which is a different feature. */ + * with `--node-island=` or `scriptc.nodeIslands.modules`, and (under + * `--node-islands=auto`) user JavaScript the pre-scan found a sloppy-only + * construct in (decision S3). The entry stays native: a program whose entry + * is an island runs the whole file in Node, which is a different feature. */ function moduleIslands( entryPath: string, policy: NodeIslandPolicy, @@ -362,15 +378,20 @@ function moduleIslands( if (!isJsSourceFileName(m.file)) continue; if (sameFile(m.file, entryPath)) continue; const marked = isMarkedUserModule(policy, m.file); - if (marked === null) continue; + const sloppy = m.markers.filter((marker) => SLOPPY_REASONS.has(marker.reason)); + if (marked === null && (policy.mode !== "auto" || sloppy.length === 0)) continue; + const first = sloppy[0]; + const reason = first === undefined ? null : classifyMarker(first); modules.set(m.file, { file: m.file, - class: "marked", + class: reason === null ? "marked" : reason.class, reason: - marked === "cli" - ? "marked with --node-island" - : 'marked "node" in scriptc.nodeIslands.modules', - decision: "marked", + reason === null + ? marked === "cli" + ? "marked with --node-island" + : 'marked "node" in scriptc.nodeIslands.modules' + : describe(reason), + decision: marked === null ? "auto" : "marked", }); } return modules; @@ -378,9 +399,10 @@ function moduleIslands( /** The packages and modules a build runs in Node islands, or null for none: * under `--node-islands=auto` (the default) every npm package the native - * pipeline refused, plus the packages and modules marked `node`; under - * `explicit` only the marked ones; under `off`, none (refusals stay - * SC2013). Each island is reported once as an SC8001 note. */ + * pipeline refused plus the user modules the pre-scan found a sloppy + * construct in, and packages marked `node`; under `explicit` only the + * marked ones; under `off`, none (refusals stay SC2013). Each island is + * reported once as an SC8001 note. */ function decideNodeIslands( entryPath: string, opts: CompileRequestOptions, @@ -400,7 +422,9 @@ function decideNodeIslands( : [], ); for (const name of lowerIslands.keys()) refused.add(name); - const scan = fe.nodeIslandScan({ userJs: false }); + // The pre-scan's user-JavaScript markers exist only where a build may act + // on them. + const scan = fe.nodeIslandScan({ userJs: policy.mode === "auto" }); const modules = moduleIslands(entryPath, policy, scan); // Modules the lowering refused keep their first reason (the reload no // longer compiles them). From 273c9e550637001296002ec1202ab87517dc6d90 Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Wed, 28 Oct 2026 13:54:08 -0700 Subject: [PATCH 04/14] Prove the sloppy island semantics against Node in the corpus Four programs whose user module needs the engine, each run under Node and as a compiled binary with byte-identical stdout, stderr and exit status: `with` (including the shadowed and nested lookups), Annex B.3.2 block functions (before, after and inside a loop), direct `eval` (reading and writing the caller's scope, and the `var` it declares landing in the caller's function), and `this` in a plain call (plain, `call`, `apply`, bound, and an arrow function's enclosing `this`). Each entry stays native and requires its module, so the programs exercise the module island path end to end rather than a whole-file island. Co-Authored-By: Claude Opus 5.5 --- .../8423-sloppy-with-island/main.js.json | 107 ++++++++++++++++++ .../main.js.json | 7 ++ .../main.js.json | 7 ++ .../8426-sloppy-this-island/main.js.json | 7 ++ .../corpus/8423-sloppy-with-island/lookup.js | 32 ++++++ tests/corpus/8423-sloppy-with-island/main.js | 10 ++ .../8423-sloppy-with-island/package.json | 3 + .../annexb.js | 47 ++++++++ .../main.js | 9 ++ .../package.json | 3 + .../8425-sloppy-direct-eval-island/main.js | 8 ++ .../package.json | 3 + .../8425-sloppy-direct-eval-island/scopes.js | 42 +++++++ tests/corpus/8426-sloppy-this-island/main.js | 9 ++ .../8426-sloppy-this-island/package.json | 3 + .../8426-sloppy-this-island/receivers.js | 29 +++++ .../8423-sloppy-with-island/main.js.json | 1 + .../main.js.json | 1 + .../main.js.json | 1 + .../8426-sloppy-this-island/main.js.json | 1 + 20 files changed, 330 insertions(+) create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/corpus/8423-sloppy-with-island/main.js.json create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/corpus/8424-sloppy-annexb-block-function-island/main.js.json create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/corpus/8425-sloppy-direct-eval-island/main.js.json create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/corpus/8426-sloppy-this-island/main.js.json create mode 100644 tests/corpus/8423-sloppy-with-island/lookup.js create mode 100644 tests/corpus/8423-sloppy-with-island/main.js create mode 100644 tests/corpus/8423-sloppy-with-island/package.json create mode 100644 tests/corpus/8424-sloppy-annexb-block-function-island/annexb.js create mode 100644 tests/corpus/8424-sloppy-annexb-block-function-island/main.js create mode 100644 tests/corpus/8424-sloppy-annexb-block-function-island/package.json create mode 100644 tests/corpus/8425-sloppy-direct-eval-island/main.js create mode 100644 tests/corpus/8425-sloppy-direct-eval-island/package.json create mode 100644 tests/corpus/8425-sloppy-direct-eval-island/scopes.js create mode 100644 tests/corpus/8426-sloppy-this-island/main.js create mode 100644 tests/corpus/8426-sloppy-this-island/package.json create mode 100644 tests/corpus/8426-sloppy-this-island/receivers.js create mode 100644 tests/harness/corpus-costs/8423-sloppy-with-island/main.js.json create mode 100644 tests/harness/corpus-costs/8424-sloppy-annexb-block-function-island/main.js.json create mode 100644 tests/harness/corpus-costs/8425-sloppy-direct-eval-island/main.js.json create mode 100644 tests/harness/corpus-costs/8426-sloppy-this-island/main.js.json diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/8423-sloppy-with-island/main.js.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/8423-sloppy-with-island/main.js.json new file mode 100644 index 000000000..c6b9ad9a0 --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/8423-sloppy-with-island/main.js.json @@ -0,0 +1,107 @@ +{ + "order": [ + "/tests/corpus/8423-sloppy-with-island/lookup.js", + "/tests/corpus/8423-sloppy-with-island/main.js" + ], + "diags": [ + { + "code": "SC0001", + "message": "'with' statements are not allowed in strict mode.", + "loc": { + "file": "/tests/corpus/8423-sloppy-with-island/lookup.js", + "start": 194, + "end": 198 + } + }, + { + "code": "SC0001", + "message": "The 'with' statement is not supported. All symbols in a 'with' block will have type 'any'.", + "loc": { + "file": "/tests/corpus/8423-sloppy-with-island/lookup.js", + "start": 194, + "end": 207 + } + }, + { + "code": "SC0001", + "message": "Cannot find name 'last'.", + "loc": { + "file": "/tests/corpus/8423-sloppy-with-island/lookup.js", + "start": 252, + "end": 256 + } + }, + { + "code": "SC0001", + "message": "'with' statements are not allowed in strict mode.", + "loc": { + "file": "/tests/corpus/8423-sloppy-with-island/lookup.js", + "start": 295, + "end": 299 + } + }, + { + "code": "SC0001", + "message": "The 'with' statement is not supported. All symbols in a 'with' block will have type 'any'.", + "loc": { + "file": "/tests/corpus/8423-sloppy-with-island/lookup.js", + "start": 295, + "end": 308 + } + }, + { + "code": "SC0001", + "message": "'with' statements are not allowed in strict mode.", + "loc": { + "file": "/tests/corpus/8423-sloppy-with-island/lookup.js", + "start": 422, + "end": 426 + } + }, + { + "code": "SC0001", + "message": "The 'with' statement is not supported. All symbols in a 'with' block will have type 'any'.", + "loc": { + "file": "/tests/corpus/8423-sloppy-with-island/lookup.js", + "start": 422, + "end": 447 + } + }, + { + "code": "SC0001", + "message": "'with' statements are not allowed in strict mode.", + "loc": { + "file": "/tests/corpus/8423-sloppy-with-island/lookup.js", + "start": 528, + "end": 532 + } + }, + { + "code": "SC0001", + "message": "The 'with' statement is not supported. All symbols in a 'with' block will have type 'any'.", + "loc": { + "file": "/tests/corpus/8423-sloppy-with-island/lookup.js", + "start": 528, + "end": 541 + } + }, + { + "code": "SC0001", + "message": "'with' statements are not allowed in strict mode.", + "loc": { + "file": "/tests/corpus/8423-sloppy-with-island/lookup.js", + "start": 548, + "end": 552 + } + }, + { + "code": "SC0001", + "message": "Cannot find name 'inner'.", + "loc": { + "file": "/tests/corpus/8423-sloppy-with-island/lookup.js", + "start": 576, + "end": 581 + } + } + ] +} diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/8424-sloppy-annexb-block-function-island/main.js.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/8424-sloppy-annexb-block-function-island/main.js.json new file mode 100644 index 000000000..c10f1a5fe --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/8424-sloppy-annexb-block-function-island/main.js.json @@ -0,0 +1,7 @@ +{ + "order": [ + "/tests/corpus/8424-sloppy-annexb-block-function-island/annexb.js", + "/tests/corpus/8424-sloppy-annexb-block-function-island/main.js" + ], + "diags": [] +} diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/8425-sloppy-direct-eval-island/main.js.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/8425-sloppy-direct-eval-island/main.js.json new file mode 100644 index 000000000..fc8715ea9 --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/8425-sloppy-direct-eval-island/main.js.json @@ -0,0 +1,7 @@ +{ + "order": [ + "/tests/corpus/8425-sloppy-direct-eval-island/scopes.js", + "/tests/corpus/8425-sloppy-direct-eval-island/main.js" + ], + "diags": [] +} diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/8426-sloppy-this-island/main.js.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/8426-sloppy-this-island/main.js.json new file mode 100644 index 000000000..ea5f8598e --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/8426-sloppy-this-island/main.js.json @@ -0,0 +1,7 @@ +{ + "order": [ + "/tests/corpus/8426-sloppy-this-island/receivers.js", + "/tests/corpus/8426-sloppy-this-island/main.js" + ], + "diags": [] +} diff --git a/tests/corpus/8423-sloppy-with-island/lookup.js b/tests/corpus/8423-sloppy-with-island/lookup.js new file mode 100644 index 000000000..2ea0f4239 --- /dev/null +++ b/tests/corpus/8423-sloppy-with-island/lookup.js @@ -0,0 +1,32 @@ +// A `with` resolves names against the record it is given, then falls back +// to the enclosing scope when the record has no such property. +var greeting = "hello"; + +function describe(record) { + with (record) { + return greeting + " " + first + " " + last; + } +} + +function fallback(record) { + with (record) { + return typeof first === "string" ? first : "nobody"; + } +} + +function shadowed() { + var first = "outer"; + with ({ first: "inner" }) { + return first + "/" + arguments.length; + } +} + +function nested(record) { + with (record) { + with (outer) { + return inner; + } + } +} + +module.exports = { describe, fallback, shadowed, nested }; diff --git a/tests/corpus/8423-sloppy-with-island/main.js b/tests/corpus/8423-sloppy-with-island/main.js new file mode 100644 index 000000000..96038479e --- /dev/null +++ b/tests/corpus/8423-sloppy-with-island/main.js @@ -0,0 +1,10 @@ +// @requires-libnode +// A sloppy CommonJS module that uses `with` runs in a Node island +// (DESIGN.md note 1 §3.2, decision S3): the native entry requires it, the +// island loads through Node at that position, and the program's stdout +// matches Node's byte for byte. +const lookup = require("./lookup.js"); + +console.log(lookup.describe({ first: "Ada", last: "Lovelace" })); +console.log(lookup.fallback({}), lookup.fallback({ first: "Grace" })); +console.log(lookup.shadowed(), lookup.nested({ outer: { inner: "deep" } })); diff --git a/tests/corpus/8423-sloppy-with-island/package.json b/tests/corpus/8423-sloppy-with-island/package.json new file mode 100644 index 000000000..5bbefffba --- /dev/null +++ b/tests/corpus/8423-sloppy-with-island/package.json @@ -0,0 +1,3 @@ +{ + "type": "commonjs" +} diff --git a/tests/corpus/8424-sloppy-annexb-block-function-island/annexb.js b/tests/corpus/8424-sloppy-annexb-block-function-island/annexb.js new file mode 100644 index 000000000..6121eb5f4 --- /dev/null +++ b/tests/corpus/8424-sloppy-annexb-block-function-island/annexb.js @@ -0,0 +1,47 @@ +function reached(flag) { + if (flag) { + function inner() { + return "block"; + } + return inner(); + } + return typeof inner; +} + +function beforeBlock() { + var seen = typeof helper; + { + function helper() { + return "helped"; + } + } + return seen + "/" + helper(); +} + +function afterBlock() { + if (true) { + function helper() { + return "first"; + } + } else { + function helper() { + return "second"; + } + } + return helper(); +} + +function looped() { + var out = []; + for (var i = 0; i < 2; i++) { + if (i === 1) { + function last() { + return "last " + i; + } + } + out.push(typeof last); + } + return out.join(","); +} + +module.exports = { reached, beforeBlock, afterBlock, looped }; diff --git a/tests/corpus/8424-sloppy-annexb-block-function-island/main.js b/tests/corpus/8424-sloppy-annexb-block-function-island/main.js new file mode 100644 index 000000000..fd11d7850 --- /dev/null +++ b/tests/corpus/8424-sloppy-annexb-block-function-island/main.js @@ -0,0 +1,9 @@ +// @requires-libnode +// Annex B.3.2 in a sloppy Annex B block function: a block-level function +// declaration also creates a var-like binding in the enclosing function, so +// the name is visible before the block runs and after it. +const annexb = require("./annexb.js"); + +console.log(annexb.reached(true), annexb.reached(false)); +console.log(annexb.beforeBlock(), annexb.afterBlock()); +console.log(annexb.looped()); diff --git a/tests/corpus/8424-sloppy-annexb-block-function-island/package.json b/tests/corpus/8424-sloppy-annexb-block-function-island/package.json new file mode 100644 index 000000000..5bbefffba --- /dev/null +++ b/tests/corpus/8424-sloppy-annexb-block-function-island/package.json @@ -0,0 +1,3 @@ +{ + "type": "commonjs" +} diff --git a/tests/corpus/8425-sloppy-direct-eval-island/main.js b/tests/corpus/8425-sloppy-direct-eval-island/main.js new file mode 100644 index 000000000..c3a1cd8d6 --- /dev/null +++ b/tests/corpus/8425-sloppy-direct-eval-island/main.js @@ -0,0 +1,8 @@ +// @requires-libnode +// A sloppy direct eval sees and writes the caller's scope, and the `var` +// it declares lands in the caller's function scope. +const scopes = require("./scopes.js"); + +console.log(scopes.readsLocal(), scopes.writesLocal()); +console.log(scopes.declaresInFunction(), scopes.declaresAtTopLevel()); +console.log(scopes.seesArgument("arg"), scopes.repeated()); diff --git a/tests/corpus/8425-sloppy-direct-eval-island/package.json b/tests/corpus/8425-sloppy-direct-eval-island/package.json new file mode 100644 index 000000000..5bbefffba --- /dev/null +++ b/tests/corpus/8425-sloppy-direct-eval-island/package.json @@ -0,0 +1,3 @@ +{ + "type": "commonjs" +} diff --git a/tests/corpus/8425-sloppy-direct-eval-island/scopes.js b/tests/corpus/8425-sloppy-direct-eval-island/scopes.js new file mode 100644 index 000000000..1f41f7275 --- /dev/null +++ b/tests/corpus/8425-sloppy-direct-eval-island/scopes.js @@ -0,0 +1,42 @@ +function readsLocal() { + var local = "seen"; + return eval("local"); +} + +function writesLocal() { + var local = 41; + eval("local = local + 1"); + return local; +} + +function declaresInFunction() { + eval('var fromEval = "declared";'); + return typeof fromEval + "/" + fromEval; +} + +var globalName = "module scope"; + +function declaresAtTopLevel() { + return eval("globalName"); +} + +function seesArgument(value) { + return eval("value + '!'"); +} + +function repeated() { + var total = 0; + for (var i = 1; i <= 3; i++) { + eval("total = total + i"); + } + return total; +} + +module.exports = { + readsLocal, + writesLocal, + declaresInFunction, + declaresAtTopLevel, + seesArgument, + repeated, +}; diff --git a/tests/corpus/8426-sloppy-this-island/main.js b/tests/corpus/8426-sloppy-this-island/main.js new file mode 100644 index 000000000..4967bd23d --- /dev/null +++ b/tests/corpus/8426-sloppy-this-island/main.js @@ -0,0 +1,9 @@ +// @requires-libnode +// In a sloppy plain call `this` is the global object, never undefined: a +// user module that observes it runs in a Node island (decision S2's +// conservative marker islandizes the module, the engine decides the value). +// The functions are called through destructured bindings, so every call +// site here is plain. +const { isGlobal, plain, viaCall, viaApply, bound, arrow } = require("./receivers.js"); + +console.log(isGlobal(), plain(), viaCall(), viaApply(), bound(), arrow()); diff --git a/tests/corpus/8426-sloppy-this-island/package.json b/tests/corpus/8426-sloppy-this-island/package.json new file mode 100644 index 000000000..5bbefffba --- /dev/null +++ b/tests/corpus/8426-sloppy-this-island/package.json @@ -0,0 +1,3 @@ +{ + "type": "commonjs" +} diff --git a/tests/corpus/8426-sloppy-this-island/receivers.js b/tests/corpus/8426-sloppy-this-island/receivers.js new file mode 100644 index 000000000..4b718bafd --- /dev/null +++ b/tests/corpus/8426-sloppy-this-island/receivers.js @@ -0,0 +1,29 @@ +function isGlobal() { + return this === globalThis; +} + +function plain() { + return this === globalThis ? "global" : "other"; +} + +// An explicit receiver is that receiver, never the global object. +function viaCall() { + return isGlobal.call({ label: "receiver" }); +} + +function viaApply() { + return isGlobal.apply({ label: "receiver" }, []); +} + +// A bound undefined receiver is still the global object in sloppy mode. +function bound() { + return isGlobal.bind(undefined)(); +} + +// An arrow has no `this` of its own: it sees the sloppy function's. +function arrow() { + var see = () => this; + return see() === globalThis; +} + +module.exports = { isGlobal, plain, viaCall, viaApply, bound, arrow }; diff --git a/tests/harness/corpus-costs/8423-sloppy-with-island/main.js.json b/tests/harness/corpus-costs/8423-sloppy-with-island/main.js.json new file mode 100644 index 000000000..5adfa8964 --- /dev/null +++ b/tests/harness/corpus-costs/8423-sloppy-with-island/main.js.json @@ -0,0 +1 @@ +{ "plain": 2620, "san": 4900 } diff --git a/tests/harness/corpus-costs/8424-sloppy-annexb-block-function-island/main.js.json b/tests/harness/corpus-costs/8424-sloppy-annexb-block-function-island/main.js.json new file mode 100644 index 000000000..224d7a7b0 --- /dev/null +++ b/tests/harness/corpus-costs/8424-sloppy-annexb-block-function-island/main.js.json @@ -0,0 +1 @@ +{ "plain": 1607, "san": 1999 } diff --git a/tests/harness/corpus-costs/8425-sloppy-direct-eval-island/main.js.json b/tests/harness/corpus-costs/8425-sloppy-direct-eval-island/main.js.json new file mode 100644 index 000000000..50ce59ee0 --- /dev/null +++ b/tests/harness/corpus-costs/8425-sloppy-direct-eval-island/main.js.json @@ -0,0 +1 @@ +{ "plain": 1366, "san": 2024 } diff --git a/tests/harness/corpus-costs/8426-sloppy-this-island/main.js.json b/tests/harness/corpus-costs/8426-sloppy-this-island/main.js.json new file mode 100644 index 000000000..244011af6 --- /dev/null +++ b/tests/harness/corpus-costs/8426-sloppy-this-island/main.js.json @@ -0,0 +1 @@ +{ "plain": 1411, "san": 1828 } From 23bb419722d3dc9740ae93c6c611c4ade81c8085 Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Wed, 28 Oct 2026 14:01:31 -0700 Subject: [PATCH 05/14] Report Test262 island outcomes as their own status MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The profile gets an `--islands` lane (decision S4). It lifts exactly the five host exclusions a Node island can express — eval, Function, arguments, script-environment, globalThis — on sloppy variants only; harness limits, the assertion surface and `$262` stay excluded, because the adapter still runs inside the island. An island variant runs as a tiny native entry that imports the adapted test, so the pre-scan can island the test module. When the build reports that module in its islands, the outcome is `island-pass` or `island-fail`: Node proved the semantics, so it is never counted as a native pass and an island failure of a lifted variant is not a native regression. The report records `islands` and `liftedVariants`, `--list` marks lifted cases, and the harness exports each assertion helper so the island can rebuild `assert` in JavaScript (a native function crosses the bridge as a callable without its namespace members). `--filter` is now repeatable, so one run can select a measured set of files. Co-Authored-By: Claude Opus 5.5 --- tests/harness/test262-runner.test.ts | 54 +++++++++++++ tests/test262/compile-worker.ts | 5 +- tests/test262/execute.ts | 71 ++++++++++++++--- tests/test262/harness.ts | 10 +++ tests/test262/run.ts | 62 +++++++++++---- tests/test262/support.mjs | 110 ++++++++++++++++++++++++--- 6 files changed, 272 insertions(+), 40 deletions(-) diff --git a/tests/harness/test262-runner.test.ts b/tests/harness/test262-runner.test.ts index 3873d1c34..46a0b5fa0 100644 --- a/tests/harness/test262-runner.test.ts +++ b/tests/harness/test262-runner.test.ts @@ -6,6 +6,8 @@ import { matchesParseNegative, metadata, prepare, + prepareIsland, + scriptEnvironment, summarize, variants, } from "../test262/support.mjs"; @@ -78,6 +80,58 @@ test("unsupported execution requirements and assertion reflection remain exclusi expect(exclusion(asyncText, metadata(asyncText), "strict")).toBeUndefined(); }); +test("the island lane lifts only the five host exclusions, on sloppy variants", () => { + const globalRead = source( + "description: global", + "assert.sameValue(typeof globalThis, 'object');", + ); + expect(exclusion(globalRead, metadata(globalRead), "sloppy")).toBe("host:globalThis"); + expect(exclusion(globalRead, metadata(globalRead), "sloppy", true)).toBeUndefined(); + // Strict variants stay in the native profile: the lane adds none. + expect(exclusion(globalRead, metadata(globalRead), "strict", true)).toBe("host:globalThis"); + for (const [description, body] of [ + ["eval", "assert.sameValue(eval('1 + 1'), 2);"], + ["Function", "assert.sameValue(Function('return 1')(), 1);"], + ["arguments", "function f() { return arguments; } assert.sameValue(f(1).length, 1);"], + ["script this", "assert.sameValue(this, undefined);"], + ]) { + const text = source(`description: ${description}`, body!); + expect(exclusion(text, metadata(text), "sloppy"), body).toBeTypeOf("string"); + expect(exclusion(text, metadata(text), "sloppy", true), body).toBeUndefined(); + } + // Harness limits and `$262` stay excluded even when the file also uses a + // lifted construct, and a CommonJS-specific name is not a host reason. + const helper = source("includes: [propertyHelper.js]", "verifyWritable({}, 'x'); eval('1');"); + expect(exclusion(helper, metadata(helper), "sloppy", true)).toBe( + "harness:propertyHelper-surface", + ); + const commonJsGlobal = source( + "description: CommonJS global", + "assert.sameValue(module.exports, {});", + ); + expect(exclusion(commonJsGlobal, metadata(commonJsGlobal), "sloppy", true)).toBe("host:module"); +}); + +test("a lifted script-environment variant keeps the script shape inside its island", () => { + const script = source("description: script this", "this.value = 1;\nassert.sameValue(value, 1);"); + expect(scriptEnvironment(script)).toBe(true); + const text = prepareIsland(script, false, "MARK", [], true); + // The harness is installed on the global object, and the module only carries + // a dead copy of the body so the pre-scan still islands the module. + expect(text).toContain("globalThis.assert = assert;"); + expect(text).toContain("globalThis.Test262Error = Test262Error;"); + expect(text).toContain('const { readFileSync } = require("node:fs");'); + expect(text).toContain('const vm = require("node:vm");'); + expect(text).toContain("vm.runInThisContext(text.slice(from + begin.length, to)"); + expect(text).toContain("if (false) { (function () {"); + expect(text).toContain("this.value = 1;"); + // The ordinary island adaptation still runs the body as module code. + expect(prepareIsland(script, false, "MARK", [])).not.toContain("vm.runInThisContext"); + // `host:script-environment` also names ESM syntax, which cannot be replayed + // as a script. + expect(scriptEnvironment("export default 1;")).toBe(false); +}); + test("the property helper only admits direct inline descriptor checks", () => { for (const body of [ "verifyProperty({}, 'x', { value: 1, configurable: false });", diff --git a/tests/test262/compile-worker.ts b/tests/test262/compile-worker.ts index 79652d867..523daa6c8 100644 --- a/tests/test262/compile-worker.ts +++ b/tests/test262/compile-worker.ts @@ -10,9 +10,12 @@ async function main(): Promise { outPath: request.binary, sanitize: request.sanitize, ...(request.backend === "default" ? {} : { backend: request.backend }), + // The native lane keeps the whole program strict; the island lane lets + // the pre-scan island sloppy user modules (decision S3). + nodeIslands: { mode: request.islands ? "auto" : "off" }, }); writeFileSync(request.result, JSON.stringify(result.ok - ? { ok: true, backend: result.backend } + ? { ok: true, backend: result.backend, islands: result.islands ?? null } : { ok: false, diagnostics: result.diagnostics })); } diff --git a/tests/test262/execute.ts b/tests/test262/execute.ts index 2ae94454d..bfaf1dcd0 100644 --- a/tests/test262/execute.ts +++ b/tests/test262/execute.ts @@ -1,17 +1,20 @@ import { spawn } from "node:child_process"; import { randomBytes } from "node:crypto"; -import { mkdtempSync, readFileSync, writeFileSync } from "node:fs"; +import { mkdtempSync, readFileSync, realpathSync, writeFileSync } from "node:fs"; import { rm } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { fileURLToPath } from "node:url"; -import { assertThrowsSource, compileFailureStatus, completion, harnessSource, prepare } from "./support.mjs"; +import { assertThrowsSource, compileFailureStatus, completion, harnessSource, prepare, prepareIsland, scriptEnvironment } from "./support.mjs"; export interface Outcome { status: string; reason?: string; phase?: string; backend?: string; + /** The entry module the compiler put in a Node island (decision S4): the + * result is then `island-pass`/`island-fail`, never a native pass. */ + island?: true; stdout?: string; stderr?: string; diagnostics?: unknown[]; @@ -60,6 +63,17 @@ export function boundedRun(command: string, args: string[], timeoutMs: number): }); } +/** Whether two spellings name the same file (macOS resolves /var through + * /private/var, so the compiler may report the longer one). */ +function sameFile(a: string, b: string): boolean { + if (a === b) return true; + try { + return realpathSync(a) === realpathSync(b); + } catch { + return false; + } +} + export async function runSource(source: string, options: { backend?: "default" | "llvm"; asyncTest?: boolean; @@ -69,23 +83,48 @@ export async function runSource(source: string, options: { compileTimeoutMs?: number; runtimeTimeoutMs?: number; keep?: boolean; + /** Measure what an island can measure (decision S4): sloppy variants only, + * with the entry a tiny native module importing the adapted test, so the + * pre-scan can island it. The entry module itself always stays native. */ + islands?: boolean; + /** The exclusion the island lane lifted for this variant, when it lifted + * one. `host:script-environment` selects the script adaptation below. */ + liftedReason?: string; } = {}): Promise { // Keep sources OUTSIDE node_modules: scriptc intentionally treats imports // under that directory as package code, with different compilation rules. const workDir = mkdtempSync(join(tmpdir(), "scriptc-test262-")); - const entry = join(workDir, options.variant === "sloppy" ? "main.cjs" : "main.js"); + // The island lane keeps `main.cjs` as the adapted test module and adds a + // native entry that imports it: prepare() and the parse-negative matching + // both name the test module, and an island entry is not a program shape. + const islandLane = options.islands === true && options.variant === "sloppy"; + const entry = join(workDir, islandLane ? "entry.ts" : options.variant === "sloppy" ? "main.cjs" : "main.js"); + const testModule = islandLane ? join(workDir, "main.cjs") : entry; const binary = join(workDir, process.platform === "win32" ? "program.exe" : "program"); const resultFile = join(workDir, "compile.json"); const requestFile = join(workDir, "request.json"); let phase = "compile"; try { const marker = `${completion}:${randomBytes(16).toString("hex")}`; - writeFileSync(entry, prepare(source, options.asyncTest ?? false, marker, options.variant, options.includes)); + // A lifted script-environment variant keeps the script shape: top-level + // `this` and `var` must be global, so the island replays the body as a + // script instead of loading it as a CommonJS module. + const script = islandLane && !(options.asyncTest ?? false) && + options.liftedReason === "host:script-environment" && scriptEnvironment(source); + const text = islandLane + ? prepareIsland(source, options.asyncTest ?? false, marker, options.includes, script) + : prepare(source, options.asyncTest ?? false, marker, options.variant, options.includes); + writeFileSync(testModule, text); + // Evaluate the harness first: its shared bindings must be initialized + // before the island requires them, or the loader has to hold their + // temporal dead zone through the island's own require (a sync cycle). + if (islandLane) writeFileSync(entry, 'import "./harness.ts";\nimport "./main.cjs";\n'); writeFileSync(join(workDir, "harness.ts"), harnessSource.replace(completion, marker)); writeFileSync(join(workDir, "assert-throws.js"), assertThrowsSource); writeFileSync(requestFile, JSON.stringify({ entry, binary, workDir, result: resultFile, backend: options.backend ?? "default", sanitize: options.sanitize ?? false, + islands: islandLane, })); const compiled = await boundedRun(process.execPath, [ "--import", "tsx", fileURLToPath(new URL("./compile-worker.ts", import.meta.url)), requestFile, @@ -100,32 +139,40 @@ export async function runSource(source: string, options: { if (!build.ok) { return finish({ status: compileFailureStatus(build.diagnostics), phase, diagnostics: build.diagnostics }); } + // The build reports the modules it ran in islands, cache hits included. + // A variant whose test module is one of them reports its own outcome: + // the verdict is Node's, so it can never count as a native pass. + const island = islandLane && Array.isArray(build.islands?.modules) && + build.islands.modules.some((file) => sameFile(file, testModule)); phase = "runtime"; const result = await boundedRun(binary, [], options.runtimeTimeoutMs ?? 10_000); - const common = { backend: build.backend }; - if (result.timeout) return finish({ ...common, status: "timeout", phase }); - if (result.signal) return finish({ ...common, status: "crash", phase, reason: result.signal, stderr: result.stderr }); + const common = { backend: build.backend, ...(island ? { island: true as const } : {}) }; + const report = (outcome: Outcome): Outcome => island + ? { ...outcome, status: outcome.status === "pass" ? "island-pass" : "island-fail" } + : outcome; + if (result.timeout) return finish(report({ ...common, status: "timeout", phase })); + if (result.signal) return finish(report({ ...common, status: "crash", phase, reason: result.signal, stderr: result.stderr })); const stderr = options.sanitize ? result.stderr.replace(/^==\d+==WARNING: ASan doesn't fully support makecontext\/swapcontext.*\n/gm, "") : result.stderr; if (result.code === 86 && stderr === "SCRIPTC_TEST262_HARNESS: reference equality requires an identity-preserving adapter\n") { - return finish({ ...common, status: "harness-refusal", phase, reason: "reference-assertion", stderr }); + return finish(report({ ...common, status: "harness-refusal", phase, reason: "reference-assertion", stderr })); } if (result.code === 86 && stderr === "SCRIPTC_TEST262_HARNESS: assert.throws needs an Error instance\n") { - return finish({ ...common, status: "harness-refusal", phase, reason: "non-error-assertion", stderr }); + return finish(report({ ...common, status: "harness-refusal", phase, reason: "non-error-assertion", stderr })); } if (result.code === 0 && !result.overflow && stderr === "" && result.stdout === `${marker}\n`) { - return finish({ ...common, status: "pass" }); + return finish(report({ ...common, status: "pass" })); } // Require the entire diagnostic, so a sanitizer report following a known // refusal cannot be hidden by the regression expectation. const fence = /^Uncaught Error: [^\n]+\[(SC\d{4}) at ([^\n]+):\d+\]\n$/.exec(stderr); - return finish({ + return finish(report({ ...common, status: fence ? (fence[2]!.endsWith("/harness.ts") ? "harness-refusal" : "runtime-refusal") : "fail", phase, reason: fence?.[1] ?? `exit ${result.code}; completion marker required`, stdout: result.stdout, stderr, - }); + })); } catch (error) { return { status: "runner-error", phase, reason: String(error), ...(options.keep ? { workDir } : {}) }; } finally { diff --git a/tests/test262/harness.ts b/tests/test262/harness.ts index 40e09734a..ab47fc96f 100644 --- a/tests/test262/harness.ts +++ b/tests/test262/harness.ts @@ -119,3 +119,13 @@ export namespace assert { if (!matches) throw new Test262Error(text(message, "Expected matching array contents")); } } + +// The island lane (decision S4) assembles `assert` in JavaScript: a native +// function crosses the bridge as a callable, but the namespace members +// attached to it do not (they are properties of the native function object). +// Test262's `assert` must be callable and carry the helpers, so each helper +// is exported on its own and the island wrapper builds the object. +export const sameValue = assert.sameValue; +export const notSameValue = assert.notSameValue; +export const assertThrows = assert.throws; +export const compareArray = assert.compareArray; diff --git a/tests/test262/run.ts b/tests/test262/run.ts index 2ad02100b..74a826030 100644 --- a/tests/test262/run.ts +++ b/tests/test262/run.ts @@ -10,16 +10,17 @@ import { async function main(): Promise { const { values } = parseArgs({ options: { - root: { type: "string" }, filter: { type: "string" }, limit: { type: "string" }, + root: { type: "string" }, filter: { type: "string", multiple: true }, limit: { type: "string" }, report: { type: "string" }, journal: { type: "string" }, workers: { type: "string" }, backend: { type: "string", default: "default" }, "compile-timeout": { type: "string" }, "runtime-timeout": { type: "string" }, keep: { type: "boolean" }, list: { type: "boolean" }, help: { type: "boolean" }, + islands: { type: "boolean" }, } }); if (values.help) { console.log(`Static scriptc Test262 profile (dynamic engine disabled) pnpm test:test262 run the vendored regression profile pnpm test:test262 --root PATH survey a complete pinned snapshot - --filter TEXT select test paths containing TEXT + --filter TEXT select test paths containing TEXT (repeatable) --limit N first N selected test files --workers N concurrent compiles (default 2) --backend default|llvm LLVM is the production backend @@ -28,9 +29,16 @@ async function main(): Promise { --compile-timeout MS --runtime-timeout MS bound each compiler/program process --list list variants and exclusions without compiling --keep retain generated sources and native artifacts + --islands measure sloppy variants an island can run SCRIPTC_SAN=1 enables sanitizers; SCRIPTC_TEST_SHARD=i/n partitions variants. Adapted strict and sloppy scripts and source-matched syntax negatives can pass. -Other variants remain visible as exclusions.`); +Other variants remain visible as exclusions. + +--islands lifts exactly the five host exclusions a Node island can express +(eval, Function, arguments, script-environment, globalThis) on sloppy +variants. A variant whose test module the compiler islands reports +island-pass/island-fail: Node proved the semantics, so it is not a native +pass and an island failure of such a variant is not a native regression.`); return; } function positive(value: string | undefined, fallback: number, name: string): number { @@ -50,29 +58,44 @@ Other variants remain visible as exclusions.`); if (snapshotDigest(root) !== pin.snapshotSha256) throw new Error("Test262 snapshot differs from the pinned revision"); } else verifyVendor(); const allPaths: string[] = values.root ? testPaths(root) : pin.tests; - const selected = allPaths.filter((path) => !values.filter || path.includes(values.filter)).slice(0, limit); + const filters = values.filter ?? []; + const selected = allPaths + .filter((path) => filters.length === 0 || filters.some((filter) => path.includes(filter))) + .slice(0, limit); if (!selected.length) throw new Error("no Test262 files matched the selection"); + const islands = values.islands === true; const cases = selected.flatMap((path) => { const source = readFileSync(join(root, path), "utf8"); const meta = metadata(source, path); - return variants(meta).map((variant: string) => ({ - id: `${path}#${variant}`, path, variant, source, features: meta.features, includes: meta.includes, negative: meta.negative, - asyncTest: meta.flags.includes("async"), - exclusion: exclusion(source, meta, variant), - })); + return variants(meta).map((variant: string) => { + const exclusionReason = exclusion(source, meta, variant); + // Only the island lane adds variants; a lifted variant is newly + // measured, never a native result that could regress (decision S4). + const islandExclusion = islands ? exclusion(source, meta, variant, true) : exclusionReason; + return { + id: `${path}#${variant}`, path, variant, source, features: meta.features, includes: meta.includes, negative: meta.negative, + asyncTest: meta.flags.includes("async"), + exclusion: islandExclusion, + lifted: islandExclusion === undefined && exclusionReason !== undefined, + liftedReason: islandExclusion === undefined ? exclusionReason : undefined, + }; + }); }); const shard = shardSelect(cases, (item) => item.id); if (values.list) { - for (const item of shard) console.log(`${item.id}\t${item.exclusion ?? "run"}`); + for (const item of shard) { + console.log(`${item.id}\t${item.exclusion ?? (item.lifted ? "run (island lane)" : "run")}`); + } return; } + const liftedCases = shard.filter((item) => item.lifted).length; const cache = join(directory, "../../node_modules/.cache/scriptc-test262"); if (process.env.SCRIPTC_CACHE_DIR === undefined) { process.env.SCRIPTC_CACHE_DIR = join(cache, "cas"); mkdirSync(process.env.SCRIPTC_CACHE_DIR, { recursive: true, mode: 0o700 }); } process.env.SCRIPTC_TEST_STABLE_TOOLCHAIN ??= "1"; - const results: (Outcome & { id: string; path: string; variant: string; features: string[] })[] = []; + const results: (Outcome & { id: string; path: string; variant: string; features: string[]; lifted: boolean })[] = []; const journalPath = values.journal ? resolve(values.journal) : undefined; if (journalPath) { mkdirSync(dirname(journalPath), { recursive: true }); @@ -88,19 +111,26 @@ Other variants remain visible as exclusions.`); : await runSource(item.source, { backend, sanitize: process.env.SCRIPTC_SAN === "1", compileTimeoutMs, runtimeTimeoutMs, keep: values.keep, asyncTest: item.asyncTest, includes: item.includes, variant: item.variant === "sloppy" ? "sloppy" : "strict", + // A source-matched syntax negative stays the native compile + // refusal it has always been; it never becomes an executable. + islands: islands && item.negative === undefined, + liftedReason: item.liftedReason, }); const outcome = item.negative?.phase === "parse" && matchesParseNegative(raw, item.source, item.variant) ? { status: "pass", phase: "compile" } : raw; - const result = { id: item.id, path: item.path, variant: item.variant, features: item.features, ...outcome }; + const result = { id: item.id, path: item.path, variant: item.variant, features: item.features, lifted: item.lifted, ...outcome }; if (journalPath) appendFileSync(journalPath, `${JSON.stringify(result)}\n`); results.push(result); if (outcome.status !== "excluded") console.log(`${outcome.status}\t${item.id}`); } })); results.sort((a, b) => a.id.localeCompare(b.id)); - const unexpected = results.filter((r) => r.status !== "excluded" && - (values.root ? r.status !== "pass" : !matchesExpectation(r.id, r))); + // A lifted variant is newly measured, so its result is data rather than a + // native regression. Island passes satisfy the profile's pass expectation + // but stay a separate status in the counts (decision S4). + const unexpected = results.filter((r) => r.status !== "excluded" && !r.lifted && + (values.root ? r.status !== "pass" && r.status !== "island-pass" : !matchesExpectation(r.id, r))); const report = { schema: "scriptc.test262.v1", revision: pin.commit, snapshotSha256: pin.snapshotSha256, @@ -113,6 +143,7 @@ Other variants remain visible as exclusions.`); upstreamTestFiles: pin.testFiles, availableTestFiles: allPaths.length, selectedTestFiles: selected.length, selectedVariants: cases.length, shard: process.env.SCRIPTC_TEST_SHARD ?? null, shardVariants: shard.length, + islands, liftedVariants: liftedCases, unexpectedResults: unexpected.length, ...summarize(results), results, }; @@ -124,7 +155,8 @@ Other variants remain visible as exclusions.`); // Exclusions are visible but not failures. An all-excluded local selection // must not turn green; an empty distributed shard is legitimate. if (unexpected.length || - (!process.env.SCRIPTC_TEST_SHARD && !results.some((r) => r.status === "pass"))) process.exitCode = 1; + (!process.env.SCRIPTC_TEST_SHARD && + !results.some((r) => r.status === "pass" || r.status === "island-pass"))) process.exitCode = 1; } main().catch((error) => { diff --git a/tests/test262/support.mjs b/tests/test262/support.mjs index 46c601b7c..f4d8eb32c 100644 --- a/tests/test262/support.mjs +++ b/tests/test262/support.mjs @@ -121,10 +121,26 @@ function argumentsOwner(node) { return undefined; } +// The only exclusions the island lane lifts (decision S4): each of these +// names a construct the standalone strict adapter cannot express but Node's +// own engine can, and an island variant runs under Node. Everything else — +// the harness includes, the assertion surface, `$262` — stays excluded, +// because the adapter still has to run for an island too. +const LIFTED_IN_ISLAND = new Set([ + "host:eval", + "host:Function", + "host:arguments", + "host:script-environment", + "host:globalThis", +]); + // The first static profile adapts scripts to standalone strict modules. Be // conservative about observable global-script semantics and helper reflection. // Exclusions are runner limitations, never implementation support claims. -export function exclusion(source, meta, variant) { +// `islands` reports which variants the island lane could measure: it never +// widens the native profile, and it lifts the five host reasons on sloppy +// variants only. +export function exclusion(source, meta, variant, islands = false) { if (meta.negative) { if (meta.negative.phase !== "parse" || meta.negative.type !== "SyntaxError" || (variant !== "strict" && variant !== "sloppy")) { return `negative-phase:${meta.negative.phase}`; @@ -142,18 +158,20 @@ export function exclusion(source, meta, variant) { const forbidden = new Set(["$262", "$DONE", "$DONOTEVALUATE", "globalThis", "eval", "Function", "print", "process", "require"]); if (variant === "sloppy") for (const name of ["module", "exports", "__dirname", "__filename"]) forbidden.add(name); if (meta.flags.includes("async")) forbidden.delete("$DONE"); + const liftable = (item) => islands && variant === "sloppy" && LIFTED_IN_ISLAND.has(item); + const refuse = (item) => { if (reason === undefined && !liftable(item)) reason = item; }; const visit = (node) => { - if (reason) return; + if (reason !== undefined) return; if ((node.kind === ts.SyntaxKind.ThisKeyword && !hasOwnThisBinding(node)) || node.kind === ts.SyntaxKind.ImportKeyword || ts.isImportDeclaration(node) || ts.isExportDeclaration(node)) { - reason = "host:script-environment"; + refuse("host:script-environment"); } else if (ts.isIdentifier(node) && forbidden.has(node.text)) { - reason = `host:${node.text}`; + refuse(`host:${node.text}`); } else if (ts.isIdentifier(node) && node.text === "arguments" && (!argumentsOwner(node) || !(ts.isPropertyAccessExpression(node.parent) && node.parent.expression === node && node.parent.name.text === "length") && !(ts.isElementAccessExpression(node.parent) && node.parent.expression === node))) { - reason = "host:arguments"; + refuse("host:arguments"); } else if (ts.isIdentifier(node) && node.text === "assert") { const parent = node.parent; if (ts.isCallExpression(parent) && parent.expression === node) { @@ -167,7 +185,7 @@ export function exclusion(source, meta, variant) { ["Error", "TypeError", "RangeError", "SyntaxError", "ReferenceError", "EvalError", "URIError"].includes(parent.parent.arguments[0].text))) ) { // Supported assertion calls; aliases, mutations, and reflection stay out. - } else reason = "harness:assert-surface"; + } else refuse("harness:assert-surface"); } else if (meta.includes.includes("compareArray.js") && ts.isIdentifier(node) && node.text === "compareArray" && !(ts.isCallExpression(node.parent) && node.parent.expression === node) && !(ts.isPropertyAccessExpression(node.parent) && node.parent.name === node && ts.isIdentifier(node.parent.expression) && node.parent.expression.text === "assert")) { @@ -176,16 +194,16 @@ export function exclusion(source, meta, variant) { // function object and its format expando). reason = "harness:compareArray-surface"; } else if (ts.isIdentifier(node) && node.text === "Test262Error") { - if (!ts.isNewExpression(node.parent) || node.parent.expression !== node) reason = "harness:Test262Error-surface"; + if (!ts.isNewExpression(node.parent) || node.parent.expression !== node) refuse("harness:Test262Error-surface"); } else if (typedArrayHelper && ts.isIdentifier(node) && node.text === "TypedArray") { // The adapted testTypedArray.js leaves out %TypedArray%. - reason = "harness:TypedArray-intrinsic"; + refuse("harness:TypedArray-intrinsic"); } else if (propertyHelper && ts.isIdentifier(node)) { - if (unsupportedPropertyHelpers.has(node.text)) reason = "harness:propertyHelper-surface"; + if (unsupportedPropertyHelpers.has(node.text)) refuse("harness:propertyHelper-surface"); else if (node.text === "verifyProperty") { const call = node.parent; if (!ts.isCallExpression(call) || call.expression !== node || call.arguments.length < 3 || !inlinePropertyDescriptor(call.arguments[2])) { - reason = "harness:propertyHelper-surface"; + refuse("harness:propertyHelper-surface"); } } } @@ -207,6 +225,69 @@ export function prepare(source, asyncTest = false, marker = completion, variant return `"use strict";\nimport { ${names} } from "./harness.ts";\n${helpers}${source}\n${end}`; } +// The adapted test of the island lane (decision S4). It runs under Node, so +// `assert` is a JavaScript object built there: a native function crosses the +// bridge as a callable, but the namespace members attached to it do not (they +// are properties of the native function object). The harness exports each +// helper on its own for exactly this. +// +// The body normally runs as the island module itself. A lifted +// `host:script-environment` variant keeps its script shape instead: the +// module carries the body between these markers only as a dead copy the +// pre-scan can island, and replays it as script code inside the island. +const scriptBegin = "//@@SCRIPTC_TEST262_SCRIPT_BEGIN@@"; +const scriptEnd = "//@@SCRIPTC_TEST262_SCRIPT_END@@"; + +export function prepareIsland(source, asyncTest = false, marker = completion, includes = [], script = false) { + const helpers = + (includes.includes("propertyHelper.js") ? `${propertyHelperSource}\n` : "") + + (includes.includes("testTypedArray.js") ? `${typedArrayHelperSource}\n` : ""); + const end = asyncTest ? "" : `;console.log(${JSON.stringify(marker)});\n`; + const lines = [ + 'const harness = require("./harness.ts");', + "const assert = (value, message) => harness.assert(value, message);", + "assert.sameValue = harness.sameValue;", + "assert.notSameValue = harness.notSameValue;", + "assert.throws = harness.assertThrows;", + "assert.compareArray = harness.compareArray;", + "const Test262Error = harness.Test262Error;", + ...(asyncTest ? ["const $DONE = harness.$DONE;"] : []), + ]; + if (!script) return `${lines.join("\n")}\n${helpers}${source}\n${end}`; + // A lifted `host:script-environment` variant is a script, not a module: its + // top-level `this` is the global object and its top-level `var` bindings are + // global properties. The island therefore replays the body as script code, + // and the module carries it only as a dead copy the pre-scan can island. + const globals = ["globalThis.assert = assert;", "globalThis.Test262Error = Test262Error;"]; + return [ + 'const { readFileSync } = require("node:fs");', + 'const vm = require("node:vm");', + ...lines, + ...globals, + `const begin = ${JSON.stringify(scriptBegin)};`, + `const stop = ${JSON.stringify(scriptEnd)};`, + 'const text = readFileSync(__filename, "utf8");', + // The markers also appear in the `begin`/`stop` definitions, so the body + // is the last block that starts with the begin marker. + "const from = text.lastIndexOf(begin);", + "const to = text.indexOf(stop, from + begin.length);", + 'if (from < 0 || to < 0) throw new Error("the adapted script markers are missing");', + `vm.runInThisContext(text.slice(from + begin.length, to) + ${JSON.stringify(end)}, { filename: __filename });`, + "if (false) { (function () {", + scriptBegin, + `${helpers}${source}`, + scriptEnd, + "}); }", + ].join("\n"); +} + +/** `host:script-environment` also covers ESM syntax; only a CommonJS-shaped + * test can be replayed as a script inside its island. */ +export function scriptEnvironment(source) { + const sf = ts.createSourceFile("test.js", source, ts.ScriptTarget.Latest, true, ts.ScriptKind.JS); + return !ts.isExternalModule(sf); +} + function parseDiagnostics(source, variant) { if (variant !== "strict" && variant !== "sloppy") return []; const prepared = prepare(source, false, completion, variant); @@ -310,8 +391,13 @@ export function compileFailureStatus(diagnostics) { } export function matchesExpectation(id, outcome) { + // Decision S4: a variant whose entry module ran in a Node island passes + // for the profile — it proved Node's semantics, which is what the + // expectation asserts — but it stays a separate status, so the counts + // never report it as a native pass. + const status = outcome.status === "island-pass" ? "pass" : outcome.status; const expected = expectations[id]; - if (!expected) return outcome.status === "pass"; - return outcome.status === expected.status && outcome.reason === expected.reason && + if (!expected) return status === "pass"; + return status === expected.status && outcome.reason === expected.reason && outcome.stderr?.includes(expected.detail) === true; } From 28de3ca7c6356e087e495011a3021d5b8a700bc6 Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Wed, 28 Oct 2026 19:49:42 -0700 Subject: [PATCH 06/14] Island modules that patch a builtin module's CommonJS exports Detect writes through a builtin module's CommonJS object (direct, aliased, namespace, defineProperty, Object.assign, delete) as the new island reason `builtin-patch` (R2.5), in Node islands they are collected alongside npm and user modules, and the SC8001 note names the patched member. A patching entry module is refused with SC8006 ("move the patch into a module of its own"), as is a patcher whose TypeScript needs more than Node's type stripping. A native module that reads a patched member through the CommonJS object gets SC8020, and npm packages that read one join the island set. Program modules also ship data files they name beside themselves, since an island loads from the extracted tree. Co-Authored-By: Claude Opus 5.5 --- .../compiler/src/diagnostics/diagnostic.ts | 49 ++++ packages/compiler/src/executable/prepare.ts | 158 +++++++++++- .../compiler/src/node-islands/detect.test.ts | 84 ++++++- packages/compiler/src/node-islands/detect.ts | 236 +++++++++++++++--- .../compiler/src/node-islands/modules.test.ts | 20 ++ packages/compiler/src/node-islands/modules.ts | 28 +++ .../compiler/src/node-islands/report.test.ts | 2 + packages/compiler/src/node-islands/rules.ts | 1 + packages/compiler/src/node-islands/scan.ts | 22 +- packages/compiler/src/node-islands/summary.ts | 9 + .../compiler/src/node-islands/type-strip.ts | 181 ++++++++++++++ .../main.ts.json | 17 ++ .../fallback/payload.txt | 1 + .../6897-node-island-builtin-patch/loader.ts | 39 +++ .../6897-node-island-builtin-patch/main.ts | 19 ++ .../node_modules/payload-reader/index.js | 5 + .../node_modules/payload-reader/package.json | 1 + .../package.json | 1 + .../main.ts.json | 1 + tests/harness/node-island.test.ts | 166 ++++++++++++ 20 files changed, 995 insertions(+), 45 deletions(-) create mode 100644 packages/compiler/src/node-islands/type-strip.ts create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6897-node-island-builtin-patch/main.ts.json create mode 100644 tests/corpus/6897-node-island-builtin-patch/fallback/payload.txt create mode 100644 tests/corpus/6897-node-island-builtin-patch/loader.ts create mode 100644 tests/corpus/6897-node-island-builtin-patch/main.ts create mode 100644 tests/corpus/6897-node-island-builtin-patch/node_modules/payload-reader/index.js create mode 100644 tests/corpus/6897-node-island-builtin-patch/node_modules/payload-reader/package.json create mode 100644 tests/corpus/6897-node-island-builtin-patch/package.json create mode 100644 tests/harness/corpus-costs/6897-node-island-builtin-patch/main.ts.json diff --git a/packages/compiler/src/diagnostics/diagnostic.ts b/packages/compiler/src/diagnostics/diagnostic.ts index a1b2f071f..f03a30874 100644 --- a/packages/compiler/src/diagnostics/diagnostic.ts +++ b/packages/compiler/src/diagnostics/diagnostic.ts @@ -354,6 +354,55 @@ export function nativeSubclassOfIslandDiag(loc: SrcLoc, detail?: string): ScrDia }; } +/** SC8006 — the entry module patches a builtin module's CommonJS object. + * The entry always stays native (a whole-file island is a different + * feature), so the patch cannot run: it belongs in a module of its own, + * which then runs in a Node island. */ +export function nodeIslandEntryPatchDiag( + file: string, + member: string, + module: string, + loc: SrcLoc, +): ScrDiagnostic { + return { + code: "SC8006", + message: `the entry module patches '${member}' on the CommonJS ${module} object; move the patch into a module of its own, which then runs in a Node island`, + loc, + hint: `the entry module ${file} stays native; only a separate module runs in Node`, + }; +} + +/** SC8006 — a user TypeScript module patches a builtin module's CommonJS + * object but uses syntax Node cannot run with type stripping only. */ +export function nodeIslandStripDiag(file: string, construct: string, loc: SrcLoc): ScrDiagnostic { + return { + code: "SC8006", + message: `'${file}' patches a builtin module's CommonJS object, but Node cannot strip its types: ${construct}`, + loc, + hint: "Node runs TypeScript by erasing types: avoid enums, namespaces, constructor parameter properties, decorators, JSX, import =/export =, extensionless relative imports and value imports of types; a separate .js module can hold this patch", + }; +} + +/** SC8020 — native code reads a builtin module member that an island + * module patches: Node would show the read the patch, the native build + * cannot. ES module bindings of builtins never see the patch (in Node + * either), so the named import is the exact spelling. */ +export function nodeIslandPatchedReadDiag( + member: string, + patchFile: string, + loc: SrcLoc, +): ScrDiagnostic { + const dot = member.indexOf("."); + const module = dot < 0 ? member : member.slice(0, dot); + const property = dot < 0 ? member : member.slice(dot + 1); + return { + code: "SC8020", + message: `'${member}' is patched by ${patchFile}, which runs in a Node island; Node would show this read through the CommonJS ${module} object the patch, the native build cannot. Use \`import { ${property} } from "node:${module}"\` (ES module bindings never see the patch, in Node either), or move this code into ${patchFile}.`, + loc, + hint: `node:${module} ES module bindings keep the original function, in Node too`, + }; +} + /** SC8020 — a stateful module the program compiles is duplicated across * the Node island boundary (an explicit "duplicate", or a form NI-6 cannot * share yet): each side has its own state. */ diff --git a/packages/compiler/src/executable/prepare.ts b/packages/compiler/src/executable/prepare.ts index 2d1e30fbc..fd71975c1 100644 --- a/packages/compiler/src/executable/prepare.ts +++ b/packages/compiler/src/executable/prepare.ts @@ -4,8 +4,11 @@ import type { FfiProfile } from "../ffi/ffi-manifest.js"; import { frontendDisposed } from "../frontend/dispose-hook.js"; import { checkerPanicDiag, + nodeIslandEntryPatchDiag, nodeIslandModuleNoteDiag, nodeIslandNoteDiag, + nodeIslandPatchedReadDiag, + nodeIslandStripDiag, nodeIslandRetryFailedDiag, npmStaticRefusalDiag, iceDiag, @@ -43,6 +46,7 @@ import { describe } from "../node-islands/summary.js"; import { resolveBareModule } from "../frontend/resolve.js"; import { isEsmFile } from "../frontend/lowering/lower-node-island.js"; import type { NodeIslandScan, PackageEdge } from "../node-islands/scan.js"; +import type { NodeIslandMarker } from "../node-islands/detect.js"; import { isJsSourceFileName } from "../frontend/tsc-codes.js"; export interface PreparedExecutableModule { @@ -83,6 +87,14 @@ export function prepareExecutableModule( // unless the policy says otherwise. The frontend reloads without them, // so their declarations type the program. const islandDecision = decideNodeIslands(entryPath, opts, fe, lowerIslands, lowerModules); + // A patched builtin the native build cannot represent (an entry module + // patch, or a native read of a patched member) fails here. + if (islandDecision !== null && islandDecision.errors.length > 0) { + const diagnostics = islandDecision.errors; + const sourceTexts = fe.sourceTexts(); + fe.dispose(); + return { ok: false, diagnostics, sourceTexts }; + } if (islandDecision !== null) { fe.dispose(); fe = createFrontend(entryPath, opts.npmStatic, undefined, undefined, { @@ -362,25 +374,57 @@ const SLOPPY_REASONS: ReadonlySet = new Set([ "sloppy-this", ]); +/** A module that patches a builtin module's CommonJS object (the user's + * decision, 2026-10-29): its patch is meant for code that shares Node's + * builtin, so it runs in Node, where an island consumer sees it. */ +const PATCH_REASONS: ReadonlySet = new Set(["builtin-patch"]); + /** The program modules that run in Node islands, by file: modules marked * with `--node-island=` or `scriptc.nodeIslands.modules`, and (under * `--node-islands=auto`) user JavaScript the pre-scan found a sloppy-only * construct in (decision S3). The entry stays native: a program whose entry * is an island runs the whole file in Node, which is a different feature. */ +interface ModuleIslandDecision { + modules: Map; + /** The entry module's own `builtin-patch` marker: the entry stays native + * (a whole-file island is a different feature), so the build refuses it + * with SC8006. */ + entryPatch: { file: string; marker: NodeIslandMarker } | null; + /** User TypeScript modules with a `builtin-patch` marker that Node + * cannot run with type stripping only (H3.5): SC8006, kept native. */ + stripProblems: { file: string; construct: string; loc: SrcLoc }[]; +} + function moduleIslands( entryPath: string, policy: NodeIslandPolicy, scan: NodeIslandScan, -): Map { +): ModuleIslandDecision { const modules = new Map(); + let entryPatch: ModuleIslandDecision["entryPatch"] = null; + const stripProblems: ModuleIslandDecision["stripProblems"] = []; for (const m of scan.modules) { if (m.origin !== "user") continue; - if (!isJsSourceFileName(m.file)) continue; - if (sameFile(m.file, entryPath)) continue; + const patches = m.markers.filter((marker) => PATCH_REASONS.has(marker.reason)); + // A TypeScript module islands only for a builtin patch (H3.2). + if (!isJsSourceFileName(m.file) && patches.length === 0) continue; + if (sameFile(m.file, entryPath)) { + if (patches.length > 0 && entryPatch === null) + entryPatch = { file: m.file, marker: patches[0]! }; + continue; + } + if (m.stripProblem !== null) { + stripProblems.push({ file: m.file, ...m.stripProblem }); + continue; + } const marked = isMarkedUserModule(policy, m.file); const sloppy = m.markers.filter((marker) => SLOPPY_REASONS.has(marker.reason)); - if (marked === null && (policy.mode !== "auto" || sloppy.length === 0)) continue; - const first = sloppy[0]; + if ( + marked === null && + (policy.mode !== "auto" || (sloppy.length === 0 && patches.length === 0)) + ) + continue; + const first = sloppy[0] ?? patches[0]; const reason = first === undefined ? null : classifyMarker(first); modules.set(m.file, { file: m.file, @@ -394,7 +438,7 @@ function moduleIslands( decision: marked === null ? "auto" : "marked", }); } - return modules; + return { modules, entryPatch, stripProblems }; } /** The packages and modules a build runs in Node islands, or null for none: @@ -409,7 +453,7 @@ function decideNodeIslands( fe: ReturnType, lowerIslands: ReadonlyMap, lowerModules: ReadonlyMap, -): (NodeIslandSet & { notes: ScrDiagnostic[] }) | null { +): (NodeIslandSet & { notes: ScrDiagnostic[]; errors: ScrDiagnostic[] }) | null { const policy = nodeIslandPolicy(readProjectConfig(entryPath), opts.nodeIslands); if (policy.mode === "off" || fe.nodeIslandScan === undefined) return null; // A package marked native stays refused: the build reports it rather @@ -425,11 +469,68 @@ function decideNodeIslands( // The pre-scan's user-JavaScript markers exist only where a build may act // on them. const scan = fe.nodeIslandScan({ userJs: policy.mode === "auto" }); - const modules = moduleIslands(entryPath, policy, scan); + const moduleDecision = moduleIslands(entryPath, policy, scan); + const modules = moduleDecision.modules; // Modules the lowering refused keep their first reason (the reload no // longer compiles them). for (const [file, island] of lowerModules) if (!modules.has(file)) modules.set(file, island); - if (refused.size === 0 && policy.forcedNode.size === 0 && modules.size === 0) return null; + const errors: ScrDiagnostic[] = []; + if (moduleDecision.entryPatch !== null) { + const { file, marker } = moduleDecision.entryPatch; + errors.push( + nodeIslandEntryPatchDiag( + file, + marker.member ?? marker.construct, + builtinModuleOf(marker.member), + marker.loc, + ), + ); + } + for (const problem of moduleDecision.stripProblems) + errors.push(nodeIslandStripDiag(problem.file, problem.construct, problem.loc)); + // Who else must see the patch (H3.4): every non-islanded module that + // reads a patched member through the builtin's CommonJS object. An npm + // package joins the island set (a patch meant for it must reach it); a + // user module is SC8020, because native code cannot see the patch. + const patchedMembers = new Map(); + for (const m of scan.modules) { + if (!modules.has(m.file)) continue; + for (const marker of m.markers) { + if (!PATCH_REASONS.has(marker.reason) || marker.member === undefined) continue; + if (!patchedMembers.has(marker.member)) patchedMembers.set(marker.member, m.file); + } + } + const readerPackages = new Map< + string, + { packageDir: string; member: string; patchFile: string } + >(); + if (policy.mode === "auto" && patchedMembers.size > 0) { + for (const m of scan.modules) { + if (modules.has(m.file)) continue; + for (const read of m.cjsReads) { + const patchFile = patchFileOf(patchedMembers, read.member); + if (patchFile === undefined) continue; + if (m.origin === "npm" && m.packageName !== null && m.packageDir !== null) { + if (!readerPackages.has(m.packageName) && !refused.has(m.packageName)) + readerPackages.set(m.packageName, { + packageDir: m.packageDir, + member: read.member, + patchFile, + }); + } else if (m.origin === "user") { + errors.push(nodeIslandPatchedReadDiag(read.member, patchFile, read.loc)); + } + } + } + } + if (errors.length > 0) return { packages: new Map(), modules: new Map(), notes: [], errors }; + if ( + refused.size === 0 && + policy.forcedNode.size === 0 && + modules.size === 0 && + readerPackages.size === 0 + ) + return null; const report = buildNodeIslandReport({ entry: entryPath, scan, @@ -505,6 +606,29 @@ function decideNodeIslands( ), ); } + // npm packages that read a patched member join the island set: their + // patch lives in the island's Node, and only a shared builtin shows it. + for (const [name, reader] of readerPackages) { + if (packages.has(name)) continue; + const version = packageVersionOf(reader.packageDir); + const reason = `it reads '${reader.member}' through the CommonJS ${builtinModuleOf(reader.member)} object, which island module ${reader.patchFile} patches`; + packages.set(name, { + name, + version, + dir: reader.packageDir, + class: "object-model", + reason, + }); + notes.push( + nodeIslandNoteDiag( + name, + version, + "object-model", + reason, + fe.npmImportSites.get(name) ?? { file: entryPath, start: 0, end: 0 }, + ), + ); + } for (const island of [...modules.values()].sort((a, b) => a.file < b.file ? -1 : a.file > b.file ? 1 : 0, )) @@ -515,7 +639,21 @@ function decideNodeIslands( end: 0, }), ); - return packages.size === 0 && modules.size === 0 ? null : { packages, modules, notes }; + return packages.size === 0 && modules.size === 0 ? null : { packages, modules, notes, errors }; +} + +/** The builtin module a patched member belongs to ("fs" of + * "fs.readFileSync"). */ +function builtinModuleOf(member: string | undefined): string { + if (member === undefined) return ""; + const dot = member.indexOf("."); + return dot < 0 ? member : member.slice(0, dot); +} + +/** The island module that patches `member`, matching an exact member or a + * whole-module patch ("fs.*"). */ +function patchFileOf(patched: ReadonlyMap, member: string): string | undefined { + return patched.get(member) ?? patched.get(`${builtinModuleOf(member)}.*`); } /** The island set grown by the npm packages whose own modules refused to diff --git a/packages/compiler/src/node-islands/detect.test.ts b/packages/compiler/src/node-islands/detect.test.ts index 72cbae9c7..52b577da8 100644 --- a/packages/compiler/src/node-islands/detect.test.ts +++ b/packages/compiler/src/node-islands/detect.test.ts @@ -1,6 +1,6 @@ import { afterAll, describe, expect, test } from "vitest"; import { closeSourceParser, parseSourceFile } from "../frontend/ts7/source-parser-node.js"; -import { scanModule } from "./detect.js"; +import { scanModule, scanModuleFacts } from "./detect.js"; afterAll(() => closeSourceParser()); @@ -213,7 +213,7 @@ describe("R2 object-model markers", () => { expect(markers("Promise = Bluebird;")).toEqual(["R2.5 builtin-write"]); expect(markers("globalThis.fetch = f;")).toEqual(["R2.5 builtin-write"]); expect(markers("const fs = require('fs'); fs.close = patched;")).toEqual([ - "R2.5 builtin-write", + "R2.5 builtin-patch", ]); expect(markers("const p = new Proxy(t, h);")).toEqual(["R2.5 proxy"]); // Modeled process state and tuning knobs stay; so do redeclared names. @@ -224,6 +224,86 @@ describe("R2 object-model markers", () => { }); }); +describe("R2.5 builtin patches (H3.1)", () => { + /** The markers and the member each builtin patch names. */ + function patches(source: string, ext = "cjs"): string[] { + const sf = parseSourceFile(`/virtual/p${serial++}.${ext}`, source, "js"); + return scanModuleFacts(sf).markers.map( + (m) => `${m.reason} ${m.member === undefined ? "-" : m.member}`, + ); + } + + test("a write through the CommonJS object is a builtin patch", () => { + expect(patches("const fs = require('fs'); fs.readFileSync = f;")).toEqual([ + "builtin-patch fs.readFileSync", + ]); + }); + + test("createRequire(), casts and aliases resolve to the same object", () => { + expect( + patches( + "const r = createRequire(import.meta.url); const fs = r('node:fs') as typeof import('fs'); (fs as { readFileSync: unknown }).readFileSync = f;", + "mjs", + ), + ).toEqual(["builtin-patch fs.readFileSync"]); + expect( + patches( + "const r = createRequire(import.meta.url); const fs = r('node:fs') as typeof import('fs'); const m = fs as { readFileSync: unknown }; m.readFileSync = f;", + "mjs", + ), + ).toEqual(["builtin-patch fs.readFileSync"]); + }); + + test("a default import is the CommonJS object", () => { + expect(patches("import fs from 'node:fs'; fs.writeFileSync = f;", "mjs")).toEqual([ + "builtin-patch fs.writeFileSync", + ]); + }); + + test("defineProperty, Object.assign, delete and bind are patches too", () => { + expect( + patches("const fs = require('fs'); Object.defineProperty(fs, 'readFileSync', { value: f });"), + ).toEqual(["builtin-patch fs.readFileSync"]); + expect(patches("const fs = require('fs'); Object.assign(fs, { readFileSync: f });")).toEqual([ + "builtin-patch fs.readFileSync", + ]); + expect(patches("const fs = require('fs'); Object.assign(fs, source);")).toEqual([ + "builtin-patch fs.*", + ]); + expect(patches("const fs = require('fs'); delete fs.readFileSync;")).toEqual([ + "builtin-patch fs.readFileSync", + ]); + expect( + patches("const fs = require('fs'); fs.readFileSync = fs.readFileSync.bind(fs);"), + ).toEqual(["builtin-patch fs.readFileSync"]); + }); + + test("writes Node itself refuses are not patches", () => { + // A namespace import is not the CommonJS object: Node throws on the + // write, and the module stays as it is today. + expect(patches("import * as fs from 'node:fs'; fs.x = 1;", "mjs")).toEqual(["builtin-write -"]); + expect(patches("import { readFileSync } from 'node:fs'; readFileSync(p);", "mjs")).toEqual([]); + expect(patches("const fs = { readFileSync: 1 }; fs.readFileSync = 2;")).toEqual([]); + expect(patches("process.exitCode = 1;")).toEqual([]); + }); +}); + +test("CommonJS object reads are recorded for the patch check (H3.4)", () => { + const reads = (source: string, ext = "cjs"): string[] => { + const sf = parseSourceFile(`/virtual/r${serial++}.${ext}`, source, "js"); + return scanModuleFacts(sf).cjsReads.map((read) => read.member); + }; + expect(reads("const fs = require('fs'); fs.readFileSync(p);")).toEqual(["fs.readFileSync"]); + expect(reads("import { readFileSync } from 'node:fs'; readFileSync(p);", "mjs")).toEqual([]); + // A write is not a read; an ES module read through the namespace import + // is not the CommonJS object either. + expect(reads("const fs = require('fs'); fs.readFileSync = f;")).toEqual([]); + expect(reads("import fs from 'node:fs'; fs.readFileSync(p);", "mjs")).toEqual([ + "fs.readFileSync", + ]); + expect(reads("import * as fs from 'node:fs'; fs.readFileSync(p);", "mjs")).toEqual([]); +}); + test("markers come back in source order with their locations", () => { const source = "Object.setPrototypeOf(a, b);\neval(x);\n"; const sf = parseSourceFile(`/virtual/order${serial++}.cjs`, source, "js"); diff --git a/packages/compiler/src/node-islands/detect.ts b/packages/compiler/src/node-islands/detect.ts index e0c341bd5..926e1d6b8 100644 --- a/packages/compiler/src/node-islands/detect.ts +++ b/packages/compiler/src/node-islands/detect.ts @@ -25,6 +25,18 @@ export interface NodeIslandMarker { /** The construct, as a short phrase for messages. */ construct: string; loc: SrcLoc; + /** For `builtin-patch`: the builtin module member the module patches + * ("fs.readFileSync"; "fs.*" when the source is not an object literal). */ + member?: string; +} + +/** A member read through a builtin module's CommonJS object (H3.4): the + * patch a Node island module installs is visible to such a read, which a + * native build cannot reproduce. */ +export interface CjsRead { + /** The member read ("fs.readFileSync"). */ + member: string; + loc: SrcLoc; } /** Markers past this count in one module add nothing to a report. */ @@ -127,6 +139,23 @@ function builtinModuleName(spec: string): string | null { return null; } +/** Parentheses and the TypeScript wrappers Node erases (`as`, + * `satisfies`, `!` and the angle-bracket assertion). */ +function stripWrappers(node: ts.Node): ts.Node { + let value = node; + for (;;) { + if ( + ts.isParenthesizedExpression(value) || + ts.isAsExpression(value) || + ts.isTypeAssertion(value) || + ts.isNonNullExpression(value) || + ts.isSatisfiesExpression(value) + ) + value = value.expression; + else return value; + } +} + function literalText(node: ts.Node | undefined): string | null { if (node === undefined) return null; if (ts.isStringLiteral(node) || ts.isNoSubstitutionTemplateLiteral(node)) return node.text; @@ -147,6 +176,15 @@ function isAssignmentOperator(kind: ts.SyntaxKind): boolean { return kind >= ts.SyntaxKind.FirstAssignment && kind <= ts.SyntaxKind.LastAssignment; } +/** The key of an object-literal property (`name`, `"name"` or a + * template without substitutions). */ +function literalKeyName(name: ts.Node | undefined): string | null { + if (name === undefined) return null; + if (ts.isIdentifier(name) || ts.isStringLiteral(name) || ts.isNoSubstitutionTemplateLiteral(name)) + return name.text; + return null; +} + function propertyName(node: ts.Node): string | null { if (ts.isPropertyAccessExpression(node)) { const name = node.name; @@ -159,12 +197,12 @@ function propertyName(node: ts.Node): string | null { /** `a.b.c` as "a.b.c" (element accesses with literal keys included), or * null when the chain has a computed key or does not start at a name. */ function chainText(node: ts.Node): string | null { - if (ts.isIdentifier(node)) return node.text; - if (node.kind === ts.SyntaxKind.ThisKeyword) return "this"; - if (ts.isParenthesizedExpression(node)) return chainText(node.expression); - if (ts.isPropertyAccessExpression(node) || ts.isElementAccessExpression(node)) { - const base = chainText(node.expression); - const name = propertyName(node); + const value = stripWrappers(node); + if (ts.isIdentifier(value)) return value.text; + if (value.kind === ts.SyntaxKind.ThisKeyword) return "this"; + if (ts.isPropertyAccessExpression(value) || ts.isElementAccessExpression(value)) { + const base = chainText(value.expression); + const name = propertyName(value); if (base === null || name === null) return null; return `${base}.${name}`; } @@ -173,15 +211,16 @@ function chainText(node: ts.Node): string | null { /** Like chainText, with computed keys spelled `[…]`: `process.env[…]`. */ function memberText(node: ts.Node): string | null { - const text = chainText(node); + const value = stripWrappers(node); + const text = chainText(value); if (text !== null) return text; - if (ts.isElementAccessExpression(node)) { - const base = memberText(node.expression); + if (ts.isElementAccessExpression(value)) { + const base = memberText(value.expression); return base === null ? null : `${base}[…]`; } - if (ts.isPropertyAccessExpression(node)) { - const base = memberText(node.expression); - const name = propertyName(node); + if (ts.isPropertyAccessExpression(value)) { + const base = memberText(value.expression); + const name = propertyName(value); return base === null || name === null ? null : `${base}.${name}`; } return null; @@ -189,15 +228,11 @@ function memberText(node: ts.Node): string | null { /** The identifier a member chain starts at (`a` of `a.b[c].d`). */ function chainRoot(node: ts.Node): ts.Identifier | null { - let cur = node; + let cur = stripWrappers(node); for (;;) { if (ts.isIdentifier(cur)) return cur; if (ts.isPropertyAccessExpression(cur) || ts.isElementAccessExpression(cur)) { - cur = cur.expression; - continue; - } - if (ts.isParenthesizedExpression(cur)) { - cur = cur.expression; + cur = stripWrappers(cur.expression); continue; } return null; @@ -275,6 +310,12 @@ class ModuleScanner { private readonly builtinBindings = new Map(); /** Local names bound to a createRequire(...) result. */ private readonly createdRequires = new Set(); + /** Local names bound to a builtin module's CommonJS object: require(), + * a createRequire'd require's result, a default import, or an alias of + * one. Writes through these names are patches of the module object. */ + private readonly cjsBuiltinObjects = new Set(); + /** Members read through a CommonJS builtin object (H3.4). */ + readonly cjsReads: CjsRead[] = []; /** Names the file constructs with `new`: a constructor's `this` is never * the global object, so its `this` uses need no island (decision S5). */ private readonly constructed = new Set(); @@ -306,13 +347,20 @@ class ModuleScanner { return this.markers; } - private add(rule: NodeIslandRule, reason: string, construct: string, node: ts.Node): void { + private add( + rule: NodeIslandRule, + reason: string, + construct: string, + node: ts.Node, + member?: string, + ): void { if (this.markers.length >= MARKER_CAP) return; this.markers.push({ rule, reason, construct, loc: { file: this.sf.fileName, start: ts.tokenStartOfNode(node), end: node.end }, + ...(member === undefined ? {} : { member }), }); } @@ -321,13 +369,16 @@ class ModuleScanner { if (ts.isIdentifier(name)) { this.declared.add(name.text); if (init === undefined) return; - let value = init; - while (ts.isParenthesizedExpression(value)) value = value.expression; + const value = stripWrappers(init); if (ts.isFunctionExpression(value) || ts.isArrowFunction(value)) this.functions.add(name.text); else if (ts.isClassExpression(value)) this.classes.add(name.text); const bound = this.builtinOf(value); - if (bound !== null) this.builtinBindings.set(name.text, bound); + if (bound !== null) { + this.builtinBindings.set(name.text, bound); + if (!bound.includes(".") && this.isCjsObjectValue(value)) + this.cjsBuiltinObjects.add(name.text); + } if (ts.isCallExpression(value) && this.isCreateRequireCallee(value.expression)) this.createdRequires.add(name.text); return; @@ -353,11 +404,25 @@ class ModuleScanner { } } + /** A value that is a builtin module's CommonJS object: a require() or a + * createRequire'd require's result of a builtin, or an alias of one. */ + private isCjsObjectValue(value: ts.Node): boolean { + if (ts.isIdentifier(value)) return this.cjsBuiltinObjects.has(value.text); + if (!ts.isCallExpression(value)) return false; + return ( + this.isRequireCallee(value.expression) || + (ts.isIdentifier(value.expression) && this.createdRequires.has(value.expression.text)) + ); + } + /** `require("fs")`, `require("node:util").inherits` → "fs", "util.inherits". */ private builtinOf(node: ts.Node): string | null { - let value = node; - while (ts.isParenthesizedExpression(value)) value = value.expression; - if (ts.isCallExpression(value) && this.isRequireCallee(value.expression)) { + const value = stripWrappers(node); + if ( + ts.isCallExpression(value) && + (this.isRequireCallee(value.expression) || + (ts.isIdentifier(value.expression) && this.createdRequires.has(value.expression.text))) + ) { const spec = literalText(value.arguments.nodes[0]); return spec === null ? null : builtinModuleName(spec); } @@ -414,7 +479,12 @@ class ModuleScanner { if (clause !== undefined) { if (clause.name !== undefined) { this.declared.add(clause.name.text); - if (module !== null) this.builtinBindings.set(clause.name.text, module); + if (module !== null) { + this.builtinBindings.set(clause.name.text, module); + // A default import is the CommonJS object (unlike a namespace + // or named import, through which Node refuses writes). + this.cjsBuiltinObjects.add(clause.name.text); + } } const bindings = clause.namedBindings; if (bindings !== undefined && ts.isNamespaceImport(bindings)) { @@ -562,6 +632,7 @@ class ModuleScanner { const text = chainText(node); if ((text === "arguments.callee" || text === "arguments.caller") && !this.isStrict(node)) this.add("R2.4", "sloppy-arguments", `'${text}'`, node); + this.recordCjsRead(node); } else if (node.kind === ts.SyntaxKind.ThisKeyword) { this.visitThis(node); } else if (ts.isObjectLiteralExpression(node)) { @@ -577,6 +648,37 @@ class ModuleScanner { } } + /** Records a read of a member of a builtin module's CommonJS object, so + * a patch installed by an island module can be checked against it + * (H3.4). The left of an assignment (and a delete or ++/-- target) is a + * write, not a read. */ + private recordCjsRead(node: ts.PropertyAccessExpression): void { + const root = chainRoot(node); + if (root === null || !this.cjsBuiltinObjects.has(root.text)) return; + const module = this.builtinBindings.get(root.text); + const text = chainText(node); + if (module === undefined || text === null) return; + const parent = node.parent; + if (parent !== undefined) { + if ( + ts.isBinaryExpression(parent) && + isAssignmentOperator(parent.operatorToken.kind) && + parent.left === node + ) + return; + if ( + ts.isDeleteExpression(parent) || + ts.isPrefixUnaryExpression(parent) || + ts.isPostfixUnaryExpression(parent) + ) + return; + } + this.cjsReads.push({ + member: module + text.slice(root.text.length), + loc: { file: this.sf.fileName, start: ts.tokenStartOfNode(node), end: node.end }, + }); + } + private visitSpecifier(spec: string, node: ts.Node): void { const builtin = builtinModuleName(spec); if (builtin === "vm") this.add("R1.1", "vm", "the 'vm' module", node); @@ -691,10 +793,67 @@ class ModuleScanner { this.isGlobal("Object")) || (text === "Reflect.defineProperty" && this.isGlobal("Reflect")) ) { + // A definition on a builtin module's CommonJS object is a patch the + // code loading that object must see (R2.5, H3.1.4). + const target = first === undefined ? undefined : stripWrappers(first); + const builtin = + target !== undefined && ts.isIdentifier(target) && this.cjsBuiltinObjects.has(target.text) + ? (this.builtinBindings.get(target.text) ?? null) + : null; + if (builtin !== null) { + const key = literalText(args[1] === undefined ? undefined : stripWrappers(args[1])); + this.add( + "R2.5", + "builtin-patch", + `'${text}()' on the builtin module '${builtin}'`, + call, + key === null ? `${builtin}.*` : `${builtin}.${key}`, + ); + return; + } if (first !== undefined && !this.isModuleObject(first) && !this.inInteropHelper(call)) this.add("R2.3", "define-property", `'${text}()'`, call); return; } + // Object.assign(fs, …) on a builtin module's CommonJS object: every + // literal key is a patch (R2.5, H3.1.4). + if (text === "Object.assign" && this.isGlobal("Object") && first !== undefined) { + const target = stripWrappers(first); + if (ts.isIdentifier(target) && this.cjsBuiltinObjects.has(target.text)) { + const builtin = this.builtinBindings.get(target.text) ?? null; + if (builtin !== null) { + const source = args.length === 2 ? stripWrappers(args[1]!) : undefined; + const keys = + source !== undefined && ts.isObjectLiteralExpression(source) + ? source.properties.nodes + .map((property) => + ts.isPropertyAssignment(property) || ts.isShorthandPropertyAssignment(property) + ? literalKeyName(property.name) + : null, + ) + .filter((key): key is string => key !== null) + : []; + if (keys.length === 0) + this.add( + "R2.5", + "builtin-patch", + `'Object.assign()' on the builtin module '${builtin}'`, + call, + `${builtin}.*`, + ); + else + for (const key of keys) + this.add( + "R2.5", + "builtin-patch", + `'Object.assign()' on the builtin module '${builtin}'`, + call, + `${builtin}.${key}`, + ); + return; + } + } + } if (text.endsWith(".__defineGetter__") || text.endsWith(".__defineSetter__")) { this.add("R2.3", "define-property", `'${text.slice(text.lastIndexOf(".") + 1)}()'`, call); return; @@ -850,11 +1009,15 @@ class ModuleScanner { if (binding !== undefined) { const member = binding + spelled.slice(root.text.length); if (!this.allowedWrite(member)) { + // A write through a builtin module's CommonJS object patches that + // object; only code sharing Node's builtin can see it (H3.1). + const patch = this.cjsBuiltinObjects.has(root.text); this.add( "R2.5", - "builtin-write", + patch ? "builtin-patch" : "builtin-write", `a ${what} of the builtin module member '${member}'`, node, + patch ? member : undefined, ); } return; @@ -966,8 +1129,21 @@ interface ArgumentsUse { paramWrite: ts.Node | null; } -/** The R1/R2 markers of one JavaScript module, in source order. */ +/** The R1/R2 markers and CommonJS-object reads of one module, in source + * order. */ +export function scanModuleFacts(sf: ts.SourceFile): { + markers: NodeIslandMarker[]; + cjsReads: CjsRead[]; +} { + const scanner = new ModuleScanner(sf); + const markers = scanner.scan(); + return { + markers: markers.sort((a, b) => a.loc.start - b.loc.start), + cjsReads: scanner.cjsReads.sort((a, b) => a.loc.start - b.loc.start), + }; +} + +/** The R1/R2 markers of one module, in source order. */ export function scanModule(sf: ts.SourceFile): NodeIslandMarker[] { - const markers = new ModuleScanner(sf).scan(); - return markers.sort((a, b) => a.loc.start - b.loc.start); + return scanModuleFacts(sf).markers; } diff --git a/packages/compiler/src/node-islands/modules.test.ts b/packages/compiler/src/node-islands/modules.test.ts index 2c0d8ec73..8f443a5b5 100644 --- a/packages/compiler/src/node-islands/modules.test.ts +++ b/packages/compiler/src/node-islands/modules.test.ts @@ -249,6 +249,26 @@ describe("the collected module tree", () => { expect(NodeIslandModuleCollector.callerPath(null, join(root, "main.ts"))).toBe("main.ts"); }); + test("a data file the module names beside itself ships with it", () => { + const root = project({ + "src/main.ts": "", + "src/loader.ts": [ + 'import { join } from "node:path";', + 'const here = "/x";', + 'const fallback = join(here, "fallback", "payload.txt");', + ].join("\n"), + "src/fallback/payload.txt": "from the fallback path\n", + "src/unrelated.txt": "not named by the module\n", + "src/helper.ts": "export const h = 1;", + }); + const collector = new NodeIslandModuleCollector(); + expect(collector.add(join(root, "src/main.ts"), "./loader.ts")).toBeNull(); + const paths = collector.tree()!.files.map((file) => file.path); + expect(paths).toContain("fallback/payload.txt"); + expect(paths).not.toContain("unrelated.txt"); + expect(paths).not.toContain("helper.ts"); + }); + test("the manifest names relative paths only, and the data is checked against it", () => { const root = project({ "main.ts": "", "m.cjs": "exports.f = () => 1;" }); const collector = new NodeIslandModuleCollector(); diff --git a/packages/compiler/src/node-islands/modules.ts b/packages/compiler/src/node-islands/modules.ts index 7fc5c46d7..ead084088 100644 --- a/packages/compiler/src/node-islands/modules.ts +++ b/packages/compiler/src/node-islands/modules.ts @@ -324,6 +324,7 @@ export class NodeIslandModuleCollector { if (!TRACED.test(file)) return null; const text = trackedReadFile(file); if (text === null) return `cannot read ${file}`; + this.addNamedData(file, text); const scan = scanModuleSpecifiers(text); if (scan.computed.length > 0) { const first = scan.computed[0]!; @@ -337,6 +338,33 @@ export class NodeIslandModuleCollector { return null; } + /** Data files beside a program module that runs in an island: the module + * resolves them from its own directory (photon redirects a package's + * read to the wasm next to the patching module), and the island runs from + * the shipped tree, so a file the module names by a string literal ships + * with it. Files the source does not name stay out, and module sources + * ship through the traced walk, never as data. */ + private addNamedData(moduleFile: string, text: string): void { + const names = new Set(); + for (const token of moduleTokens(text)) + if (token.kind === "string" && !token.value.includes("${")) names.add(basename(token.value)); + if (names.size === 0) return; + const walk = (dir: string): void => { + const entries = trackedAccessibleEntries(dir); + if (entries === null) return; + for (const file of entries.files) { + const path = join(dir, file); + if (!names.has(file) || TRACED.test(file) || path === moduleFile) continue; + this.files.add(path); + } + for (const sub of entries.directories) { + if (sub === "node_modules" || sub === ".git") continue; + walk(join(dir, sub)); + } + }; + walk(dirname(moduleFile)); + } + /** A package: its whole directory, then its dependencies. */ private addPackage(found: string): void { const dir = this.placed(found); diff --git a/packages/compiler/src/node-islands/report.test.ts b/packages/compiler/src/node-islands/report.test.ts index d4221d31f..0b743ddc3 100644 --- a/packages/compiler/src/node-islands/report.test.ts +++ b/packages/compiler/src/node-islands/report.test.ts @@ -20,6 +20,8 @@ function mod(pkg: string | null, file: string, markers: NodeIslandMarker[] = []) packageDir: pkg === null ? null : `${NM}/${pkg}`, packageName: pkg, markers, + cjsReads: [], + stripProblem: null, dropped: false, }; } diff --git a/packages/compiler/src/node-islands/rules.ts b/packages/compiler/src/node-islands/rules.ts index b074ac5bc..a1967e5b5 100644 --- a/packages/compiler/src/node-islands/rules.ts +++ b/packages/compiler/src/node-islands/rules.ts @@ -73,6 +73,7 @@ const REASON_RULES: Record = { "annexb-block-function": "R2.4", "sloppy-this": "R2.4", "builtin-write": "R2.5", + "builtin-patch": "R2.5", proxy: "R2.5", }; diff --git a/packages/compiler/src/node-islands/scan.ts b/packages/compiler/src/node-islands/scan.ts index 70d7355fc..07c43b93e 100644 --- a/packages/compiler/src/node-islands/scan.ts +++ b/packages/compiler/src/node-islands/scan.ts @@ -12,7 +12,8 @@ import { resolveBareModule } from "../frontend/resolve.js"; import { isJsSourceFileName } from "../frontend/tsc-codes.js"; import { isRelativeSpecifier, packageNameOfSpecifier } from "../frontend/workspace-registry.js"; import { moduleSourceCandidates } from "../frontend/module-source-candidates.js"; -import { scanModule, type NodeIslandMarker } from "./detect.js"; +import { scanModuleFacts, type CjsRead, type NodeIslandMarker } from "./detect.js"; +import { stripProblem, type StripProblem } from "./type-strip.js"; import { originOf, packageDirOf, type Origin } from "./origin.js"; export interface ScannedModule { @@ -22,6 +23,11 @@ export interface ScannedModule { packageDir: string | null; packageName: string | null; markers: NodeIslandMarker[]; + /** Members the module reads through a builtin's CommonJS object (H3.4). */ + cjsReads: CjsRead[]; + /** A user TypeScript module with a `builtin-patch` marker that Node + * cannot run with type stripping only (H3.5): it stays native. */ + stripProblem: StripProblem | null; /** True for a module of a package the drop-and-reload loop removed. */ dropped: boolean; } @@ -150,14 +156,24 @@ export function scanFiles( if (origin === "builtin" || origin === "shim") continue; const at = origin === "npm" ? packageDirOf(sf.fileName) : null; const js = isJsSourceFileName(sf.fileName); - let markers = js && (origin === "npm" || opts.userJs) ? scanModule(sf) : []; - if (markers.length > 0 && !sloppyParsable(program, sf)) markers = []; + // A user TypeScript module is scanned only for the builtin-patch marker + // (H3.2): its other markers are JavaScript semantics, and sloppy + // parsing is never required of TypeScript. + const userTs = !js && origin === "user" && !sf.isDeclarationFile && opts.userJs; + let facts: ReturnType = { markers: [], cjsReads: [] }; + if ((js && (origin === "npm" || opts.userJs)) || userTs) facts = scanModuleFacts(sf); + let markers = userTs + ? facts.markers.filter((marker) => marker.reason === "builtin-patch") + : facts.markers; + if (js && markers.length > 0 && !sloppyParsable(program, sf)) markers = []; modules.push({ file: sf.fileName, origin, packageDir: at === null ? null : at.dir, packageName: at === null ? null : at.name, markers, + cjsReads: facts.cjsReads, + stripProblem: userTs && markers.length > 0 ? stripProblem(program, sf) : null, dropped: opts.dropped, }); if (!opts.edges) continue; diff --git a/packages/compiler/src/node-islands/summary.ts b/packages/compiler/src/node-islands/summary.ts index 8108d985c..c73399561 100644 --- a/packages/compiler/src/node-islands/summary.ts +++ b/packages/compiler/src/node-islands/summary.ts @@ -78,9 +78,18 @@ export interface NodeIslandReport { unknownPackages: string[]; } +/** A reason whose report sentence says what it means for the build, in + * addition to the pre-scan construct. */ +const REASON_SENTENCES: Readonly> = { + "builtin-patch": "it patches a builtin module's CommonJS export, which only code in Node can see", +}; + /** "R2.1 constructor functions: 'new Foo()' of a function" or * "SC2012: ". */ export function describe(r: NodeIslandReason): string { + const sentence = REASON_SENTENCES[r.reason]; + if (sentence !== undefined) + return `${r.rule === null ? "" : `${r.rule} `}${r.reason}: ${sentence} (${r.message})`; if (r.source === "pre-scan" && r.rule !== null) return `${r.rule} ${r.message}`; const rule = r.rule === null ? "" : `${r.rule} `; return r.code === null ? `${rule}${r.message}` : `${rule}${r.code}: ${r.message}`; diff --git a/packages/compiler/src/node-islands/type-strip.ts b/packages/compiler/src/node-islands/type-strip.ts new file mode 100644 index 000000000..4e8c61b45 --- /dev/null +++ b/packages/compiler/src/node-islands/type-strip.ts @@ -0,0 +1,181 @@ +/* Node runs TypeScript by erasing types in place ("strip" mode: process. + * features.typescript === "strip" in the embedded libnode). It refuses + * syntax that needs a transform (ERR_UNSUPPORTED_TYPESCRIPT_SYNTAX) and it + * resolves relative imports only with their written extension, so a module + * that needs either cannot run in a Node island as written (H3.5). The + * checks are syntactic except the one for a plain `import { T }` of a + * type, which needs the checker: Node keeps that import and fails at load. */ +import type { SrcLoc } from "../ir/ir.js"; +import * as ts from "../frontend/ts7/adapter.js"; + +export interface StripProblem { + /** The construct, as a short phrase for the SC8006 message. */ + construct: string; + loc: SrcLoc; +} + +/** Extensions Node's type-stripping loader resolves for a relative + * specifier. Node requires one for an ES module specifier, written in + * full. */ +const RESOLVED_EXTENSIONS = [".ts", ".mts", ".cts", ".js", ".mjs", ".cjs", ".json"]; + +function locOf(node: ts.Node): SrcLoc { + return { + file: ts.getSourceFileOfNode(node).fileName, + start: ts.tokenStartOfNode(node), + end: node.end, + }; +} + +function isRelativeSpecifier(spec: string): boolean { + return spec.startsWith("./") || spec.startsWith("../"); +} + +function hasResolvedExtension(spec: string): boolean { + const clean = spec.replace(/[?#].*$/, ""); + return RESOLVED_EXTENSIONS.some((extension) => clean.endsWith(extension)); +} + +function hasModifier(node: ts.Node, kind: ts.SyntaxKind): boolean { + return node.modifiers?.nodes.some((modifier) => modifier.kind === kind) ?? false; +} + +/** Whether a namespace/module declaration has runtime values (anything + * besides types and other ambient declarations). */ +function namespaceHasValues(body: ts.Node | undefined): boolean { + if (body === undefined) return false; + if (ts.isModuleDeclaration(body)) return true; + if (!ts.isModuleBlock(body)) return false; + return body.statements.nodes.some((statement) => !isTypeOnlyStatement(statement)); +} + +function isTypeOnlyStatement(statement: ts.Node): boolean { + if (ts.isInterfaceDeclaration(statement) || ts.isTypeAliasDeclaration(statement)) return true; + if (ts.isModuleDeclaration(statement)) return !namespaceHasValues(statement.body); + return false; +} + +/** The value meaning of an imported name, following alias symbols. */ +function importedValue(checker: ts.TypeChecker, element: ts.ImportSpecifier): boolean | null { + const local = element.name; + let symbol = checker.getSymbolAtLocation(local); + if (symbol === undefined) return null; + const seen = new Set(); + while ((symbol.flags & ts.SymbolFlags.Alias) !== 0 && !seen.has(symbol)) { + seen.add(symbol); + symbol = checker.getAliasedSymbol(symbol); + } + return (symbol.flags & ts.SymbolFlags.Value) !== 0; +} + +/** The first construct in `sf` that Node cannot run with type stripping + * only, or null. Only called for a user TypeScript module the pre-scan + * marked as patching a builtin, so the walk costs nothing elsewhere. */ +export function stripProblem(program: ts.Program, sf: ts.SourceFile): StripProblem | null { + let checker: ts.TypeChecker | null = null; + let found: StripProblem | null = null; + ts.walkPreorder(sf, (node) => { + if (found !== null) return "stop"; + const report = (construct: string): void => { + found = { construct, loc: locOf(node) }; + }; + if (ts.isEnumDeclaration(node)) { + if (!hasModifier(node, ts.SyntaxKind.DeclareKeyword)) report("an 'enum' declaration"); + return undefined; + } + if (ts.isModuleDeclaration(node)) { + if (!hasModifier(node, ts.SyntaxKind.DeclareKeyword) && namespaceHasValues(node.body)) + report("a 'namespace' declaration with values"); + return undefined; + } + if (ts.isConstructorDeclaration(node)) { + for (const parameter of node.parameters.nodes) { + if ( + parameter.modifiers?.nodes.some( + (modifier) => + modifier.kind === ts.SyntaxKind.PublicKeyword || + modifier.kind === ts.SyntaxKind.PrivateKeyword || + modifier.kind === ts.SyntaxKind.ProtectedKeyword || + modifier.kind === ts.SyntaxKind.ReadonlyKeyword, + ) === true + ) { + report("a constructor parameter property"); + return undefined; + } + } + return undefined; + } + if (ts.isImportEqualsDeclaration(node)) { + report("an 'import = require()' declaration"); + return undefined; + } + if (ts.isExportAssignment(node) && node.isExportEquals === true) { + report("an 'export =' assignment"); + return undefined; + } + if ( + node.modifiers?.nodes.some((modifier) => modifier.kind === ts.SyntaxKind.Decorator) === true + ) { + report("a decorator"); + return undefined; + } + if (ts.isJsxElement(node) || ts.isJsxSelfClosingElement(node) || ts.isJsxFragment(node)) { + report("JSX"); + return undefined; + } + if (ts.isImportDeclaration(node)) { + const spec = ts.isStringLiteral(node.moduleSpecifier) ? node.moduleSpecifier.text : null; + if (spec !== null && isRelativeSpecifier(spec) && !hasResolvedExtension(spec)) { + report(`the relative import '${spec}' without a file extension`); + return undefined; + } + const clause = node.importClause; + if ( + clause === undefined || + clause.phaseModifier === ts.SyntaxKind.TypeKeyword || + clause.namedBindings === undefined || + !ts.isNamedImports(clause.namedBindings) + ) + return undefined; + checker ??= program.getTypeChecker(); + for (const element of clause.namedBindings.elements.nodes) { + if (element.isTypeOnly) continue; + if (importedValue(checker, element) === false) { + found = { + construct: `the import of '${element.name.text}', which is only a type (use \`import type\`)`, + loc: locOf(element), + }; + return "stop"; + } + } + return undefined; + } + if (ts.isExportDeclaration(node)) { + const specifier = node.moduleSpecifier; + if ( + specifier !== undefined && + ts.isStringLiteral(specifier) && + isRelativeSpecifier(specifier.text) && + !hasResolvedExtension(specifier.text) + ) + report(`the relative export from '${specifier.text}' without a file extension`); + return undefined; + } + if ( + ts.isCallExpression(node) && + node.expression.kind === ts.SyntaxKind.ImportKeyword && + node.arguments.nodes.length > 0 + ) { + const argument = node.arguments.nodes[0]; + if ( + argument !== undefined && + ts.isStringLiteral(argument) && + isRelativeSpecifier(argument.text) && + !hasResolvedExtension(argument.text) + ) + report(`the relative import() of '${argument.text}' without a file extension`); + } + return undefined; + }); + return found; +} diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6897-node-island-builtin-patch/main.ts.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6897-node-island-builtin-patch/main.ts.json new file mode 100644 index 000000000..12765200f --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6897-node-island-builtin-patch/main.ts.json @@ -0,0 +1,17 @@ +{ + "order": [ + "/tests/corpus/6897-node-island-builtin-patch/loader.ts", + "/tests/corpus/6897-node-island-builtin-patch/main.ts" + ], + "diags": [ + { + "code": "SC0001", + "message": "Could not find a declaration file for module 'payload-reader'. '/tests/corpus/6897-node-island-builtin-patch/node_modules/payload-reader/index.js' implicitly has an 'any' type.\n Try `npm i --save-dev @types/payload-reader` if it exists or add a new declaration (.d.ts) file containing `declare module 'payload-reader';`", + "loc": { + "file": "/tests/corpus/6897-node-island-builtin-patch/loader.ts", + "start": 1175, + "end": 1191 + } + } + ] +} diff --git a/tests/corpus/6897-node-island-builtin-patch/fallback/payload.txt b/tests/corpus/6897-node-island-builtin-patch/fallback/payload.txt new file mode 100644 index 000000000..057938d12 --- /dev/null +++ b/tests/corpus/6897-node-island-builtin-patch/fallback/payload.txt @@ -0,0 +1 @@ +from the fallback path diff --git a/tests/corpus/6897-node-island-builtin-patch/loader.ts b/tests/corpus/6897-node-island-builtin-patch/loader.ts new file mode 100644 index 000000000..56ad94ea3 --- /dev/null +++ b/tests/corpus/6897-node-island-builtin-patch/loader.ts @@ -0,0 +1,39 @@ +// Patches the CommonJS fs object while a package that reads its data file +// through require("fs") loads, then restores it. +import { createRequire } from "node:module"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; + +const require = createRequire(import.meta.url); +const fs = require("fs") as typeof import("fs"); +type ReadFileSync = typeof fs.readFileSync; +const here = dirname(fileURLToPath(import.meta.url)); + +export interface LoadResult { + text: string; + patchedCalls: number; + restored: boolean; +} + +export async function loadPayload(): Promise { + const original: ReadFileSync = fs.readFileSync.bind(fs); + const mutableFs = fs as { readFileSync: ReadFileSync }; + let patchedCalls = 0; + const patched = ((...args: Parameters) => { + patchedCalls++; + const [file, options] = args; + if (typeof file === "string" && file.endsWith("payload.txt") && !fs.existsSync(file)) { + return original(join(here, "fallback", "payload.txt"), options); + } + return original(...args); + }) as ReadFileSync; + mutableFs.readFileSync = patched; + let text: string; + try { + const mod = (await import("payload-reader")) as { default: { payload: string } }; + text = mod.default.payload.trim(); + } finally { + mutableFs.readFileSync = original; + } + return { text, patchedCalls, restored: fs.readFileSync === original }; +} diff --git a/tests/corpus/6897-node-island-builtin-patch/main.ts b/tests/corpus/6897-node-island-builtin-patch/main.ts new file mode 100644 index 000000000..861ddd786 --- /dev/null +++ b/tests/corpus/6897-node-island-builtin-patch/main.ts @@ -0,0 +1,19 @@ +// @requires-libnode +// A module that patches a builtin module's CommonJS export (the photon +// idiom): loader.ts replaces fs.readFileSync while a package loads, so the +// package's own require("fs").readFileSync finds a payload that is not at +// its default path. Because it patches a builtin, loader.ts runs in a Node +// island automatically and shares Node's one fs with the package; the +// entry stays native. Its named import of readFileSync is not patched: +// Node's ES module bindings of builtins do not follow CommonJS patches. +import { readFileSync } from "node:fs"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; +import { loadPayload } from "./loader.ts"; + +const here = dirname(fileURLToPath(import.meta.url)); +const result = await loadPayload(); +console.log("package read:", result.text); +console.log("calls through the patch:", result.patchedCalls); +console.log("restored:", result.restored); +console.log("native read:", readFileSync(join(here, "fallback", "payload.txt"), "utf8").trim()); diff --git a/tests/corpus/6897-node-island-builtin-patch/node_modules/payload-reader/index.js b/tests/corpus/6897-node-island-builtin-patch/node_modules/payload-reader/index.js new file mode 100644 index 000000000..cd25d1010 --- /dev/null +++ b/tests/corpus/6897-node-island-builtin-patch/node_modules/payload-reader/index.js @@ -0,0 +1,5 @@ +// Reads its data file next to itself through the CommonJS fs object, the +// way wasm-bindgen packages read their .wasm file. +const fs = require("fs"); +const path = require("path"); +exports.payload = fs.readFileSync(path.join(__dirname, "payload.txt"), "utf8"); diff --git a/tests/corpus/6897-node-island-builtin-patch/node_modules/payload-reader/package.json b/tests/corpus/6897-node-island-builtin-patch/node_modules/payload-reader/package.json new file mode 100644 index 000000000..78905724c --- /dev/null +++ b/tests/corpus/6897-node-island-builtin-patch/node_modules/payload-reader/package.json @@ -0,0 +1 @@ +{ "name": "payload-reader", "version": "1.0.0", "main": "index.js" } diff --git a/tests/corpus/6897-node-island-builtin-patch/package.json b/tests/corpus/6897-node-island-builtin-patch/package.json new file mode 100644 index 000000000..ebbfc464b --- /dev/null +++ b/tests/corpus/6897-node-island-builtin-patch/package.json @@ -0,0 +1 @@ +{ "name": "builtin-patch-island", "private": true, "type": "module" } diff --git a/tests/harness/corpus-costs/6897-node-island-builtin-patch/main.ts.json b/tests/harness/corpus-costs/6897-node-island-builtin-patch/main.ts.json new file mode 100644 index 000000000..6fb6039dd --- /dev/null +++ b/tests/harness/corpus-costs/6897-node-island-builtin-patch/main.ts.json @@ -0,0 +1 @@ +{ "plain": 2500 } diff --git a/tests/harness/node-island.test.ts b/tests/harness/node-island.test.ts index 215d75384..326f6530a 100644 --- a/tests/harness/node-island.test.ts +++ b/tests/harness/node-island.test.ts @@ -12,6 +12,7 @@ import { cpSync, existsSync, mkdirSync, + mkdtempSync, readFileSync, readdirSync, realpathSync, @@ -19,6 +20,7 @@ import { statSync, writeFileSync, } from "node:fs"; +import { tmpdir } from "node:os"; import { basename, dirname, join } from "node:path"; import { promisify } from "node:util"; import { beforeAll, describe, expect, test } from "vitest"; @@ -336,6 +338,170 @@ describe.skipIf(skip)("Node islands in-process", () => { ); expect(conflicting).toEqual([]); }); + + // A module that patches a builtin module's CommonJS object runs in a Node + // island automatically (H3): the patch is meant for code that shares + // Node's builtin, and only Node has that object. + describe("a module that patches a builtin module", () => { + const corpus = join(repoRoot, "tests/corpus/6897-node-island-builtin-patch"); + + /** A scratch copy of the corpus, with main.ts optionally replaced. It + * lives outside node_modules: a package copied below one is a library + * with no declaration file, and its implicit `any` fails the build. */ + function copyCorpus(name: string, main?: string, extra: Record = {}): string { + const dir = mkdtempSync(join(tmpdir(), `scriptc-builtin-patch-${name}-`)); + cpSync(corpus, dir, { recursive: true }); + if (main !== undefined) writeFileSync(join(dir, "main.ts"), main); + for (const [file, text] of Object.entries(extra)) writeFileSync(join(dir, file), text); + return dir; + } + + /** A build with a cache of its own: a hit reports the islands but not + * the SC8001 notes, and these tests read the notes. */ + async function buildDir(dir: string, name: string) { + return withFreshBuildCache(() => + compile(join(dir, "main.ts"), { + outPath: join(dir, name), + outDir: dir, + sanitize, + libnode: pack!, + }), + ); + } + + async function withFreshBuildCache(build: () => Promise): Promise { + const cache = mkdtempSync(join(tmpdir(), "scriptc-builtin-patch-cache-")); + const previous = process.env["SCRIPTC_CACHE_DIR"]; + process.env["SCRIPTC_CACHE_DIR"] = cache; + try { + return await build(); + } finally { + if (previous === undefined) delete process.env["SCRIPTC_CACHE_DIR"]; + else process.env["SCRIPTC_CACHE_DIR"] = previous; + rmSync(cache, { recursive: true, force: true }); + } + } + + function messages(result: { diagnostics?: { code: string; message: string }[] }): string[] { + return (result.diagnostics ?? []).map((d) => `${d.code}: ${d.message}`); + } + + test("islands only the patching module, with the builtin-patch reason", async () => { + const dir = copyCorpus("auto"); + const result = await buildDir(dir, "main"); + expect(messages(result)).toEqual([]); + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.islands?.modules.map((file) => basename(file))).toEqual(["loader.ts"]); + const notes = (result.warnings ?? []).filter( + (d) => d.code === "SC8001" && d.message.includes("loader.ts"), + ); + expect(notes).toHaveLength(1); + expect(notes[0]!.message).toContain("builtin-patch"); + expect(notes[0]!.message).toContain("fs.readFileSync"); + expect(notes[0]!.message).toContain("Node island"); + }, 300_000); + + test("a native read of the patched member is SC8020", async () => { + const dir = copyCorpus( + "read", + `import fs from "node:fs"; +import { loadPayload } from "./loader.ts"; +const result = await loadPayload(); +console.log(result.text, fs.readFileSync("payload.txt", "utf8")); +`, + ); + const result = await buildDir(dir, "main"); + expect(result.ok).toBe(false); + const list = messages(result); + const read = list.find((m) => m.startsWith("SC8020:")); + expect(read).toBeDefined(); + expect(read).toContain("fs.readFileSync"); + expect(read).toContain("loader.ts"); + }, 300_000); + + test("--node-islands=off makes the patch a native refusal again", async () => { + const dir = copyCorpus( + "off", + 'import { patch } from "./loader.ts";\npatch();\nconsole.log("ok");\n', + ); + // The package the corpus loads has no declaration file, and without + // an island its implicit `any` would fail before the patch refusal. + writeFileSync( + join(dir, "loader.ts"), + `import { createRequire } from "node:module"; +const require = createRequire(import.meta.url); +const fs = require("fs") as typeof import("fs"); +const original = fs.readFileSync.bind(fs); +export function patch(): void { + fs.readFileSync = original as typeof fs.readFileSync; +} +`, + ); + const result = await withFreshBuildCache(() => + compile(join(dir, "main.ts"), { + outPath: join(dir, "main"), + outDir: dir, + sanitize, + libnode: pack!, + nodeIslands: { mode: "off" }, + }), + ); + expect(result.ok).toBe(false); + if (result.ok) return; + expect( + result.diagnostics.some( + (d) => /^SC(1090|2020)$/.test(d.code) && d.loc.file.endsWith("loader.ts"), + ), + ).toBe(true); + }, 300_000); + + test("an entry module that patches is SC8006", async () => { + const dir = copyCorpus( + "entry", + `import { createRequire } from "node:module"; +const require = createRequire(import.meta.url); +const fs = require("fs") as typeof import("fs"); +fs.readFileSync = (() => "") as typeof fs.readFileSync; +console.log(fs.existsSync("x")); +`, + ); + const result = await buildDir(dir, "main"); + expect(result.ok).toBe(false); + const entry = messages(result).find( + (m) => m.startsWith("SC8006:") && m.includes("move the patch into a module of its own"), + ); + expect(entry).toBeDefined(); + expect(entry).toContain("fs.readFileSync"); + }); + + test("a TypeScript patcher Node cannot strip is SC8006", async () => { + const dir = copyCorpus( + "enum", + `import { patched } from "./enumpatch.ts"; +console.log(patched); +`, + { + "enumpatch.ts": `import { createRequire } from "node:module"; +enum Kind { + A, +} +const require = createRequire(import.meta.url); +const fs = require("fs") as typeof import("fs"); +fs.readFileSync = (() => "") as typeof fs.readFileSync; +export const patched = Kind.A; +`, + }, + ); + const result = await buildDir(dir, "main"); + expect(result.ok).toBe(false); + const stripped = messages(result).find( + (m) => m.startsWith("SC8006:") && m.includes("Node cannot strip its types"), + ); + expect(stripped).toBeDefined(); + expect(stripped).toContain("enum"); + }); + }); }); // Lowering checks every argument and result type for JSON data; IR output From ef3745ce07edea937c5c0930e887882e6c1de481 Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Thu, 29 Oct 2026 18:11:42 -0700 Subject: [PATCH 07/14] Number the sloppy island corpus program 8427 `8423-accessor-member-kind-mismatch.ts` already owns 8423 on main. Co-Authored-By: Claude Opus 5.5 --- .../main.js.json | 26 +++++++++---------- .../lookup.js | 0 .../main.js | 0 .../package.json | 0 .../main.js.json | 0 5 files changed, 13 insertions(+), 13 deletions(-) rename packages/compiler/test/ts7/baselines/order-parity/tests/corpus/{8423-sloppy-with-island => 8427-sloppy-with-island}/main.js.json (68%) rename tests/corpus/{8423-sloppy-with-island => 8427-sloppy-with-island}/lookup.js (100%) rename tests/corpus/{8423-sloppy-with-island => 8427-sloppy-with-island}/main.js (100%) rename tests/corpus/{8423-sloppy-with-island => 8427-sloppy-with-island}/package.json (100%) rename tests/harness/corpus-costs/{8423-sloppy-with-island => 8427-sloppy-with-island}/main.js.json (100%) diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/8423-sloppy-with-island/main.js.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/8427-sloppy-with-island/main.js.json similarity index 68% rename from packages/compiler/test/ts7/baselines/order-parity/tests/corpus/8423-sloppy-with-island/main.js.json rename to packages/compiler/test/ts7/baselines/order-parity/tests/corpus/8427-sloppy-with-island/main.js.json index c6b9ad9a0..cb38c0ce6 100644 --- a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/8423-sloppy-with-island/main.js.json +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/8427-sloppy-with-island/main.js.json @@ -1,14 +1,14 @@ { "order": [ - "/tests/corpus/8423-sloppy-with-island/lookup.js", - "/tests/corpus/8423-sloppy-with-island/main.js" + "/tests/corpus/8427-sloppy-with-island/lookup.js", + "/tests/corpus/8427-sloppy-with-island/main.js" ], "diags": [ { "code": "SC0001", "message": "'with' statements are not allowed in strict mode.", "loc": { - "file": "/tests/corpus/8423-sloppy-with-island/lookup.js", + "file": "/tests/corpus/8427-sloppy-with-island/lookup.js", "start": 194, "end": 198 } @@ -17,7 +17,7 @@ "code": "SC0001", "message": "The 'with' statement is not supported. All symbols in a 'with' block will have type 'any'.", "loc": { - "file": "/tests/corpus/8423-sloppy-with-island/lookup.js", + "file": "/tests/corpus/8427-sloppy-with-island/lookup.js", "start": 194, "end": 207 } @@ -26,7 +26,7 @@ "code": "SC0001", "message": "Cannot find name 'last'.", "loc": { - "file": "/tests/corpus/8423-sloppy-with-island/lookup.js", + "file": "/tests/corpus/8427-sloppy-with-island/lookup.js", "start": 252, "end": 256 } @@ -35,7 +35,7 @@ "code": "SC0001", "message": "'with' statements are not allowed in strict mode.", "loc": { - "file": "/tests/corpus/8423-sloppy-with-island/lookup.js", + "file": "/tests/corpus/8427-sloppy-with-island/lookup.js", "start": 295, "end": 299 } @@ -44,7 +44,7 @@ "code": "SC0001", "message": "The 'with' statement is not supported. All symbols in a 'with' block will have type 'any'.", "loc": { - "file": "/tests/corpus/8423-sloppy-with-island/lookup.js", + "file": "/tests/corpus/8427-sloppy-with-island/lookup.js", "start": 295, "end": 308 } @@ -53,7 +53,7 @@ "code": "SC0001", "message": "'with' statements are not allowed in strict mode.", "loc": { - "file": "/tests/corpus/8423-sloppy-with-island/lookup.js", + "file": "/tests/corpus/8427-sloppy-with-island/lookup.js", "start": 422, "end": 426 } @@ -62,7 +62,7 @@ "code": "SC0001", "message": "The 'with' statement is not supported. All symbols in a 'with' block will have type 'any'.", "loc": { - "file": "/tests/corpus/8423-sloppy-with-island/lookup.js", + "file": "/tests/corpus/8427-sloppy-with-island/lookup.js", "start": 422, "end": 447 } @@ -71,7 +71,7 @@ "code": "SC0001", "message": "'with' statements are not allowed in strict mode.", "loc": { - "file": "/tests/corpus/8423-sloppy-with-island/lookup.js", + "file": "/tests/corpus/8427-sloppy-with-island/lookup.js", "start": 528, "end": 532 } @@ -80,7 +80,7 @@ "code": "SC0001", "message": "The 'with' statement is not supported. All symbols in a 'with' block will have type 'any'.", "loc": { - "file": "/tests/corpus/8423-sloppy-with-island/lookup.js", + "file": "/tests/corpus/8427-sloppy-with-island/lookup.js", "start": 528, "end": 541 } @@ -89,7 +89,7 @@ "code": "SC0001", "message": "'with' statements are not allowed in strict mode.", "loc": { - "file": "/tests/corpus/8423-sloppy-with-island/lookup.js", + "file": "/tests/corpus/8427-sloppy-with-island/lookup.js", "start": 548, "end": 552 } @@ -98,7 +98,7 @@ "code": "SC0001", "message": "Cannot find name 'inner'.", "loc": { - "file": "/tests/corpus/8423-sloppy-with-island/lookup.js", + "file": "/tests/corpus/8427-sloppy-with-island/lookup.js", "start": 576, "end": 581 } diff --git a/tests/corpus/8423-sloppy-with-island/lookup.js b/tests/corpus/8427-sloppy-with-island/lookup.js similarity index 100% rename from tests/corpus/8423-sloppy-with-island/lookup.js rename to tests/corpus/8427-sloppy-with-island/lookup.js diff --git a/tests/corpus/8423-sloppy-with-island/main.js b/tests/corpus/8427-sloppy-with-island/main.js similarity index 100% rename from tests/corpus/8423-sloppy-with-island/main.js rename to tests/corpus/8427-sloppy-with-island/main.js diff --git a/tests/corpus/8423-sloppy-with-island/package.json b/tests/corpus/8427-sloppy-with-island/package.json similarity index 100% rename from tests/corpus/8423-sloppy-with-island/package.json rename to tests/corpus/8427-sloppy-with-island/package.json diff --git a/tests/harness/corpus-costs/8423-sloppy-with-island/main.js.json b/tests/harness/corpus-costs/8427-sloppy-with-island/main.js.json similarity index 100% rename from tests/harness/corpus-costs/8423-sloppy-with-island/main.js.json rename to tests/harness/corpus-costs/8427-sloppy-with-island/main.js.json From 306db2f3b1102a6af2516f0b4c4f0bf512f2f95c Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Thu, 29 Oct 2026 21:27:18 -0500 Subject: [PATCH 08/14] Address review comments on builtin-patch islands and the Test262 lane - Record CommonJS builtin reads through bracket notation and destructuring, so a native consumer of a patched member is islanded or reported with SC8020. A computed key or rest element reads the whole module and matches any patch of it. - Keep the named-data walk of an island module inside its directory: a symbolic link that leads out of it is not followed, and a link loop ends the walk. - Skip the extension check for import type and export type, which Node erases without resolving the specifier. - Scan TypeScript island modules for the program modules they import, so a stateful module used by both sides is shared instead of copied. - Export the Test262 assertion form of compareArray as assertCompareArray, so the harness no longer declares compareArray twice, and give island tests the bare compareArray as native ones have. --- packages/compiler/src/executable/prepare.ts | 18 +++- .../frontend/lowering/lower-shared-modules.ts | 13 ++- .../compiler/src/node-islands/detect.test.ts | 20 ++++ packages/compiler/src/node-islands/detect.ts | 92 +++++++++++++++++-- .../compiler/src/node-islands/modules.test.ts | 19 ++++ packages/compiler/src/node-islands/modules.ts | 18 +++- .../src/node-islands/type-strip.test.ts | 43 +++++++++ .../compiler/src/node-islands/type-strip.ts | 7 +- tests/harness/node-island.test.ts | 53 +++++++++++ tests/harness/test262-runner.test.ts | 37 ++++++++ tests/test262/harness.ts | 6 +- tests/test262/support.mjs | 10 +- 12 files changed, 315 insertions(+), 21 deletions(-) create mode 100644 packages/compiler/src/node-islands/type-strip.test.ts diff --git a/packages/compiler/src/executable/prepare.ts b/packages/compiler/src/executable/prepare.ts index fd71975c1..a00ffacea 100644 --- a/packages/compiler/src/executable/prepare.ts +++ b/packages/compiler/src/executable/prepare.ts @@ -646,14 +646,24 @@ function decideNodeIslands( * "fs.readFileSync"). */ function builtinModuleOf(member: string | undefined): string { if (member === undefined) return ""; - const dot = member.indexOf("."); - return dot < 0 ? member : member.slice(0, dot); + const end = /[.[]/.exec(member); + return end === null ? member : member.slice(0, end.index); } /** The island module that patches `member`, matching an exact member or a - * whole-module patch ("fs.*"). */ + * whole-module patch ("fs.*"). A read the scan cannot name ("fs.*", a + * computed key or a rest element) matches every patch of its module, and so + * does a patch through a computed key ("fs[…]"). */ function patchFileOf(patched: ReadonlyMap, member: string): string | undefined { - return patched.get(member) ?? patched.get(`${builtinModuleOf(member)}.*`); + const module = builtinModuleOf(member); + const exact = patched.get(member) ?? patched.get(`${module}.*`); + if (exact !== undefined) return exact; + const unnamedRead = member === `${module}.*`; + for (const [patch, file] of patched) { + if (builtinModuleOf(patch) !== module) continue; + if (unnamedRead || patch.includes("[")) return file; + } + return undefined; } /** The island set grown by the npm packages whose own modules refused to diff --git a/packages/compiler/src/frontend/lowering/lower-shared-modules.ts b/packages/compiler/src/frontend/lowering/lower-shared-modules.ts index ecae17894..94843bcb7 100644 --- a/packages/compiler/src/frontend/lowering/lower-shared-modules.ts +++ b/packages/compiler/src/frontend/lowering/lower-shared-modules.ts @@ -417,15 +417,22 @@ export function sharedModulePlans(lowerer: Lowerer): SharedModulePlan[] { packageJsFiles(island.dir, queue); // A program module the build runs in a Node island is loaded by Node, so // the program modules IT imports are shared with it like an island - // package's (DESIGN.md note 1 §3.2). - for (const module of lowerer.nodeIslands?.modules.values() ?? []) queue.push(real(module.file)); + // package's (DESIGN.md note 1 §3.2). It may be TypeScript (Node strips + // its types), so it is scanned whatever its extension. + const islandModules = new Set(); + for (const module of lowerer.nodeIslands?.modules.values() ?? []) { + const file = real(module.file); + islandModules.add(file); + queue.push(file); + } if (queue.length === 0 && candidates.size === 0) return plans; const seen = new Set(); // Classes island code extends, by the compiled module that exports them. const extended = new Map(); while (queue.length > 0 && seen.size < MAX_ISLAND_FILES) { const file = queue.pop()!; - if (seen.has(file) || compiled.has(file) || !isJsFile(file)) continue; + if (seen.has(file) || compiled.has(file)) continue; + if (!isJsFile(file) && !islandModules.has(file)) continue; seen.add(file); let text: string; try { diff --git a/packages/compiler/src/node-islands/detect.test.ts b/packages/compiler/src/node-islands/detect.test.ts index 52b577da8..9723530d0 100644 --- a/packages/compiler/src/node-islands/detect.test.ts +++ b/packages/compiler/src/node-islands/detect.test.ts @@ -302,6 +302,26 @@ test("CommonJS object reads are recorded for the patch check (H3.4)", () => { "fs.readFileSync", ]); expect(reads("import * as fs from 'node:fs'; fs.readFileSync(p);", "mjs")).toEqual([]); + // Bracket notation: a literal key names the member, a computed one reads + // a member the scan cannot name. + expect(reads("const fs = require('fs'); fs['readFileSync'](p);")).toEqual(["fs.readFileSync"]); + expect(reads("const fs = require('fs'); fs[name](p);")).toEqual(["fs.*"]); + expect(reads("const fs = require('fs'); fs['readFileSync'] = f;")).toEqual([]); + // Destructuring reads each property it takes when it runs. + expect(reads("const { readFileSync } = require('node:fs');")).toEqual(["fs.readFileSync"]); + expect(reads("const fs = require('fs'); const { readFileSync: read, ...rest } = fs;")).toEqual([ + "fs.readFileSync", + "fs.*", + ]); + expect(reads("const { promises: { readFile } } = require('fs');")).toEqual([ + "fs.promises", + "fs.promises.readFile", + ]); + expect( + reads("import fs from 'node:fs'; let readFileSync; ({ readFileSync } = fs);", "mjs"), + ).toEqual(["fs.readFileSync"]); + // A namespace import is not the CommonJS object. + expect(reads("import * as fs from 'node:fs'; const { readFileSync } = fs;", "mjs")).toEqual([]); }); test("markers come back in source order with their locations", () => { diff --git a/packages/compiler/src/node-islands/detect.ts b/packages/compiler/src/node-islands/detect.ts index 926e1d6b8..6931af42b 100644 --- a/packages/compiler/src/node-islands/detect.ts +++ b/packages/compiler/src/node-islands/detect.ts @@ -162,6 +162,14 @@ function literalText(node: ts.Node | undefined): string | null { return null; } +/** A property name the scan can spell: an identifier, a string or number + * literal, or a computed literal key. */ +function staticKey(node: ts.Node): string | null { + if (ts.isIdentifier(node) || ts.isNumericLiteral(node)) return node.text; + if (ts.isComputedPropertyName(node)) return literalText(stripWrappers(node.expression)); + return literalText(node); +} + /** A specifier that folds at build time: a literal, or a concatenation of * literals. */ function isFoldableSpecifier(node: ts.Node): boolean { @@ -384,6 +392,7 @@ class ModuleScanner { return; } if (ts.isObjectBindingPattern(name)) { + this.recordCjsPatternReads(name, init); const base = init === undefined ? null : this.builtinOf(init); for (const element of name.elements.nodes) { if (!ts.isBindingElement(element)) continue; @@ -606,9 +615,11 @@ class ModuleScanner { private visit(node: ts.Node): void { if (ts.isCallExpression(node)) this.visitCall(node); else if (ts.isNewExpression(node)) this.visitNew(node); - else if (ts.isBinaryExpression(node) && isAssignmentOperator(node.operatorToken.kind)) + else if (ts.isBinaryExpression(node) && isAssignmentOperator(node.operatorToken.kind)) { this.visitWrite(node.left, node, "write"); - else if (ts.isDeleteExpression(node)) this.visitDelete(node); + if (node.operatorToken.kind === ts.SyntaxKind.EqualsToken) + this.recordCjsPatternReads(node.left, node.right); + } else if (ts.isDeleteExpression(node)) this.visitDelete(node); else if (ts.isPrefixUnaryExpression(node) || ts.isPostfixUnaryExpression(node)) { if ( node.operator === ts.SyntaxKind.PlusPlusToken || @@ -633,6 +644,8 @@ class ModuleScanner { if ((text === "arguments.callee" || text === "arguments.caller") && !this.isStrict(node)) this.add("R2.4", "sloppy-arguments", `'${text}'`, node); this.recordCjsRead(node); + } else if (ts.isElementAccessExpression(node)) { + this.recordCjsRead(node); } else if (node.kind === ts.SyntaxKind.ThisKeyword) { this.visitThis(node); } else if (ts.isObjectLiteralExpression(node)) { @@ -651,13 +664,14 @@ class ModuleScanner { /** Records a read of a member of a builtin module's CommonJS object, so * a patch installed by an island module can be checked against it * (H3.4). The left of an assignment (and a delete or ++/-- target) is a - * write, not a read. */ - private recordCjsRead(node: ts.PropertyAccessExpression): void { + * write, not a read. A computed key reads a member the scan cannot name, + * so it is recorded as the whole module ("fs.*"). */ + private recordCjsRead(node: ts.PropertyAccessExpression | ts.ElementAccessExpression): void { const root = chainRoot(node); if (root === null || !this.cjsBuiltinObjects.has(root.text)) return; const module = this.builtinBindings.get(root.text); + if (module === undefined) return; const text = chainText(node); - if (module === undefined || text === null) return; const parent = node.parent; if (parent !== undefined) { if ( @@ -673,12 +687,78 @@ class ModuleScanner { ) return; } + this.pushCjsRead(text === null ? `${module}.*` : module + text.slice(root.text.length), node); + } + + private pushCjsRead(member: string, node: ts.Node): void { this.cjsReads.push({ - member: module + text.slice(root.text.length), + member, loc: { file: this.sf.fileName, start: ts.tokenStartOfNode(node), end: node.end }, }); } + /** The reads of a destructuring whose source is a builtin module's + * CommonJS object (`const { readFileSync } = require("fs")`, + * `({ readFileSync } = fs)`): each property it takes is read when the + * destructuring runs, so it sees a patch installed before. A rest element + * or a computed key reads members the scan cannot name ("fs.*"). */ + private recordCjsPatternReads(pattern: ts.Node, source: ts.Node | undefined): void { + if (source === undefined) return; + const value = stripWrappers(source); + if (!this.isCjsObjectValue(value)) return; + const module = this.builtinOf(value); + if (module === null || module.includes(".")) return; + this.recordPatternReads(pattern, module); + } + + private recordPatternReads(pattern: ts.Node, base: string): void { + const target = stripWrappers(pattern); + const elements: { + key: ts.Node | undefined; + value: ts.Node | undefined; + rest: boolean; + node: ts.Node; + }[] = []; + if (ts.isObjectBindingPattern(target)) { + for (const element of target.elements.nodes) { + if (!ts.isBindingElement(element)) continue; + elements.push({ + key: element.propertyName ?? element.name, + value: element.name, + rest: element.dotDotDotToken !== undefined, + node: element, + }); + } + } else if (ts.isObjectLiteralExpression(target)) { + for (const prop of target.properties.nodes) { + if (ts.isPropertyAssignment(prop)) + elements.push({ key: prop.name, value: prop.initializer, rest: false, node: prop }); + else if (ts.isShorthandPropertyAssignment(prop)) + elements.push({ key: prop.name, value: undefined, rest: false, node: prop }); + else if (ts.isSpreadAssignment(prop)) + elements.push({ key: undefined, value: undefined, rest: true, node: prop }); + } + } else return; + for (const element of elements) { + const key = element.rest || element.key === undefined ? null : staticKey(element.key); + if (key === null) { + this.pushCjsRead(`${base.split(".")[0]}.*`, element.node); + continue; + } + const member = `${base}.${key}`; + this.pushCjsRead(member, element.node); + // A nested pattern reads the member's own properties too. + if (element.value !== undefined) { + let inner = stripWrappers(element.value); + // `{ readFileSync = fallback }` (a default) and `{ a: { b } = {} }`. + if (ts.isBinaryExpression(inner) && inner.operatorToken.kind === ts.SyntaxKind.EqualsToken) + inner = stripWrappers(inner.left); + if (ts.isObjectBindingPattern(inner) || ts.isObjectLiteralExpression(inner)) + this.recordPatternReads(inner, member); + } + } + } + private visitSpecifier(spec: string, node: ts.Node): void { const builtin = builtinModuleName(spec); if (builtin === "vm") this.add("R1.1", "vm", "the 'vm' module", node); diff --git a/packages/compiler/src/node-islands/modules.test.ts b/packages/compiler/src/node-islands/modules.test.ts index 8f443a5b5..f69f954eb 100644 --- a/packages/compiler/src/node-islands/modules.test.ts +++ b/packages/compiler/src/node-islands/modules.test.ts @@ -269,6 +269,25 @@ describe("the collected module tree", () => { expect(paths).not.toContain("helper.ts"); }); + test("named data stays inside the module's directory: links out of it are not followed", () => { + const outside = project({ "secrets/config.json": "{}", "token.txt": "secret\n" }); + const root = project({ + "src/main.ts": "", + "src/loader.ts": 'const names = ["config.json", "token.txt", "payload.txt"];', + "src/data/payload.txt": "shipped\n", + }); + symlinkSync(join(outside, "secrets"), join(root, "src/linked"), "dir"); + symlinkSync(join(outside, "token.txt"), join(root, "src/data/token.txt")); + // A link back up the tree must not loop the walk. + symlinkSync(join(root, "src"), join(root, "src/data/loop"), "dir"); + const collector = new NodeIslandModuleCollector(); + expect(collector.add(join(root, "src/main.ts"), "./loader.ts")).toBeNull(); + const paths = collector.tree()!.files.map((file) => file.path); + expect(paths).toContain("data/payload.txt"); + expect(paths.filter((path) => path.endsWith("config.json"))).toEqual([]); + expect(paths.filter((path) => path.endsWith("token.txt"))).toEqual([]); + }); + test("the manifest names relative paths only, and the data is checked against it", () => { const root = project({ "main.ts": "", "m.cjs": "exports.f = () => 1;" }); const collector = new NodeIslandModuleCollector(); diff --git a/packages/compiler/src/node-islands/modules.ts b/packages/compiler/src/node-islands/modules.ts index ead084088..e01a0b6e1 100644 --- a/packages/compiler/src/node-islands/modules.ts +++ b/packages/compiler/src/node-islands/modules.ts @@ -343,18 +343,32 @@ export class NodeIslandModuleCollector { * read to the wasm next to the patching module), and the island runs from * the shipped tree, so a file the module names by a string literal ships * with it. Files the source does not name stay out, and module sources - * ship through the traced walk, never as data. */ + * ship through the traced walk, never as data. The walk stays inside the + * module's directory: a symbolic link (to a file or a directory) that + * leads outside it is not followed, so nothing from elsewhere on the + * machine is embedded. */ private addNamedData(moduleFile: string, text: string): void { const names = new Set(); for (const token of moduleTokens(text)) if (token.kind === "string" && !token.value.includes("${")) names.add(basename(token.value)); if (names.size === 0) return; + const root = dirname(moduleFile); + const inside = (path: string): boolean => { + const rel = relative(root, path); + return rel === "" || (!rel.startsWith("..") && !isAbsolute(rel)); + }; + const visited = new Set(); const walk = (dir: string): void => { + const real = trackedRealpath(dir); + if (real === null || !inside(real) || visited.has(real)) return; + visited.add(real); const entries = trackedAccessibleEntries(dir); if (entries === null) return; for (const file of entries.files) { const path = join(dir, file); if (!names.has(file) || TRACED.test(file) || path === moduleFile) continue; + const target = trackedRealpath(path); + if (target === null || !inside(target)) continue; this.files.add(path); } for (const sub of entries.directories) { @@ -362,7 +376,7 @@ export class NodeIslandModuleCollector { walk(join(dir, sub)); } }; - walk(dirname(moduleFile)); + walk(root); } /** A package: its whole directory, then its dependencies. */ diff --git a/packages/compiler/src/node-islands/type-strip.test.ts b/packages/compiler/src/node-islands/type-strip.test.ts new file mode 100644 index 000000000..77045a787 --- /dev/null +++ b/packages/compiler/src/node-islands/type-strip.test.ts @@ -0,0 +1,43 @@ +import { afterAll, expect, test } from "vitest"; +import type * as ts from "../frontend/ts7/adapter.js"; +import { closeSourceParser, parseSourceFile } from "../frontend/ts7/source-parser-node.js"; +import { stripProblem } from "./type-strip.js"; + +afterAll(() => closeSourceParser()); + +let serial = 0; +/** The SC8006 construct for a TypeScript module, or null. None of these + * cases needs the checker (only a plain named import of a type does). */ +function problem(source: string): string | null { + const sf = parseSourceFile(`/virtual/s${serial++}.ts`, source, "ts"); + const program = { + getTypeChecker(): never { + throw new Error("the checker is not needed here"); + }, + } as unknown as ts.Program; + return stripProblem(program, sf)?.construct ?? null; +} + +test("a relative specifier Node resolves needs its file extension", () => { + expect(problem('import { a } from "./a";')).toBe( + "the relative import './a' without a file extension", + ); + expect(problem('export { a } from "./a";')).toBe( + "the relative export from './a' without a file extension", + ); + expect(problem('const m = await import("./a");')).toBe( + "the relative import() of './a' without a file extension", + ); + expect(problem('import "./a.ts";\nexport * from "./b.js";')).toBeNull(); +}); + +test("type-only imports and exports are erased, so their specifiers need no extension", () => { + expect(problem('import type { A } from "./types";')).toBeNull(); + expect(problem('import type Default from "./types";')).toBeNull(); + expect(problem('export type { A } from "./types";')).toBeNull(); + expect(problem('export type * from "./types";')).toBeNull(); + // An inline `type` element leaves the declaration, which Node still loads. + expect(problem('import { type A } from "./types";')).toBe( + "the relative import './types' without a file extension", + ); +}); diff --git a/packages/compiler/src/node-islands/type-strip.ts b/packages/compiler/src/node-islands/type-strip.ts index 4e8c61b45..1394d831f 100644 --- a/packages/compiler/src/node-islands/type-strip.ts +++ b/packages/compiler/src/node-islands/type-strip.ts @@ -124,15 +124,16 @@ export function stripProblem(program: ts.Program, sf: ts.SourceFile): StripProbl return undefined; } if (ts.isImportDeclaration(node)) { + const clause = node.importClause; + // `import type` is erased whole, so Node never resolves its specifier. + if (clause?.phaseModifier === ts.SyntaxKind.TypeKeyword) return undefined; const spec = ts.isStringLiteral(node.moduleSpecifier) ? node.moduleSpecifier.text : null; if (spec !== null && isRelativeSpecifier(spec) && !hasResolvedExtension(spec)) { report(`the relative import '${spec}' without a file extension`); return undefined; } - const clause = node.importClause; if ( clause === undefined || - clause.phaseModifier === ts.SyntaxKind.TypeKeyword || clause.namedBindings === undefined || !ts.isNamedImports(clause.namedBindings) ) @@ -152,7 +153,9 @@ export function stripProblem(program: ts.Program, sf: ts.SourceFile): StripProbl } if (ts.isExportDeclaration(node)) { const specifier = node.moduleSpecifier; + // `export type { T } from` and `export type * from` are erased whole. if ( + !node.isTypeOnly && specifier !== undefined && ts.isStringLiteral(specifier) && isRelativeSpecifier(specifier.text) && diff --git a/tests/harness/node-island.test.ts b/tests/harness/node-island.test.ts index 326f6530a..6f4450c41 100644 --- a/tests/harness/node-island.test.ts +++ b/tests/harness/node-island.test.ts @@ -501,6 +501,59 @@ export const patched = Kind.A; expect(stripped).toBeDefined(); expect(stripped).toContain("enum"); }); + + test("a TypeScript patcher shares the program modules it imports with native code", async () => { + // state.ts is a stateful program module that both the native entry and + // the islanded patcher import: Node runs one copy of it, so the island + // must reach the program's copy, not one of its own. + const dir = copyCorpus( + "shared-state", + `import { bump, count } from "./state.ts"; +import { patchAndBump } from "./statepatch.ts"; +bump(); +const fromIsland = patchAndBump(); +console.log(fromIsland, count()); +`, + { + "state.ts": `let value = 0; +export function bump(): number { + value++; + return value; +} +export function count(): number { + return value; +} +`, + "statepatch.ts": `import { createRequire } from "node:module"; +import { bump } from "./state.ts"; +const require = createRequire(import.meta.url); +const fs = require("fs") as typeof import("fs"); +const original = fs.readFileSync; +export function patchAndBump(): number { + fs.readFileSync = original; + return bump(); +} +`, + }, + ); + const result = await buildDir(dir, "main"); + expect(messages(result)).toEqual([]); + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.islands?.modules.map((file) => basename(file))).toEqual(["statepatch.ts"]); + // Node strips types only outside node_modules, and the shared island + // cache of these tests lives below the repository's node_modules. + const islandCache = mkdtempSync(join(tmpdir(), "scriptc-island-cache-")); + try { + const out = await run(join(dir, "main"), [], { + ...process.env, + XDG_CACHE_HOME: islandCache, + }); + expect(out).toEqual({ stdout: "2 2\n", stderr: "", code: 0 }); + } finally { + rmSync(islandCache, { recursive: true, force: true }); + } + }, 300_000); }); }); diff --git a/tests/harness/test262-runner.test.ts b/tests/harness/test262-runner.test.ts index 46a0b5fa0..e1eb96a5b 100644 --- a/tests/harness/test262-runner.test.ts +++ b/tests/harness/test262-runner.test.ts @@ -1,7 +1,10 @@ +import { join } from "node:path"; +import ts from "typescript"; import { expect, test } from "vitest"; import { boundedRun } from "../test262/execute.js"; import { compileFailureStatus, + directory, exclusion, matchesParseNegative, metadata, @@ -132,6 +135,40 @@ test("a lifted script-environment variant keeps the script shape inside its isla expect(scriptEnvironment("export default 1;")).toBe(false); }); +test("the harness declares each name once and exports every name the island prelude reads", () => { + const file = join(directory, "harness.ts"); + const program = ts.createProgram([file], { + noEmit: true, + strict: true, + target: ts.ScriptTarget.ES2022, + module: ts.ModuleKind.NodeNext, + moduleResolution: ts.ModuleResolutionKind.NodeNext, + allowImportingTsExtensions: true, + }); + const sf = program.getSourceFile(file)!; + // Duplicate identifier, redeclared block-scoped variable or export, and + // duplicate function implementation: each fails every executable variant. + const duplicates = program + .getSemanticDiagnostics(sf) + .filter((d) => [2300, 2323, 2393, 2451].includes(d.code)) + .map((d) => ts.flattenDiagnosticMessageText(d.messageText, "\n")); + expect(duplicates).toEqual([]); + const checker = program.getTypeChecker(); + const exported = new Set( + checker.getExportsOfModule(checker.getSymbolAtLocation(sf)!).map((symbol) => symbol.name), + ); + const prelude = prepareIsland(source("description: names"), true, "MARK", [], true); + const read = [...prelude.matchAll(/(? match[1]!, + ); + expect(read.length).toBeGreaterThan(0); + expect(read.filter((name) => !exported.has(name))).toEqual([]); + // The bare compareArray and the assertion are different functions. + expect(prelude).toContain("assert.compareArray = harness.assertCompareArray;"); + expect(prelude).toContain("const compareArray = harness.compareArray;"); + expect(prelude).toContain("globalThis.compareArray = compareArray;"); +}); + test("the property helper only admits direct inline descriptor checks", () => { for (const body of [ "verifyProperty({}, 'x', { value: 1, configurable: false });", diff --git a/tests/test262/harness.ts b/tests/test262/harness.ts index ab47fc96f..577d3b3f0 100644 --- a/tests/test262/harness.ts +++ b/tests/test262/harness.ts @@ -124,8 +124,10 @@ export namespace assert { // function crosses the bridge as a callable, but the namespace members // attached to it do not (they are properties of the native function object). // Test262's `assert` must be callable and carry the helpers, so each helper -// is exported on its own and the island wrapper builds the object. +// is exported on its own and the island wrapper builds the object. The +// assertion form of compareArray takes its own name: `compareArray` is the +// bare function above. export const sameValue = assert.sameValue; export const notSameValue = assert.notSameValue; export const assertThrows = assert.throws; -export const compareArray = assert.compareArray; +export const assertCompareArray = assert.compareArray; diff --git a/tests/test262/support.mjs b/tests/test262/support.mjs index f4d8eb32c..b907f3e19 100644 --- a/tests/test262/support.mjs +++ b/tests/test262/support.mjs @@ -249,7 +249,8 @@ export function prepareIsland(source, asyncTest = false, marker = completion, in "assert.sameValue = harness.sameValue;", "assert.notSameValue = harness.notSameValue;", "assert.throws = harness.assertThrows;", - "assert.compareArray = harness.compareArray;", + "assert.compareArray = harness.assertCompareArray;", + "const compareArray = harness.compareArray;", "const Test262Error = harness.Test262Error;", ...(asyncTest ? ["const $DONE = harness.$DONE;"] : []), ]; @@ -258,7 +259,12 @@ export function prepareIsland(source, asyncTest = false, marker = completion, in // top-level `this` is the global object and its top-level `var` bindings are // global properties. The island therefore replays the body as script code, // and the module carries it only as a dead copy the pre-scan can island. - const globals = ["globalThis.assert = assert;", "globalThis.Test262Error = Test262Error;"]; + const globals = [ + "globalThis.assert = assert;", + "globalThis.compareArray = compareArray;", + "globalThis.Test262Error = Test262Error;", + ...(asyncTest ? ["globalThis.$DONE = $DONE;"] : []), + ]; return [ 'const { readFileSync } = require("node:fs");', 'const vm = require("node:vm");', From 58b650dca706b9c56acdf33d0cf97530f25cf066 Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Thu, 29 Oct 2026 19:48:40 -0700 Subject: [PATCH 09/14] Record the sloppy island corpus program's preflight on the current base The recorded entry came from the branch's older base. The preflight of lookup.js no longer reports the two `Cannot find name` errors for the names a `with` resolves against its record (`last` in describe, `inner` in the nested with); the `with`-in-strict-mode errors and the rest of the entry are unchanged. The corpus differential still matches Node on the program's output, so this records the behavior the tree has today. Co-Authored-By: Claude Opus 5.5 --- .../8427-sloppy-with-island/main.js.json | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/8427-sloppy-with-island/main.js.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/8427-sloppy-with-island/main.js.json index cb38c0ce6..eec7c13b3 100644 --- a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/8427-sloppy-with-island/main.js.json +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/8427-sloppy-with-island/main.js.json @@ -22,15 +22,6 @@ "end": 207 } }, - { - "code": "SC0001", - "message": "Cannot find name 'last'.", - "loc": { - "file": "/tests/corpus/8427-sloppy-with-island/lookup.js", - "start": 252, - "end": 256 - } - }, { "code": "SC0001", "message": "'with' statements are not allowed in strict mode.", @@ -93,15 +84,6 @@ "start": 548, "end": 552 } - }, - { - "code": "SC0001", - "message": "Cannot find name 'inner'.", - "loc": { - "file": "/tests/corpus/8427-sloppy-with-island/lookup.js", - "start": 576, - "end": 581 - } } ] } From 7ab5485c9929a0fef3d1b303445c7bd57e5dd916 Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Thu, 29 Oct 2026 23:33:27 -0500 Subject: [PATCH 10/14] Keep program island modules eager and ship only the data paths they name A program module that runs in a Node island loads with nodeIsland.require instead of nodeIsland.defer, so the build-time probe never evaluates the program's own source. Deferral and its probe stay with island packages. A program island module ships the data files it names by their exact path relative to its directory, including consecutive literal arguments of one call (join(here, "fallback", "payload.txt")). A file elsewhere in the tree with the same name no longer ships. Files the source does not name ship when scriptc.nodeIslands.files declares them (paths or globs relative to the package.json); a declared path without a file is SC8004. The island module order in the lowering is cached in a Map the lowering owns instead of a WeakMap. --- .../frontend/lowering/lower-node-island.ts | 11 +- .../compiler/src/frontend/lowering/lowerer.ts | 24 +++- packages/compiler/src/frontend/pipeline.ts | 5 +- .../compiler/src/node-islands/boundary.ts | 10 ++ .../compiler/src/node-islands/modules.test.ts | 81 ++++++++++- packages/compiler/src/node-islands/modules.ts | 133 ++++++++++++++---- .../compiler/src/node-islands/snapshot.ts | 6 +- packages/compiler/src/project-config.test.ts | 11 ++ packages/compiler/src/project-config.ts | 24 ++++ tests/harness/node-island.test.ts | 92 +++++++++++- 10 files changed, 358 insertions(+), 39 deletions(-) diff --git a/packages/compiler/src/frontend/lowering/lower-node-island.ts b/packages/compiler/src/frontend/lowering/lower-node-island.ts index 742b47d4b..b2d0d99e5 100644 --- a/packages/compiler/src/frontend/lowering/lower-node-island.ts +++ b/packages/compiler/src/frontend/lowering/lower-node-island.ts @@ -48,6 +48,7 @@ import { import { crossingOf, isForeignType, + isIslandProgramModule, isIslandSubclassDecl, markIslandSubclass, islandOwnerOfFile, @@ -402,8 +403,16 @@ function loadIslandModule(lowerer: Lowerer, path: string, loc: SrcLoc): IrExpr { /** A static import's load: `nodeIsland.defer(path, snapshot)`, which the * build decides after lowering (node-islands/snapshot.ts, scope-B NI-7): * a module proved effect-free at load keeps it with its interface snapshot - * and evaluates on first use; any other becomes nodeIsland.require. */ + * and evaluates on first use; any other becomes nodeIsland.require. + * + * A program module that runs in an island loads eagerly + * (`nodeIsland.require`): deciding a deferral means evaluating the module + * in the build-time probe, and the build never runs the program's own + * source. */ function deferIslandModule(lowerer: Lowerer, path: string, loc: SrcLoc): IrExpr { + const islands = lowerer.nodeIslands; + if (islands !== null && isIslandProgramModule(islands, path)) + return loadIslandModule(lowerer, path, loc); return { kind: "libCall", fn: "nodeIsland.defer", diff --git a/packages/compiler/src/frontend/lowering/lowerer.ts b/packages/compiler/src/frontend/lowering/lowerer.ts index ef9d5c31c..cedb12e6c 100644 --- a/packages/compiler/src/frontend/lowering/lowerer.ts +++ b/packages/compiler/src/frontend/lowering/lowerer.ts @@ -293,6 +293,7 @@ import { intersectionTypeDiag, noLoweringDiag, overloadedSignatureTypeDiag, + projectConfigDiag, recordShapeMismatchDiag, stdlibSnapshotValueUseDiag, unsupportedNativeApiDiag, @@ -786,7 +787,8 @@ import { maybePromiseUnionOf, promiseArmedUnionOf, } from "./maybe-promise.js"; -import { NodeIslandModuleCollector } from "../../node-islands/modules.js"; +import { NodeIslandModuleCollector, declaredIslandFiles } from "../../node-islands/modules.js"; +import { readProjectConfig } from "../../project-config.js"; /** Entry function name. '%' cannot appear in a TS identifier, so a user * function can never collide with it (mangling is injective per prefix). */ @@ -9850,6 +9852,7 @@ export class Lowerer { refuseX509DynValues(this, functions); markInferredFactChecks(functions, this.program); const sloppyDynWrites = sloppyDynamicWrites(this, functions); + if (this.usesNodeIsland) this.addDeclaredIslandFiles(); const module: IrModule | null = this.diags.length > 0 ? null @@ -11145,6 +11148,25 @@ export class Lowerer { this.diags.push(d); } + /** `scriptc.nodeIslands.files`: files the program's island modules read + * without naming their path ship with those modules. A declared path + * without a file is SC8004. */ + private addDeclaredIslandFiles(): void { + if (this.nodeIslands === null || this.nodeIslands.modules.size === 0) return; + const declared = declaredIslandFiles(readProjectConfig(this.entry.fileName)); + const loc: SrcLoc = { file: this.entry.fileName, start: 0, end: 0 }; + for (const path of declared.missing) + this.pushDiag( + projectConfigDiag( + "scriptc.nodeIslands.files", + `names ${path}, which does not exist`, + "remove the entry or create the file", + loc, + ), + ); + for (const file of declared.files) this.nodeIslandModules.addDeclaredFile(file); + } + /** The module's Node island record: the collected module tree, with each * call's caller literal and each load's path literal set to its path in * that tree. The executable build fills in the libnode pack (index.ts). */ diff --git a/packages/compiler/src/frontend/pipeline.ts b/packages/compiler/src/frontend/pipeline.ts index cbd1fb238..aba2dcd8b 100644 --- a/packages/compiler/src/frontend/pipeline.ts +++ b/packages/compiler/src/frontend/pipeline.ts @@ -920,8 +920,9 @@ function loadFrontend( lower: (opts, valid) => { // The module order without the island modules, computed once per load: // a reload keeps the array the lowering appends its dynamic import - // modules to. - const islandFiltered = new WeakMap(); + // modules to. The map lives as long as this lowering, which owns the + // loads it keys. + const islandFiltered = new Map(); const lowerOrder = (l: typeof finalLoad): SourceFile[] => { if (islandModules === null) return l.moduleOrder; let kept = islandFiltered.get(l.moduleOrder); diff --git a/packages/compiler/src/node-islands/boundary.ts b/packages/compiler/src/node-islands/boundary.ts index bd317e23f..41f4ab845 100644 --- a/packages/compiler/src/node-islands/boundary.ts +++ b/packages/compiler/src/node-islands/boundary.ts @@ -14,6 +14,7 @@ * calls. A union is foreign when every arm that is not null or undefined * is foreign (the dyn representation holds null and undefined). */ import * as ts from "../frontend/ts7/adapter.js"; +import { trackedRealpath } from "../frontend/input-tracker.js"; import { packageDirOf } from "./origin.js"; /** One package the build runs in a Node island. */ @@ -60,6 +61,15 @@ export function islandModuleOfFile(islands: NodeIslandSet, file: string): Island return islands.modules.get(file) ?? islands.modules.get(normalizePath(file)) ?? null; } +/** Whether `file` (a path as the checker spells it, or its real path) is a + * program module of the island set. */ +export function isIslandProgramModule(islands: NodeIslandSet, file: string): boolean { + if (islandModuleOfFile(islands, file) !== null) return true; + for (const module of islands.modules.values()) + if ((trackedRealpath(module.file) ?? module.file) === file) return true; + return false; +} + /** The island (package or module) that owns a declaration or JavaScript * file, or null. Modules first: a user module is never inside a package. */ export function islandOwnerOfFile( diff --git a/packages/compiler/src/node-islands/modules.test.ts b/packages/compiler/src/node-islands/modules.test.ts index f69f954eb..499a59e78 100644 --- a/packages/compiler/src/node-islands/modules.test.ts +++ b/packages/compiler/src/node-islands/modules.test.ts @@ -2,8 +2,11 @@ import { mkdirSync, mkdtempSync, realpathSync, symlinkSync, writeFileSync } from import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; import { describe, expect, test } from "vitest"; +import { readProjectConfig } from "../project-config.js"; import { NodeIslandModuleCollector, + declaredIslandFiles, + namedDataPaths, nodeIslandManifest, readNodeIslandModules, requireModuleSpecifiers, @@ -19,6 +22,26 @@ function project(files: Record): string { return root; } +describe("data paths a module names", () => { + test("string literals, and consecutive literal arguments joined", () => { + const names = namedDataPaths(` + const a = readFileSync(join(here, "fallback", "payload.txt")); + const b = "assets/config.json"; + const c = resolve(__dirname, "x", base, "y"); + // "comment.txt" + const d = \`\${dir}/template.txt\`; + `); + expect([...names].sort()).toEqual([ + "assets/config.json", + "fallback", + "fallback/payload.txt", + "payload.txt", + "x", + "y", + ]); + }); +}); + describe("module specifiers in JavaScript", () => { test("string-literal loads in every form", () => { const scan = scanModuleSpecifiers(` @@ -273,7 +296,8 @@ describe("the collected module tree", () => { const outside = project({ "secrets/config.json": "{}", "token.txt": "secret\n" }); const root = project({ "src/main.ts": "", - "src/loader.ts": 'const names = ["config.json", "token.txt", "payload.txt"];', + "src/loader.ts": + 'const names = ["linked/config.json", "data/token.txt", "data/payload.txt", "data/loop/data/payload.txt"];', "src/data/payload.txt": "shipped\n", }); symlinkSync(join(outside, "secrets"), join(root, "src/linked"), "dir"); @@ -286,6 +310,61 @@ describe("the collected module tree", () => { expect(paths).toContain("data/payload.txt"); expect(paths.filter((path) => path.endsWith("config.json"))).toEqual([]); expect(paths.filter((path) => path.endsWith("token.txt"))).toEqual([]); + expect(paths.filter((path) => path.includes("loop"))).toEqual([]); + }); + + test("only the exact path a module names ships, never a file of the same name elsewhere", () => { + const outside = project({ "outside.txt": "private\n" }); + const root = project({ + "src/main.ts": "", + "src/loader.cjs": [ + 'const fs = require("node:fs");', + 'const config = fs.readFileSync(__dirname + "/" + "assets/config.json");', + 'const up = "../notes.txt";', + `const away = "${join(outside, "outside.txt")}";`, + 'const deps = "node_modules/dep/config.json";', + ].join("\n"), + "src/assets/config.json": "{}", + "src/private/config.json": '{ "token": "secret" }', + "src/config.json": '{ "token": "secret" }', + "src/node_modules/dep/config.json": "{}", + "notes.txt": "beside the source directory\n", + }); + const collector = new NodeIslandModuleCollector(); + expect(collector.add(join(root, "src/main.ts"), "./loader.cjs")).toBeNull(); + const paths = collector.tree()!.files.map((file) => file.path); + expect(paths.filter((path) => !path.endsWith(".cjs"))).toEqual(["assets/config.json"]); + }); + + test("files scriptc.nodeIslands.files declares ship with the island modules", () => { + const root = project({ + "package.json": JSON.stringify({ + scriptc: { nodeIslands: { files: ["data/*.json", "docs/readme.txt", "absent.bin"] } }, + }), + "main.ts": "", + "data/a.json": "{}", + "data/b.json": "{}", + "data/c.txt": "", + "data/nested/d.json": "{}", + "docs/readme.txt": "", + "node_modules/x/data/e.json": "{}", + }); + const declared = declaredIslandFiles(readProjectConfig(join(root, "main.ts"))); + expect(declared.files).toEqual([ + join(root, "data/a.json"), + join(root, "data/b.json"), + join(root, "docs/readme.txt"), + ]); + expect(declared.missing).toEqual(["absent.bin"]); + const collector = new NodeIslandModuleCollector(); + expect(collector.add(join(root, "main.ts"), "./data/a.json")).toBeNull(); + for (const file of declared.files) collector.addDeclaredFile(file); + expect(collector.tree()!.files.map((file) => file.path)).toEqual([ + "data/a.json", + "data/b.json", + "docs/readme.txt", + "package.json", + ]); }); test("the manifest names relative paths only, and the data is checked against it", () => { diff --git a/packages/compiler/src/node-islands/modules.ts b/packages/compiler/src/node-islands/modules.ts index e01a0b6e1..ae97de055 100644 --- a/packages/compiler/src/node-islands/modules.ts +++ b/packages/compiler/src/node-islands/modules.ts @@ -40,7 +40,9 @@ import { trackedRealpath, } from "../frontend/input-tracker.js"; import type { IrNodeIslandModules } from "../ir/ir.js"; +import type { ProjectConfig } from "../project-config.js"; import { packageDirOf } from "./origin.js"; +import { globMatch } from "./policy.js"; export type NodeIslandModuleTree = IrNodeIslandModules; type NodeIslandModuleFile = IrNodeIslandModules["files"][number]; @@ -341,42 +343,39 @@ export class NodeIslandModuleCollector { /** Data files beside a program module that runs in an island: the module * resolves them from its own directory (photon redirects a package's * read to the wasm next to the patching module), and the island runs from - * the shipped tree, so a file the module names by a string literal ships - * with it. Files the source does not name stay out, and module sources - * ship through the traced walk, never as data. The walk stays inside the - * module's directory: a symbolic link (to a file or a directory) that - * leads outside it is not followed, so nothing from elsewhere on the - * machine is embedded. */ + * the shipped tree, so a file the module names by its path relative to + * the module's directory ships with it (namedDataPaths). Only the exact + * path ships: a file elsewhere in the tree with the same name stays out, + * and so does every file the source does not name; such files ship when + * `scriptc.nodeIslands.files` declares them (addDeclaredFile). Module + * sources ship through the traced walk, never as data. A named path + * stays inside the module's directory: one that leads outside it, + * through `..` or a symbolic link, or into node_modules or .git, is not + * shipped, so nothing from elsewhere on the machine is embedded. */ private addNamedData(moduleFile: string, text: string): void { - const names = new Set(); - for (const token of moduleTokens(text)) - if (token.kind === "string" && !token.value.includes("${")) names.add(basename(token.value)); - if (names.size === 0) return; const root = dirname(moduleFile); const inside = (path: string): boolean => { const rel = relative(root, path); - return rel === "" || (!rel.startsWith("..") && !isAbsolute(rel)); + return rel !== "" && !rel.startsWith("..") && !isAbsolute(rel); }; - const visited = new Set(); - const walk = (dir: string): void => { - const real = trackedRealpath(dir); - if (real === null || !inside(real) || visited.has(real)) return; - visited.add(real); - const entries = trackedAccessibleEntries(dir); - if (entries === null) return; - for (const file of entries.files) { - const path = join(dir, file); - if (!names.has(file) || TRACED.test(file) || path === moduleFile) continue; - const target = trackedRealpath(path); - if (target === null || !inside(target)) continue; - this.files.add(path); - } - for (const sub of entries.directories) { - if (sub === "node_modules" || sub === ".git") continue; - walk(join(dir, sub)); - } - }; - walk(root); + for (const name of namedDataPaths(text)) { + if (isAbsolute(name) || name.includes("\0")) continue; + const path = resolve(root, name); + if (!inside(path) || path === moduleFile || TRACED.test(path)) continue; + const parts = relative(root, path).split(sep); + if (parts.includes("node_modules") || parts.includes(".git")) continue; + if (!trackedFileExists(path)) continue; + const target = trackedRealpath(path); + if (target === null || !inside(target)) continue; + // A link inside the directory stays a link in the tree (placed). + this.files.add(this.placed(path)); + } + } + + /** A file `scriptc.nodeIslands.files` declares for the island modules of + * the program (declaredIslandFiles): shipped at its path in the tree. */ + addDeclaredFile(file: string): void { + this.files.add(this.placed(file)); } /** A package: its whole directory, then its dependencies. */ @@ -632,6 +631,78 @@ export interface ModuleSpecifierScan { computed: { at: number; form: string }[]; } +/** The files `scriptc.nodeIslands.files` declares: each path, relative to + * the package.json, and the files under the package.json's directory each + * glob matches (node_modules and .git directories excluded). `missing` + * lists the paths without a file. */ +export function declaredIslandFiles(config: ProjectConfig): { files: string[]; missing: string[] } { + const patterns = config.nodeIslands.files; + if (patterns.length === 0) return { files: [], missing: [] }; + const base = config.manifest === null ? process.cwd() : dirname(config.manifest); + const files = new Set(); + const missing: string[] = []; + const globs: string[] = []; + for (const pattern of patterns) { + const path = resolve(base, pattern); + if (!/[*?]/.test(pattern)) { + if (trackedFileExists(path)) files.add(path); + else missing.push(pattern); + } else globs.push(portable(path)); + } + if (globs.length > 0) { + const walk = (dir: string): void => { + const entries = trackedAccessibleEntries(dir); + if (entries === null) return; + for (const file of entries.files) { + const path = join(dir, file); + if (globs.some((glob) => globMatch(glob, portable(path)))) files.add(path); + } + for (const sub of entries.directories) + if (sub !== "node_modules" && sub !== ".git") walk(join(dir, sub)); + }; + walk(base); + } + return { files: [...files].sort(), missing }; +} + +/** The relative paths a module's source names by string literals: each + * literal (`"data/table.json"`), and the literals passed as consecutive + * arguments of one call joined with `/`, as `path.join(here, "fallback", + * "payload.txt")` names `fallback/payload.txt`. Comments and template + * literals with substitutions name nothing. */ +export function namedDataPaths(text: string): Set { + const tokens = moduleTokens(text); + const names = new Set(); + let run: string[] = []; + const end = (): void => { + if (run.length > 1) names.add(run.join("/")); + run = []; + }; + for (let k = 0; k < tokens.length; k++) { + const t = tokens[k]!; + if (t.kind === "string") { + if (t.value.includes("${")) { + end(); + continue; + } + names.add(t.value); + const before = tokens[k - 1]; + const after = tokens[k + 1]; + const argument = + before?.kind === "punct" && + (before.value === "(" || before.value === ",") && + after?.kind === "punct" && + (after.value === ")" || after.value === ","); + if (!argument) end(); + else run.push(t.value); + continue; + } + if (t.kind !== "punct" || t.value !== ",") end(); + } + end(); + return names; +} + /** The string-literal arguments of `requireModule(...)` calls in source * code, comments and template text excluded. */ export function requireModuleSpecifiers(text: string): string[] { diff --git a/packages/compiler/src/node-islands/snapshot.ts b/packages/compiler/src/node-islands/snapshot.ts index 4da7cee06..66c70a24d 100644 --- a/packages/compiler/src/node-islands/snapshot.ts +++ b/packages/compiler/src/node-islands/snapshot.ts @@ -1,8 +1,10 @@ /* Deferred island modules: the decision, the build-time probe and the * interface snapshot (scope-B NI-7, DESIGN.md §2.7, §5.4). * - * Lowering emits `nodeIsland.defer(path, "")` for every static import of an - * island module (lower-node-island.ts). After lowering, once the build + * Lowering emits `nodeIsland.defer(path, "")` for every static import of a + * module of an island package (lower-node-island.ts); a program module that + * runs in an island loads eagerly, so the probe never evaluates the + * program's own source. After lowering, once the build * knows the libnode pack the executable will load, finalizeDeferredIslands * decides each one: * diff --git a/packages/compiler/src/project-config.test.ts b/packages/compiler/src/project-config.test.ts index ddf93d79d..7282390ac 100644 --- a/packages/compiler/src/project-config.test.ts +++ b/packages/compiler/src/project-config.test.ts @@ -17,6 +17,7 @@ test("a package.json without a scriptc key has the default configuration", () => expect(c.nodeIslands).toEqual({ auto: true, modules: [], + files: [], node: null, nodeVersion: null, packaging: "embedded", @@ -103,6 +104,7 @@ test("nodeIslands settings", () => { nodeIslands: { auto: false, modules: ["src/plugins/*.ts"], + files: ["data/*.json"], node: "/usr/local/bin/node", nodeVersion: ">=24.13.1", packaging: "beside", @@ -115,6 +117,7 @@ test("nodeIslands settings", () => { expect(c.nodeIslands).toEqual({ auto: false, modules: ["src/plugins/*.ts"], + files: ["data/*.json"], node: "/usr/local/bin/node", nodeVersion: ">=24.13.1", packaging: "beside", @@ -133,6 +136,14 @@ test("publicEnv must be an array of variable names", () => { expect(c.nodeIslands.publicEnv).toEqual([]); }); +test("files must be an array of paths or globs", () => { + const c = config({ nodeIslands: { files: "data/config.json" } }); + expect(c.diagnostics.map((d) => d.message)).toEqual([ + `package.json 'scriptc.nodeIslands.files' must be an array of file paths or globs, not "data/config.json"`, + ]); + expect(c.nodeIslands.files).toEqual([]); +}); + test("unknown keys and values are SC8004, located at the key", () => { const text = JSON.stringify( { diff --git a/packages/compiler/src/project-config.ts b/packages/compiler/src/project-config.ts index 9b52993db..653b9678d 100644 --- a/packages/compiler/src/project-config.ts +++ b/packages/compiler/src/project-config.ts @@ -14,6 +14,8 @@ * "zod": { "shapes": false } // no shape-inference overlay * }, * "nodeIslands": { "auto": true, "modules": ["src/plugins/*.ts"] }, + * // files island modules of the program read, shipped with them + * // (node-islands/modules.ts): "nodeIslands": { "files": ["data/*.json"] } * "wasm": ["build/module.wasm"], * "typePatches": { // frontend/type-patches/layer.ts * "commander@^15.0.0": { @@ -60,6 +62,10 @@ export interface NodeIslandsSetting { /** User modules (paths or globs relative to the package.json) that run * in Node. */ modules: string[]; + /** Files (paths or globs relative to the package.json) that ship with + * the island modules of the program, beside the files their source names + * by path (node-islands/modules.ts). */ + files: string[]; /** The `node` executable recorded into the binary. */ node: string | null; /** A semver range checked at the helper handshake. */ @@ -131,6 +137,7 @@ const PACKAGE_SETTING_KEYS = [...Object.keys(PACKAGE_OBJECT_KEYS), "shapes"]; const NODE_ISLANDS_KEYS = [ "auto", "modules", + "files", "node", "nodeVersion", "packaging", @@ -154,6 +161,7 @@ function defaultNodeIslands(): NodeIslandsSetting { return { auto: true, modules: [], + files: [], node: null, nodeVersion: null, packaging: "embedded", @@ -344,6 +352,22 @@ export function parseProjectConfig(manifest: string, text: string): ProjectConfi `write "modules": ["src/plugins/loader.ts"]`, ); } else setting.modules = list; + } else if (key === "files") { + const list: string[] = []; + let ok = Array.isArray(v); + if (Array.isArray(v)) { + for (const entry of v as unknown[]) { + if (typeof entry === "string" && entry !== "") list.push(entry); + else ok = false; + } + } + if (!ok) { + problem( + ["nodeIslands", key], + `must be an array of file paths or globs, not ${shown(v)}`, + `write "files": ["data/config.json"]`, + ); + } else setting.files = list; } else if (key === "publicEnv") { const list: string[] = []; let ok = Array.isArray(v); diff --git a/tests/harness/node-island.test.ts b/tests/harness/node-island.test.ts index 6f4450c41..46e64cfc2 100644 --- a/tests/harness/node-island.test.ts +++ b/tests/harness/node-island.test.ts @@ -23,7 +23,7 @@ import { import { tmpdir } from "node:os"; import { basename, dirname, join } from "node:path"; import { promisify } from "node:util"; -import { beforeAll, describe, expect, test } from "vitest"; +import { beforeAll, describe, expect, test, vi } from "vitest"; import { compile, deserializeModule } from "@scriptc/compiler"; import { definedFunctionNames, nodeIslandSymbols } from "../../scripts/zero-cost-compare.mjs"; import { libnodePack } from "./libnode.js"; @@ -555,8 +555,98 @@ export function patchAndBump(): number { } }, 300_000); }); + + // The build-time probe evaluates modules to decide their deferral. A + // program module that runs in an island loads eagerly, so the build never + // evaluates the program's own source; the executable evaluates it when + // the entry requires it. + test("the build does not evaluate a program module that runs in an island", async () => { + const dir = sloppyModuleProject("probe"); + const cache = mkdtempSync(join(tmpdir(), "scriptc-sloppy-probe-cache-")); + const islandCache = mkdtempSync(join(tmpdir(), "scriptc-island-cache-")); + const previous = process.env["SCRIPTC_CACHE_DIR"]; + process.env["SCRIPTC_CACHE_DIR"] = cache; + try { + const result = await compile(join(dir, "main.js"), { + outPath: join(dir, "main"), + outDir: dir, + sanitize, + libnode: pack!, + }); + if (!result.ok) + throw new Error(result.diagnostics.map((d) => `${d.code}: ${d.message}`).join("\n")); + expect(result.islands?.modules.map((file) => basename(file))).toEqual(["sloppy.js"]); + expect(existsSync(join(dir, "evaluated.txt"))).toBe(false); + const out = await run(join(dir, "main"), [], { ...process.env, XDG_CACHE_HOME: islandCache }); + expect(out).toEqual({ stdout: "2\n", stderr: "", code: 0 }); + expect(existsSync(join(dir, "evaluated.txt"))).toBe(false); + } finally { + if (previous === undefined) delete process.env["SCRIPTC_CACHE_DIR"]; + else process.env["SCRIPTC_CACHE_DIR"] = previous; + rmSync(cache, { recursive: true, force: true }); + rmSync(islandCache, { recursive: true, force: true }); + rmSync(dir, { recursive: true, force: true }); + } + }, 300_000); }); +/** A CommonJS program whose entry requires a sloppy-mode module (a `with` + * statement islands it under the default `--node-islands=auto`). The + * module writes evaluated.txt beside itself when it evaluates. */ +function sloppyModuleProject(name: string): string { + const dir = mkdtempSync(join(tmpdir(), `scriptc-sloppy-${name}-`)); + writeFileSync(join(dir, "package.json"), "{}\n"); + writeFileSync( + join(dir, "main.js"), + 'const { value } = require("./sloppy.js");\nconsole.log(value());\n', + ); + writeFileSync( + join(dir, "sloppy.js"), + [ + 'require("fs").writeFileSync(require("path").join(__dirname, "evaluated.txt"), "evaluated\\n");', + "function value() {", + " with ({ n: 1 }) {", + " return n + 1;", + " }", + "}", + "module.exports = { value };", + "", + ].join("\n"), + ); + return dir; +} + +// A program module that runs in an island never reaches the deferral +// decision, whose probe evaluates the modules it decides: lowering loads it +// eagerly. SCRIPTC_NODE_ISLANDS_TRACE=defer prints every decision; IR +// output needs no libnode pack. +test("a program module that runs in an island is never a deferral candidate", async () => { + const dir = sloppyModuleProject("defer"); + const previous = process.env["SCRIPTC_NODE_ISLANDS_TRACE"]; + process.env["SCRIPTC_NODE_ISLANDS_TRACE"] = "defer"; + const written: string[] = []; + const spy = vi.spyOn(process.stderr, "write").mockImplementation((chunk) => { + written.push(String(chunk)); + return true; + }); + try { + const outPath = join(dir, "main.ir.json"); + const result = await compile(join(dir, "main.js"), { outPath, outDir: dir, outputKind: "ir" }); + if (!result.ok) + throw new Error(result.diagnostics.map((d) => `${d.code}: ${d.message}`).join("\n")); + expect(written.filter((line) => line.includes("sloppy.js"))).toEqual([]); + const ir = JSON.stringify(deserializeModule(readFileSync(outPath, "utf8"))); + expect(ir).toContain("nodeIsland.require"); + expect(ir).not.toContain("nodeIsland.defer"); + expect(existsSync(join(dir, "evaluated.txt"))).toBe(false); + } finally { + spy.mockRestore(); + if (previous === undefined) delete process.env["SCRIPTC_NODE_ISLANDS_TRACE"]; + else process.env["SCRIPTC_NODE_ISLANDS_TRACE"] = previous; + rmSync(dir, { recursive: true, force: true }); + } +}, 300_000); + // Lowering checks every argument and result type for JSON data; IR output // needs no libnode pack. test("recursive JSON types cross into Node", async () => { From 932a14673cc6f23c88971399694e86f8c5620c1c Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Fri, 30 Oct 2026 00:06:16 -0500 Subject: [PATCH 11/14] Refuse unresolved island data paths instead of declaring extra files Removes scriptc.nodeIslands.files. A program island module ships only the data files it names by their exact path, now also a literal appended to __dirname ("/data.json"). When a module names a file by a path where there is none while a file of that name is in its directory, the build fails with SC8006 naming the path to use, instead of building an island that cannot read the file. --- .../compiler/src/diagnostics/diagnostic.ts | 12 ++ .../frontend/lowering/lower-node-island.ts | 11 +- .../compiler/src/frontend/lowering/lowerer.ts | 24 +-- .../compiler/src/node-islands/modules.test.ts | 41 ++--- packages/compiler/src/node-islands/modules.ts | 143 ++++++++++-------- packages/compiler/src/project-config.test.ts | 11 -- packages/compiler/src/project-config.ts | 24 --- 7 files changed, 116 insertions(+), 150 deletions(-) diff --git a/packages/compiler/src/diagnostics/diagnostic.ts b/packages/compiler/src/diagnostics/diagnostic.ts index f03a30874..750b1e948 100644 --- a/packages/compiler/src/diagnostics/diagnostic.ts +++ b/packages/compiler/src/diagnostics/diagnostic.ts @@ -234,6 +234,18 @@ export function nodeIslandModuleDiag(message: string, loc: SrcLoc): ScrDiagnosti }; } +/** SC8006 — a program module that runs in a Node island names a data file + * by a path where there is none while a file of that name is in its + * directory: the build ships data files by their exact path only. */ +export function nodeIslandDataFileDiag(message: string, loc: SrcLoc): ScrDiagnostic { + return { + code: "SC8006", + message, + loc, + hint: "name the data file by its path relative to the module that reads it; the build ships a Node island module's data files by their exact path", + }; +} + /* ── SC6xxx: divergences from Node (warnings) ─────────────────────────── */ /** SC6001 — a value passed to (or declared as) a narrower record type is diff --git a/packages/compiler/src/frontend/lowering/lower-node-island.ts b/packages/compiler/src/frontend/lowering/lower-node-island.ts index b2d0d99e5..250b5d530 100644 --- a/packages/compiler/src/frontend/lowering/lower-node-island.ts +++ b/packages/compiler/src/frontend/lowering/lower-node-island.ts @@ -43,6 +43,7 @@ import { resolveBareModule } from "../resolve.js"; import { nativeSubclassOfIslandDiag, nodeIslandBoundaryDiag, + nodeIslandDataFileDiag, nodeIslandModuleDiag, } from "../../diagnostics/diagnostic.js"; import { @@ -140,7 +141,10 @@ export function lowerNodeApiCall( const caller = callerPath(call); const problem = lowerer.nodeIslandModules.add(caller, specifierNode.text); if (problem !== null) { - lowerer.pushDiag(nodeIslandModuleDiag(`${surface}: ${problem}`, locOf(specifierNode))); + const diag = lowerer.nodeIslandModules.isDataFileProblem(problem) + ? nodeIslandDataFileDiag + : nodeIslandModuleDiag; + lowerer.pushDiag(diag(`${surface}: ${problem}`, locOf(specifierNode))); throw new PoisonError(); } const specifier = lowerer.lowerExprExpecting(specifierNode, STRING); @@ -373,7 +377,10 @@ function islandLoadPath(lowerer: Lowerer, path: string, loc: SrcLoc): IrExpr { } const problem = lowerer.nodeIslandModules.addLoaded(real); if (problem !== null) { - lowerer.pushDiag(nodeIslandModuleDiag(problem, loc)); + const diag = lowerer.nodeIslandModules.isDataFileProblem(problem) + ? nodeIslandDataFileDiag + : nodeIslandModuleDiag; + lowerer.pushDiag(diag(problem, loc)); throw new PoisonError(); } lowerer.nodeIslandLoads.push({ literal, path: real }); diff --git a/packages/compiler/src/frontend/lowering/lowerer.ts b/packages/compiler/src/frontend/lowering/lowerer.ts index cedb12e6c..ef9d5c31c 100644 --- a/packages/compiler/src/frontend/lowering/lowerer.ts +++ b/packages/compiler/src/frontend/lowering/lowerer.ts @@ -293,7 +293,6 @@ import { intersectionTypeDiag, noLoweringDiag, overloadedSignatureTypeDiag, - projectConfigDiag, recordShapeMismatchDiag, stdlibSnapshotValueUseDiag, unsupportedNativeApiDiag, @@ -787,8 +786,7 @@ import { maybePromiseUnionOf, promiseArmedUnionOf, } from "./maybe-promise.js"; -import { NodeIslandModuleCollector, declaredIslandFiles } from "../../node-islands/modules.js"; -import { readProjectConfig } from "../../project-config.js"; +import { NodeIslandModuleCollector } from "../../node-islands/modules.js"; /** Entry function name. '%' cannot appear in a TS identifier, so a user * function can never collide with it (mangling is injective per prefix). */ @@ -9852,7 +9850,6 @@ export class Lowerer { refuseX509DynValues(this, functions); markInferredFactChecks(functions, this.program); const sloppyDynWrites = sloppyDynamicWrites(this, functions); - if (this.usesNodeIsland) this.addDeclaredIslandFiles(); const module: IrModule | null = this.diags.length > 0 ? null @@ -11148,25 +11145,6 @@ export class Lowerer { this.diags.push(d); } - /** `scriptc.nodeIslands.files`: files the program's island modules read - * without naming their path ship with those modules. A declared path - * without a file is SC8004. */ - private addDeclaredIslandFiles(): void { - if (this.nodeIslands === null || this.nodeIslands.modules.size === 0) return; - const declared = declaredIslandFiles(readProjectConfig(this.entry.fileName)); - const loc: SrcLoc = { file: this.entry.fileName, start: 0, end: 0 }; - for (const path of declared.missing) - this.pushDiag( - projectConfigDiag( - "scriptc.nodeIslands.files", - `names ${path}, which does not exist`, - "remove the entry or create the file", - loc, - ), - ); - for (const file of declared.files) this.nodeIslandModules.addDeclaredFile(file); - } - /** The module's Node island record: the collected module tree, with each * call's caller literal and each load's path literal set to its path in * that tree. The executable build fills in the libnode pack (index.ts). */ diff --git a/packages/compiler/src/node-islands/modules.test.ts b/packages/compiler/src/node-islands/modules.test.ts index 499a59e78..45769cdbe 100644 --- a/packages/compiler/src/node-islands/modules.test.ts +++ b/packages/compiler/src/node-islands/modules.test.ts @@ -2,10 +2,8 @@ import { mkdirSync, mkdtempSync, realpathSync, symlinkSync, writeFileSync } from import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; import { describe, expect, test } from "vitest"; -import { readProjectConfig } from "../project-config.js"; import { NodeIslandModuleCollector, - declaredIslandFiles, namedDataPaths, nodeIslandManifest, readNodeIslandModules, @@ -31,7 +29,7 @@ describe("data paths a module names", () => { // "comment.txt" const d = \`\${dir}/template.txt\`; `); - expect([...names].sort()).toEqual([ + expect([...names.keys()].sort()).toEqual([ "assets/config.json", "fallback", "fallback/payload.txt", @@ -336,35 +334,20 @@ describe("the collected module tree", () => { expect(paths.filter((path) => !path.endsWith(".cjs"))).toEqual(["assets/config.json"]); }); - test("files scriptc.nodeIslands.files declares ship with the island modules", () => { + test("a named file that is only elsewhere in the module's directory is a build error", () => { const root = project({ - "package.json": JSON.stringify({ - scriptc: { nodeIslands: { files: ["data/*.json", "docs/readme.txt", "absent.bin"] } }, - }), - "main.ts": "", - "data/a.json": "{}", - "data/b.json": "{}", - "data/c.txt": "", - "data/nested/d.json": "{}", - "docs/readme.txt": "", - "node_modules/x/data/e.json": "{}", + "src/main.ts": "", + "src/loader.cjs": + 'const fs = require("node:fs");\nfs.readFileSync(__dirname + "/config.json");\n', + "src/private/config.json": '{ "token": "secret" }', }); - const declared = declaredIslandFiles(readProjectConfig(join(root, "main.ts"))); - expect(declared.files).toEqual([ - join(root, "data/a.json"), - join(root, "data/b.json"), - join(root, "docs/readme.txt"), - ]); - expect(declared.missing).toEqual(["absent.bin"]); const collector = new NodeIslandModuleCollector(); - expect(collector.add(join(root, "main.ts"), "./data/a.json")).toBeNull(); - for (const file of declared.files) collector.addDeclaredFile(file); - expect(collector.tree()!.files.map((file) => file.path)).toEqual([ - "data/a.json", - "data/b.json", - "docs/readme.txt", - "package.json", - ]); + const problem = collector.add(join(root, "src/main.ts"), "./loader.cjs"); + expect(problem).toBe( + `${join(root, "src/loader.cjs")}:2 names the file 'config.json', which is not at that path beside the module; the build ships a module's data files by their exact path, so 'private/config.json' does not ship with the Node island. Name the file by its path relative to the module ('private/config.json')`, + ); + expect(collector.isDataFileProblem(problem!)).toBe(true); + expect(collector.tree()!.files.map((file) => file.path)).not.toContain("private/config.json"); }); test("the manifest names relative paths only, and the data is checked against it", () => { diff --git a/packages/compiler/src/node-islands/modules.ts b/packages/compiler/src/node-islands/modules.ts index ae97de055..4f7569de8 100644 --- a/packages/compiler/src/node-islands/modules.ts +++ b/packages/compiler/src/node-islands/modules.ts @@ -40,9 +40,7 @@ import { trackedRealpath, } from "../frontend/input-tracker.js"; import type { IrNodeIslandModules } from "../ir/ir.js"; -import type { ProjectConfig } from "../project-config.js"; import { packageDirOf } from "./origin.js"; -import { globMatch } from "./policy.js"; export type NodeIslandModuleTree = IrNodeIslandModules; type NodeIslandModuleFile = IrNodeIslandModules["files"][number]; @@ -178,6 +176,14 @@ export class NodeIslandModuleCollector { private readonly loaded = new Set(); /** Program modules whose source the program provides (addProvided). */ private readonly provided = new Set(); + /** The problems addNamedData reported (isDataFileProblem). */ + private readonly dataProblems = new Set(); + + /** Whether `problem`, returned by add or addLoaded, is a data file the + * build does not ship (addNamedData) rather than a module. */ + isDataFileProblem(problem: string): boolean { + return this.dataProblems.has(problem); + } /** Collects the module `specifier` names from `caller` (a real path). * Returns null, or the reason the build cannot ship it. */ @@ -326,7 +332,8 @@ export class NodeIslandModuleCollector { if (!TRACED.test(file)) return null; const text = trackedReadFile(file); if (text === null) return `cannot read ${file}`; - this.addNamedData(file, text); + const dataProblem = this.addNamedData(file, text); + if (dataProblem !== null) return dataProblem; const scan = scanModuleSpecifiers(text); if (scan.computed.length > 0) { const first = scan.computed[0]!; @@ -345,37 +352,70 @@ export class NodeIslandModuleCollector { * read to the wasm next to the patching module), and the island runs from * the shipped tree, so a file the module names by its path relative to * the module's directory ships with it (namedDataPaths). Only the exact - * path ships: a file elsewhere in the tree with the same name stays out, - * and so does every file the source does not name; such files ship when - * `scriptc.nodeIslands.files` declares them (addDeclaredFile). Module - * sources ship through the traced walk, never as data. A named path - * stays inside the module's directory: one that leads outside it, - * through `..` or a symbolic link, or into node_modules or .git, is not - * shipped, so nothing from elsewhere on the machine is embedded. */ - private addNamedData(moduleFile: string, text: string): void { + * path ships, and module sources ship through the traced walk, never as + * data. A named path stays inside the module's directory: one that leads + * outside it, through `..` or a symbolic link, or into node_modules or + * .git, is not shipped, so nothing from elsewhere on the machine is + * embedded. + * + * Returns the reason the build cannot ship the module when it names a + * file by a path where there is none while a file of that name is inside + * its directory: the island would read a file the build left out. */ + private addNamedData(moduleFile: string, text: string): string | null { const root = dirname(moduleFile); const inside = (path: string): boolean => { const rel = relative(root, path); return rel !== "" && !rel.startsWith("..") && !isAbsolute(rel); }; - for (const name of namedDataPaths(text)) { + const unresolved: { name: string; at: number }[] = []; + for (const [name, at] of namedDataPaths(text)) { if (isAbsolute(name) || name.includes("\0")) continue; const path = resolve(root, name); if (!inside(path) || path === moduleFile || TRACED.test(path)) continue; const parts = relative(root, path).split(sep); if (parts.includes("node_modules") || parts.includes(".git")) continue; - if (!trackedFileExists(path)) continue; + if (!trackedFileExists(path)) { + if (basename(name).includes(".")) unresolved.push({ name, at }); + continue; + } const target = trackedRealpath(path); if (target === null || !inside(target)) continue; // A link inside the directory stays a link in the tree (placed). this.files.add(this.placed(path)); } - } - - /** A file `scriptc.nodeIslands.files` declares for the island modules of - * the program (declaredIslandFiles): shipped at its path in the tree. */ - addDeclaredFile(file: string): void { - this.files.add(this.placed(file)); + if (unresolved.length === 0) return null; + // The files of the module's directory by name, links leading outside it + // not followed. + const byName = new Map(); + const visited = new Set(); + const walk = (dir: string): void => { + const real = trackedRealpath(dir); + if (real === null || (real !== root && !inside(real)) || visited.has(real)) return; + visited.add(real); + const entries = trackedAccessibleEntries(dir); + if (entries === null) return; + for (const file of entries.files) { + const path = join(dir, file); + if (byName.has(file) || TRACED.test(file) || path === moduleFile) continue; + const target = trackedRealpath(path); + if (target !== null && inside(target)) byName.set(file, path); + } + for (const sub of entries.directories) + if (sub !== "node_modules" && sub !== ".git") walk(join(dir, sub)); + }; + walk(root); + for (const { name, at } of unresolved) { + const found = byName.get(basename(name)); + // A file another named path ships (`"payload.txt"` beside + // `join(here, "fallback", "payload.txt")`) is not missing. + if (found === undefined || this.files.has(trackedRealpath(found) ?? found)) continue; + const line = text.slice(0, at).split("\n").length; + const there = portable(relative(root, found)); + const problem = `${moduleFile}:${line} names the file '${name}', which is not at that path beside the module; the build ships a module's data files by their exact path, so '${there}' does not ship with the Node island. Name the file by its path relative to the module ('${there}')`; + this.dataProblems.add(problem); + return problem; + } + return null; } /** A package: its whole directory, then its dependencies. */ @@ -631,51 +671,24 @@ export interface ModuleSpecifierScan { computed: { at: number; form: string }[]; } -/** The files `scriptc.nodeIslands.files` declares: each path, relative to - * the package.json, and the files under the package.json's directory each - * glob matches (node_modules and .git directories excluded). `missing` - * lists the paths without a file. */ -export function declaredIslandFiles(config: ProjectConfig): { files: string[]; missing: string[] } { - const patterns = config.nodeIslands.files; - if (patterns.length === 0) return { files: [], missing: [] }; - const base = config.manifest === null ? process.cwd() : dirname(config.manifest); - const files = new Set(); - const missing: string[] = []; - const globs: string[] = []; - for (const pattern of patterns) { - const path = resolve(base, pattern); - if (!/[*?]/.test(pattern)) { - if (trackedFileExists(path)) files.add(path); - else missing.push(pattern); - } else globs.push(portable(path)); - } - if (globs.length > 0) { - const walk = (dir: string): void => { - const entries = trackedAccessibleEntries(dir); - if (entries === null) return; - for (const file of entries.files) { - const path = join(dir, file); - if (globs.some((glob) => globMatch(glob, portable(path)))) files.add(path); - } - for (const sub of entries.directories) - if (sub !== "node_modules" && sub !== ".git") walk(join(dir, sub)); - }; - walk(base); - } - return { files: [...files].sort(), missing }; -} - -/** The relative paths a module's source names by string literals: each +/** The relative paths a module's source names by string literals, with the + * offset of the first literal that names each: each * literal (`"data/table.json"`), and the literals passed as consecutive * arguments of one call joined with `/`, as `path.join(here, "fallback", * "payload.txt")` names `fallback/payload.txt`. Comments and template - * literals with substitutions name nothing. */ -export function namedDataPaths(text: string): Set { + * literals with substitutions name nothing; `__dirname + "/data.json"` names + * `data.json`. */ +export function namedDataPaths(text: string): Map { const tokens = moduleTokens(text); - const names = new Set(); - let run: string[] = []; + const names = new Map(); + const add = (name: string, at: number): void => { + if (!names.has(name)) names.set(name, at); + }; + /** The open brackets around the current token. */ + const open: string[] = []; + let run: Token[] = []; const end = (): void => { - if (run.length > 1) names.add(run.join("/")); + if (run.length > 1) add(run.map((t) => t.value).join("/"), run[0]!.at); run = []; }; for (let k = 0; k < tokens.length; k++) { @@ -685,18 +698,26 @@ export function namedDataPaths(text: string): Set { end(); continue; } - names.add(t.value); + add(t.value, t.at); const before = tokens[k - 1]; const after = tokens[k + 1]; + // `__dirname + "/data.json"` names data.json beside the module. + if (before?.kind === "punct" && before.value === "+" && t.value.startsWith("/")) + add(t.value.slice(1), t.at); const argument = + open[open.length - 1] === "(" && before?.kind === "punct" && (before.value === "(" || before.value === ",") && after?.kind === "punct" && (after.value === ")" || after.value === ","); if (!argument) end(); - else run.push(t.value); + else run.push(t); continue; } + if (t.kind === "punct") { + if (t.value === "(" || t.value === "[" || t.value === "{") open.push(t.value); + else if (t.value === ")" || t.value === "]" || t.value === "}") open.pop(); + } if (t.kind !== "punct" || t.value !== ",") end(); } end(); diff --git a/packages/compiler/src/project-config.test.ts b/packages/compiler/src/project-config.test.ts index 7282390ac..ddf93d79d 100644 --- a/packages/compiler/src/project-config.test.ts +++ b/packages/compiler/src/project-config.test.ts @@ -17,7 +17,6 @@ test("a package.json without a scriptc key has the default configuration", () => expect(c.nodeIslands).toEqual({ auto: true, modules: [], - files: [], node: null, nodeVersion: null, packaging: "embedded", @@ -104,7 +103,6 @@ test("nodeIslands settings", () => { nodeIslands: { auto: false, modules: ["src/plugins/*.ts"], - files: ["data/*.json"], node: "/usr/local/bin/node", nodeVersion: ">=24.13.1", packaging: "beside", @@ -117,7 +115,6 @@ test("nodeIslands settings", () => { expect(c.nodeIslands).toEqual({ auto: false, modules: ["src/plugins/*.ts"], - files: ["data/*.json"], node: "/usr/local/bin/node", nodeVersion: ">=24.13.1", packaging: "beside", @@ -136,14 +133,6 @@ test("publicEnv must be an array of variable names", () => { expect(c.nodeIslands.publicEnv).toEqual([]); }); -test("files must be an array of paths or globs", () => { - const c = config({ nodeIslands: { files: "data/config.json" } }); - expect(c.diagnostics.map((d) => d.message)).toEqual([ - `package.json 'scriptc.nodeIslands.files' must be an array of file paths or globs, not "data/config.json"`, - ]); - expect(c.nodeIslands.files).toEqual([]); -}); - test("unknown keys and values are SC8004, located at the key", () => { const text = JSON.stringify( { diff --git a/packages/compiler/src/project-config.ts b/packages/compiler/src/project-config.ts index 653b9678d..9b52993db 100644 --- a/packages/compiler/src/project-config.ts +++ b/packages/compiler/src/project-config.ts @@ -14,8 +14,6 @@ * "zod": { "shapes": false } // no shape-inference overlay * }, * "nodeIslands": { "auto": true, "modules": ["src/plugins/*.ts"] }, - * // files island modules of the program read, shipped with them - * // (node-islands/modules.ts): "nodeIslands": { "files": ["data/*.json"] } * "wasm": ["build/module.wasm"], * "typePatches": { // frontend/type-patches/layer.ts * "commander@^15.0.0": { @@ -62,10 +60,6 @@ export interface NodeIslandsSetting { /** User modules (paths or globs relative to the package.json) that run * in Node. */ modules: string[]; - /** Files (paths or globs relative to the package.json) that ship with - * the island modules of the program, beside the files their source names - * by path (node-islands/modules.ts). */ - files: string[]; /** The `node` executable recorded into the binary. */ node: string | null; /** A semver range checked at the helper handshake. */ @@ -137,7 +131,6 @@ const PACKAGE_SETTING_KEYS = [...Object.keys(PACKAGE_OBJECT_KEYS), "shapes"]; const NODE_ISLANDS_KEYS = [ "auto", "modules", - "files", "node", "nodeVersion", "packaging", @@ -161,7 +154,6 @@ function defaultNodeIslands(): NodeIslandsSetting { return { auto: true, modules: [], - files: [], node: null, nodeVersion: null, packaging: "embedded", @@ -352,22 +344,6 @@ export function parseProjectConfig(manifest: string, text: string): ProjectConfi `write "modules": ["src/plugins/loader.ts"]`, ); } else setting.modules = list; - } else if (key === "files") { - const list: string[] = []; - let ok = Array.isArray(v); - if (Array.isArray(v)) { - for (const entry of v as unknown[]) { - if (typeof entry === "string" && entry !== "") list.push(entry); - else ok = false; - } - } - if (!ok) { - problem( - ["nodeIslands", key], - `must be an array of file paths or globs, not ${shown(v)}`, - `write "files": ["data/config.json"]`, - ); - } else setting.files = list; } else if (key === "publicEnv") { const list: string[] = []; let ok = Array.isArray(v); From 6874c3b5626fa70826c141a508acffc4735d0d8f Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Fri, 30 Oct 2026 00:18:39 -0500 Subject: [PATCH 12/14] Ship a joined island data path when another directory has its name A module that names fallback/payload.txt through join(here, "fallback", "payload.txt") also names "payload.txt" alone. That name was checked against the first file of that name the directory walk found, so a payload.txt in another subdirectory made the build fail with SC8006 even though the joined path ships. A name whose file a named path of the same module ships is no longer reported. --- .../compiler/src/node-islands/modules.test.ts | 15 +++++++++++++++ packages/compiler/src/node-islands/modules.ts | 9 +++++++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/packages/compiler/src/node-islands/modules.test.ts b/packages/compiler/src/node-islands/modules.test.ts index 45769cdbe..381fac346 100644 --- a/packages/compiler/src/node-islands/modules.test.ts +++ b/packages/compiler/src/node-islands/modules.test.ts @@ -334,6 +334,21 @@ describe("the collected module tree", () => { expect(paths.filter((path) => !path.endsWith(".cjs"))).toEqual(["assets/config.json"]); }); + test("a joined data path ships even when another directory has a file of that name", () => { + const root = project({ + "src/main.ts": "", + "src/loader.cjs": + 'const { join } = require("node:path");\nrequire("node:fs").readFileSync(join(__dirname, "fallback", "payload.txt"));\n', + "src/a-other/payload.txt": "not named by the module\n", + "src/fallback/payload.txt": "shipped\n", + }); + const collector = new NodeIslandModuleCollector(); + expect(collector.add(join(root, "src/main.ts"), "./loader.cjs")).toBeNull(); + const paths = collector.tree()!.files.map((file) => file.path); + expect(paths).toContain("fallback/payload.txt"); + expect(paths).not.toContain("a-other/payload.txt"); + }); + test("a named file that is only elsewhere in the module's directory is a build error", () => { const root = project({ "src/main.ts": "", diff --git a/packages/compiler/src/node-islands/modules.ts b/packages/compiler/src/node-islands/modules.ts index 4f7569de8..1e3e43ab5 100644 --- a/packages/compiler/src/node-islands/modules.ts +++ b/packages/compiler/src/node-islands/modules.ts @@ -368,6 +368,8 @@ export class NodeIslandModuleCollector { return rel !== "" && !rel.startsWith("..") && !isAbsolute(rel); }; const unresolved: { name: string; at: number }[] = []; + /** The names of the files this module's named paths ship. */ + const shippedNames = new Set(); for (const [name, at] of namedDataPaths(text)) { if (isAbsolute(name) || name.includes("\0")) continue; const path = resolve(root, name); @@ -382,6 +384,7 @@ export class NodeIslandModuleCollector { if (target === null || !inside(target)) continue; // A link inside the directory stays a link in the tree (placed). this.files.add(this.placed(path)); + shippedNames.add(basename(path)); } if (unresolved.length === 0) return null; // The files of the module's directory by name, links leading outside it @@ -405,9 +408,11 @@ export class NodeIslandModuleCollector { }; walk(root); for (const { name, at } of unresolved) { + // A name of a file another named path ships (`"payload.txt"` beside + // `join(here, "fallback", "payload.txt")`) is not missing, wherever + // other files of that name are. + if (shippedNames.has(basename(name))) continue; const found = byName.get(basename(name)); - // A file another named path ships (`"payload.txt"` beside - // `join(here, "fallback", "payload.txt")`) is not missing. if (found === undefined || this.files.has(trackedRealpath(found) ?? found)) continue; const line = text.slice(0, at).split("\n").length; const there = portable(relative(root, found)); From e4bc919aecc814c0ab628a28ab9d5a2fccfbaf49 Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Fri, 30 Oct 2026 01:18:35 -0500 Subject: [PATCH 13/14] Refuse to evaluate program source in the island deferral probe The build-time probe loads island package modules to decide their deferral. A package can require a file outside node_modules (a program module it shares, or any project file), and that file evaluated on the build machine. The probe's resolve hook now throws for a file outside every node_modules directory before it evaluates, so the module that asked for it stays eager. This also covers a program module passed to the probe directly. --- .../src/node-islands/probe-recorder.test.ts | 74 +++++++++++++++++++ .../src/node-islands/probe-recorder.ts | 14 +++- 2 files changed, 86 insertions(+), 2 deletions(-) create mode 100644 packages/compiler/src/node-islands/probe-recorder.test.ts diff --git a/packages/compiler/src/node-islands/probe-recorder.test.ts b/packages/compiler/src/node-islands/probe-recorder.test.ts new file mode 100644 index 000000000..2e82be3e9 --- /dev/null +++ b/packages/compiler/src/node-islands/probe-recorder.test.ts @@ -0,0 +1,74 @@ +import { spawnSync } from "node:child_process"; +import { existsSync, mkdirSync, mkdtempSync, realpathSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import { afterEach, expect, test } from "vitest"; +import { PROBE_RECORDER } from "./probe-recorder.js"; + +const made: string[] = []; +afterEach(() => { + for (const dir of made.splice(0)) rmSync(dir, { recursive: true, force: true }); +}); + +function project(files: Record): string { + const root = realpathSync(mkdtempSync(join(tmpdir(), "scriptc-probe-recorder-"))); + made.push(root); + for (const [path, text] of Object.entries(files)) { + mkdirSync(dirname(join(root, path)), { recursive: true }); + writeFileSync(join(root, path), text); + } + return root; +} + +interface ProbeResult { + path: string; + throws?: string; + root?: number; +} + +/** Runs the recorder in a Node process of its own, as the probe host runs + * it in libnode, over `modules`. */ +function probe(root: string, modules: string[]): ProbeResult[] { + writeFileSync(join(root, "recorder.cjs"), PROBE_RECORDER); + writeFileSync( + join(root, "run.cjs"), + `const out = require("./recorder.cjs").probe({ modules: ${JSON.stringify(modules)} });\n` + + `process.stdout.write(JSON.stringify(out.results));\n`, + ); + const run = spawnSync(process.execPath, [join(root, "run.cjs")], { cwd: root, encoding: "utf8" }); + expect(run.status, run.stderr).toBe(0); + return JSON.parse(run.stdout) as ProbeResult[]; +} + +// A source file that, when it evaluates, writes `evaluated-` beside itself. +const marker = (name: string): string => + `require("fs").writeFileSync(require("path").join(__dirname, "evaluated-${name}"), "");\nmodule.exports = { n: 1 };\n`; + +test("the probe evaluates an installed package", () => { + const root = project({ + "node_modules/pkg/package.json": '{ "name": "pkg" }', + "node_modules/pkg/index.js": "module.exports = { answer: 42 };\n", + }); + const [result] = probe(root, [join(root, "node_modules/pkg/index.js")]); + expect(result!.throws).toBeUndefined(); + expect(result!.root).toBeDefined(); +}); + +test("the probe never evaluates the program's own source", () => { + const root = project({ + "src/program.js": marker("program"), + "src/shared.js": marker("shared"), + "node_modules/reaches/package.json": '{ "name": "reaches" }', + "node_modules/reaches/index.js": 'module.exports = require("../../src/shared.js");\n', + }); + const results = probe(root, [ + join(root, "src/program.js"), + join(root, "node_modules/reaches/index.js"), + ]); + expect(results.map((r) => r.throws)).toEqual([ + `the build does not evaluate ${join(root, "src/program.js")}, a file of the program`, + `the build does not evaluate ${join(root, "src/shared.js")}, a file of the program`, + ]); + expect(existsSync(join(root, "src/evaluated-program"))).toBe(false); + expect(existsSync(join(root, "src/evaluated-shared"))).toBe(false); +}); diff --git a/packages/compiler/src/node-islands/probe-recorder.ts b/packages/compiler/src/node-islands/probe-recorder.ts index 499de3593..af29be084 100644 --- a/packages/compiler/src/node-islands/probe-recorder.ts +++ b/packages/compiler/src/node-islands/probe-recorder.ts @@ -20,6 +20,8 @@ * (island-local, since native code has its own builtins, but * observable by other island modules evaluated out of order); * - every file the load required (the module's closure), and a throw; + * a load that resolves a file outside every node_modules directory (the + * program's own source) throws before that file evaluates; * * and takes the interface snapshot of the module's value (snapshot.ts). * Calls made by Node's own internals (the module loader reading files, @@ -429,11 +431,19 @@ exports.probe = function probe(input) { undo.push(() => hook.disable()); // The closure: a resolve hook (no frame of ours stays on the stack while // modules evaluate, so Node's node_modules checks see the real callers). + // The probe evaluates installed packages only: a file outside every + // node_modules directory is the program's own source (a program module, + // or a project file a package requires), and its load throws before the + // file evaluates, so the module that asked for it stays eager. const hooks = Module.registerHooks({ resolve(specifier, context, nextResolve) { const result = nextResolve(specifier, context); - if (recording && typeof result.url === "string" && result.url.startsWith("file:")) - files.add(require("node:url").fileURLToPath(result.url)); + if (recording && typeof result.url === "string" && result.url.startsWith("file:")) { + const file = require("node:url").fileURLToPath(result.url); + if (!/[\\/]node_modules[\\/]/.test(file)) + throw new Error("the build does not evaluate " + file + ", a file of the program"); + files.add(file); + } return result; }, }); From 1ec321ed77de89726e33c810b324aee0a8b4e20a Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Fri, 30 Oct 2026 01:30:27 -0500 Subject: [PATCH 14/14] Keep a probe load eager when it catches the refusal of a program file A package that catches the probe's refusal to load a program file evaluated a fallback, and the probe recorded that load as clean and snapshotted the fallback's exports, which the executable never has. A load in which the resolve hook refused a program file is now recorded as throwing the refusal whether or not the load caught it, and so is a later load that reaches one of its files through the module cache. --- .../src/node-islands/probe-recorder.test.ts | 22 +++++++++++++++++++ .../src/node-islands/probe-recorder.ts | 22 +++++++++++++++++-- 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/packages/compiler/src/node-islands/probe-recorder.test.ts b/packages/compiler/src/node-islands/probe-recorder.test.ts index 2e82be3e9..f67522a38 100644 --- a/packages/compiler/src/node-islands/probe-recorder.test.ts +++ b/packages/compiler/src/node-islands/probe-recorder.test.ts @@ -72,3 +72,25 @@ test("the probe never evaluates the program's own source", () => { expect(existsSync(join(root, "src/evaluated-program"))).toBe(false); expect(existsSync(join(root, "src/evaluated-shared"))).toBe(false); }); + +test("a package that catches the refusal does not defer, nor does one that loads it later", () => { + const root = project({ + "src/config.js": marker("config"), + "node_modules/catches/package.json": '{ "name": "catches" }', + "node_modules/catches/index.js": + 'let config;\ntry { config = require("../../src/config.js"); } catch { config = { fallback: true }; }\nmodule.exports = config;\n', + "node_modules/later/package.json": '{ "name": "later" }', + "node_modules/later/index.js": 'module.exports = { config: require("catches") };\n', + "node_modules/clean/package.json": '{ "name": "clean" }', + "node_modules/clean/index.js": "module.exports = { ok: true };\n", + }); + const results = probe(root, [ + join(root, "node_modules/catches/index.js"), + join(root, "node_modules/later/index.js"), + join(root, "node_modules/clean/index.js"), + ]); + const refusal = `the build does not evaluate ${join(root, "src/config.js")}, a file of the program`; + expect(results.map((r) => r.throws)).toEqual([refusal, refusal, undefined]); + expect(results.map((r) => r.root === undefined)).toEqual([true, true, false]); + expect(existsSync(join(root, "src/evaluated-config"))).toBe(false); +}); diff --git a/packages/compiler/src/node-islands/probe-recorder.ts b/packages/compiler/src/node-islands/probe-recorder.ts index af29be084..33a357b43 100644 --- a/packages/compiler/src/node-islands/probe-recorder.ts +++ b/packages/compiler/src/node-islands/probe-recorder.ts @@ -21,7 +21,8 @@ * observable by other island modules evaluated out of order); * - every file the load required (the module's closure), and a throw; * a load that resolves a file outside every node_modules directory (the - * program's own source) throws before that file evaluates; + * program's own source) throws before that file evaluates, and its + * record is that throw even when the load catches it; * * and takes the interface snapshot of the module's value (snapshot.ts). * Calls made by Node's own internals (the module loader reading files, @@ -190,6 +191,11 @@ exports.probe = function probe(input) { const inputs = [], volatile = [], effects = [], tty = new Map(), envKeys = new Map(); const files = new Set(); let recording = false; + // The first program file the current load asked for (the resolve hook), + // and, for every file a load with a refusal evaluated, that program file: + // the module cache keeps what such a file exported without it. + let refused = null; + const refusedBy = new Map(); const note = (list, what) => { if (recording && !internalCaller()) list.push(list === volatile ? what : what + " (" + lastCaller + ")"); }; @@ -440,8 +446,10 @@ exports.probe = function probe(input) { const result = nextResolve(specifier, context); if (recording && typeof result.url === "string" && result.url.startsWith("file:")) { const file = require("node:url").fileURLToPath(result.url); - if (!/[\\/]node_modules[\\/]/.test(file)) + if (!/[\\/]node_modules[\\/]/.test(file)) { + if (refused === null) refused = file; throw new Error("the build does not evaluate " + file + ", a file of the program"); + } files.add(file); } return result; @@ -511,6 +519,7 @@ exports.probe = function probe(input) { pendingTimers.clear(); loadPromises.clear(); tty.clear(); envKeys.clear(); files.clear(); + refused = null; files.add(path); currentFiles = files; currentPackageDir = null; @@ -522,6 +531,15 @@ exports.probe = function probe(input) { recording = true; try { exportsValue = createRequire(path)(path); } catch (e) { threw = String((e && e.message) || e); } recording = false; + // A load that catches the refusal evaluates a fallback the executable + // never runs: its exports are not the module's, and neither are those of + // a later load that reaches one of its files through the module cache. + if (refused === null) + for (const f of files) if (refusedBy.has(f)) { refused = refusedBy.get(f); break; } + if (refused !== null) { + threw = "the build does not evaluate " + refused + ", a file of the program"; + for (const f of files) if (!refusedBy.has(f)) refusedBy.set(f, refused); + } const after = shot(); const root = threw === undefined ? value(exportsValue, 0) : undefined; const mutations = [];