From a4399d62264c0db296088786909d676f6ea721db Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Wed, 28 Oct 2026 13:53:13 -0700 Subject: [PATCH 01/13] 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 404648051ee..29a34660d68 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 a5f4fdbe2c8..a1b2f071ff0 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 604af05b9ca..491cc5fa251 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 a85e17f370e..fc6484438db 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 00299ce580a..6091b8ce4e7 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 dc49bb99fb7..742b47d4bb9 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 ea0cfb702ef..ecae1789467 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 ab3e5cc8343..cbd1fb23871 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 34ea20b28d9..927b56e687d 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 be43393912d..bd317e23fae 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 72dbf3cd045..945bdfd220f 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 390e4c8036f145562888578368e007c256edfc77 Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Wed, 28 Oct 2026 13:53:24 -0700 Subject: [PATCH 02/13] 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 2a21395da52..72cbae9c792 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 f612d58e56a..e0c341bd525 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 25f6863140c..b074ac5bc48 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 0ac082e7325..70d7355fcc0 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 d17ce31a5256f9cb6bdb753a13fe13d7471fb2c0 Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Wed, 28 Oct 2026 13:53:32 -0700 Subject: [PATCH 03/13] 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 fc6484438db..2d1e30fbcf0 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 b46b7167889d1c68adb708e3098ca359bdbb0d33 Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Wed, 28 Oct 2026 13:54:08 -0700 Subject: [PATCH 04/13] 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 00000000000..c6b9ad9a050 --- /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 00000000000..c10f1a5fec6 --- /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 00000000000..fc8715ea9c1 --- /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 00000000000..ea5f8598e71 --- /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 00000000000..2ea0f42397d --- /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 00000000000..96038479e91 --- /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 00000000000..5bbefffbabe --- /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 00000000000..6121eb5f420 --- /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 00000000000..fd11d7850b3 --- /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 00000000000..5bbefffbabe --- /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 00000000000..c3a1cd8d65f --- /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 00000000000..5bbefffbabe --- /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 00000000000..1f41f7275bf --- /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 00000000000..4967bd23d57 --- /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 00000000000..5bbefffbabe --- /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 00000000000..4b718bafd57 --- /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 00000000000..5adfa8964b6 --- /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 00000000000..224d7a7b0fb --- /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 00000000000..50ce59ee05a --- /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 00000000000..244011af6cf --- /dev/null +++ b/tests/harness/corpus-costs/8426-sloppy-this-island/main.js.json @@ -0,0 +1 @@ +{ "plain": 1411, "san": 1828 } From 60113fc360cd169175178b08902e7c5ba81554cc Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Wed, 28 Oct 2026 14:01:31 -0700 Subject: [PATCH 05/13] 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 3873d1c348a..46a0b5fa051 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 79652d867b2..523daa6c895 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 2ae94454d36..bfaf1dcd013 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 40e09734a50..ab47fc96f08 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 2ad02100bc7..74a826030c9 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 46c601b7cd5..f4d8eb32c1f 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 c410b2490b67b944960493a97e9b8754ac7dfd00 Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Wed, 28 Oct 2026 19:49:42 -0700 Subject: [PATCH 06/13] 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 a1b2f071ff0..f03a3087497 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 2d1e30fbcf0..fd71975c133 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 72cbae9c792..52b577da860 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 e0c341bd525..926e1d6b815 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 2c0d8ec734d..8f443a5b57e 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 7fc5c46d763..ead084088d6 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 d4221d31f1c..0b743ddc388 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 b074ac5bc48..a1967e5b5d8 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 70d7355fcc0..07c43b93ec3 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 8108d985c0a..c7339956149 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 00000000000..4e8c61b45be --- /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 00000000000..12765200f73 --- /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 00000000000..057938d1240 --- /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 00000000000..56ad94ea388 --- /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 00000000000..861ddd786b6 --- /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 00000000000..cd25d1010aa --- /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 00000000000..78905724cbb --- /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 00000000000..ebbfc464b68 --- /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 00000000000..6fb6039dd3a --- /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 215d75384fb..326f6530a84 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 90ebc6ddd4442b50817153dad853d4befea0f78c Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Thu, 29 Oct 2026 18:11:42 -0700 Subject: [PATCH 07/13] 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 c6b9ad9a050..cb38c0ce62e 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 240d75f3a77c9c131772a8008241b6275312f2bc Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Wed, 28 Oct 2026 22:08:22 -0700 Subject: [PATCH 08/13] Name the Node island reasons the pi census refusals carry The pi re-measure (CENSUS-2026-10-29-int3) lists 27 refusals under its Node island class. Read as reasons, they split into two very different groups: five are needs of the JavaScript runtime itself (dynamic module loading, a worker entry path, a host class base, a require() the runtime owns), which an island can serve, and the rest are compiler gaps or another family's work. Without a reason id the report cannot tell them apart and counts every one as a gap. The refusal sites that predate reason ids now map to their rule: 'import.meta.resolve with a runtime-computed specifier' and 'this Worker entry point' are dynamic-resolve and worker-entry (both SC2020 in the census, SC1090 elsewhere, so a shape may name both codes), the CJS namespace import of a program module is cjs-namespace, and 'extending classes not declared in the program' is host-class-base (R1.6). The test table pins the exact census message shapes, taken from results/pi-run/cov/build-rem3-2026-10-29.txt. Co-Authored-By: Claude Opus 5.5 --- .../src/node-islands/classify.test.ts | 82 +++++++++++++++++++ .../compiler/src/node-islands/classify.ts | 35 +++++++- packages/compiler/src/node-islands/rules.ts | 4 + packages/compiler/src/node-islands/summary.ts | 2 + 4 files changed, 119 insertions(+), 4 deletions(-) create mode 100644 packages/compiler/src/node-islands/classify.test.ts diff --git a/packages/compiler/src/node-islands/classify.test.ts b/packages/compiler/src/node-islands/classify.test.ts new file mode 100644 index 00000000000..b89129321db --- /dev/null +++ b/packages/compiler/src/node-islands/classify.test.ts @@ -0,0 +1,82 @@ +/* Reason ids for refusal messages that carry none (DESIGN.md §1.3): the + * shapes a program meets in the wild decide whether a module may run in a + * Node island, so the table keeps them mapped to their rule. */ +import { describe, expect, test } from "vitest"; +import type { ScrDiagnostic } from "../diagnostics/diagnostic.js"; +import { classifyDiagnostic } from "./classify.js"; + +type Code = ScrDiagnostic["code"]; + +function refusal(code: Code, message: string): ScrDiagnostic { + return { code, message, loc: { file: "/app/src/module.ts", start: 0, end: 0 } }; +} + +const CASES: { reason: string; rule: string; code: Code; message: string }[] = [ + { + reason: "cjs-namespace", + rule: "R1.2", + code: "SC1090", + message: + "dynamic import of the program's own CommonJS module 'highlight.js/lib/index.js' (its namespace comes from module.exports through Node's CJS lexer, which has no compiled story — require it, or import it statically) is not supported yet", + }, + { + reason: "dynamic-import", + rule: "R1.2", + code: "SC1090", + message: + "dynamic import() of computed specifiers (the module graph resolves at build time — the specifier must be a string literal) are not supported yet", + }, + { + reason: "dynamic-resolve", + rule: "R1.2", + code: "SC2020", + message: + "'import.meta.resolve with a runtime-computed specifier' is part of the standard library types but has no scriptc lowering yet", + }, + { + reason: "worker-entry", + rule: "R1.2", + code: "SC2020", + message: + "'this Worker entry point' is part of the standard library types but has no scriptc lowering yet", + }, + { + reason: "host-class-base", + rule: "R1.6", + code: "SC1090", + message: "extending classes not declared in the program ('WebSocket') is not supported yet", + }, + { + reason: "dynamic-require", + rule: "R1.2", + code: "SC2020", + message: "'require' is typed by @types/node but has no scriptc lowering yet", + }, + { + reason: "dynamic-require", + rule: "R1.2", + code: "SC2020", + message: + "'createRequire's require with a computed specifier or this argument shape' is part of the standard library types but has no scriptc lowering yet", + }, +]; + +describe("refusal messages", () => { + for (const c of CASES) { + test(c.reason, () => { + const reason = classifyDiagnostic(refusal(c.code, c.message), "refusal"); + expect(reason.reason).toBe(c.reason); + expect(reason.rule).toBe(c.rule); + expect(reason.class).toBe("runtime"); + }); + } + + test("a construct the runtime cannot help with stays a gap", () => { + const reason = classifyDiagnostic( + refusal("SC1090", "union discriminant reads are not supported yet"), + "refusal", + ); + expect(reason.class).toBe("gap"); + expect(reason.rule).toBeNull(); + }); +}); diff --git a/packages/compiler/src/node-islands/classify.ts b/packages/compiler/src/node-islands/classify.ts index b08ac63aaee..00a7db1759a 100644 --- a/packages/compiler/src/node-islands/classify.ts +++ b/packages/compiler/src/node-islands/classify.ts @@ -11,18 +11,44 @@ import { classOfRule, ruleInfo, ruleOfReason } from "./rules.js"; import type { RefusalRecord } from "./scan.js"; import type { NodeIslandReason, NodeIslandReasonSource } from "./summary.js"; -/** Message shapes of refusal sites that predate reason ids. */ -const MESSAGE_REASONS: { code: string; pattern: RegExp; reason: string }[] = [ +/** Message shapes of refusal sites that predate reason ids. A shape that + * one refusal site reports as SC1090 and another as SC2020 names both. */ +const MESSAGE_REASONS: { code: string | readonly string[]; pattern: RegExp; reason: string }[] = [ { code: "SC2010", pattern: /./, reason: "code-from-string" }, { code: "SC1010", pattern: /'(?:node:)?vm'/, reason: "vm" }, { code: "SC1090", pattern: /^dynamic import\(\) of computed/, reason: "dynamic-import" }, + { + code: "SC1090", + pattern: /^dynamic import of the program's own CommonJS module /, + reason: "cjs-namespace", + }, { code: "SC1090", pattern: /require\(\) with a computed specifier/, reason: "dynamic-require" }, + { + code: ["SC1090", "SC2020"], + pattern: /^'import\.meta\.resolve with a runtime-computed specifier'/, + reason: "dynamic-resolve", + }, + { + code: ["SC1090", "SC2020"], + pattern: /^'?this Worker entry point/, + reason: "worker-entry", + }, + { + code: "SC1090", + pattern: /^extending classes not declared in the program/, + reason: "host-class-base", + }, { code: "SC1090", pattern: /'with' statement/, reason: "with" }, { code: "SC1090", pattern: /sloppy-mode parameter aliases/, reason: "sloppy-arguments" }, { code: "SC2020", pattern: /^'(?:eval|Function)'/, reason: "code-from-string" }, { code: "SC2020", - pattern: /createRequire's require with this argument shape/, + pattern: /createRequire's require with (?:a computed specifier or )?this argument shape/, + reason: "dynamic-require", + }, + { + code: "SC2020", + pattern: /^'require' is typed by @types\/node/, reason: "dynamic-require", }, { code: "SC2020", pattern: /^native addon /, reason: "native-addon" }, @@ -43,7 +69,8 @@ const MESSAGE_REASONS: { code: string; pattern: RegExp; reason: string }[] = [ function reasonIdOf(d: ScrDiagnostic): string | null { if (d.reason !== undefined) return d.reason; for (const entry of MESSAGE_REASONS) { - if (entry.code === d.code && entry.pattern.test(d.message)) return entry.reason; + const codes = typeof entry.code === "string" ? [entry.code] : entry.code; + if (codes.includes(d.code) && entry.pattern.test(d.message)) return entry.reason; } return null; } diff --git a/packages/compiler/src/node-islands/rules.ts b/packages/compiler/src/node-islands/rules.ts index a1967e5b5d8..3dbfd561111 100644 --- a/packages/compiler/src/node-islands/rules.ts +++ b/packages/compiler/src/node-islands/rules.ts @@ -58,11 +58,15 @@ const REASON_RULES: Record = { vm: "R1.1", "dynamic-require": "R1.2", "dynamic-import": "R1.2", + "dynamic-resolve": "R1.2", + "worker-entry": "R1.2", + "cjs-namespace": "R1.2", "unresolved-import": "R1.2", "native-addon": "R1.3", "loader-hook": "R1.4", "engine-internal": "R1.5", "weak-ref": "R1.6", + "host-class-base": "R1.6", "constructor-function": "R2.1", "prototype-write": "R2.2", "define-property": "R2.3", diff --git a/packages/compiler/src/node-islands/summary.ts b/packages/compiler/src/node-islands/summary.ts index c7339956149..035732fcab6 100644 --- a/packages/compiler/src/node-islands/summary.ts +++ b/packages/compiler/src/node-islands/summary.ts @@ -82,6 +82,8 @@ export interface NodeIslandReport { * 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", + "host-class-base": + "it extends a class only the JavaScript runtime has, and native code cannot inherit it", }; /** "R2.1 constructor functions: 'new Foo()' of a function" or From 4757d21c1ad4be83b69c8e08cf53b19e32a115f7 Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Wed, 28 Oct 2026 22:08:30 -0700 Subject: [PATCH 09/13] Report an untraceable island load as SC8006, never as a crash The island module tree refuses a program module whose loads it cannot trace: the module runs in Node from the shipped tree, where nothing else provides what it loads, so a computed import() or require() would fail there where the native build ran. The refusal pushes SC8006 and throws PoisonError, which a statement fence around a module body catches. Collection runs before those fences: collectIslandImport and collectIslandRequires are called from collectNpmImports, outside any fence, so the same refusal escaped as an uncaught PoisonError and the CLI died instead of reporting. A module that patches a builtin (H3) and loads a computed specifier is exactly that shape, and it is the first thing a migration of the pi census sites meets. The two collection entry points now catch their own PoisonError: the diagnostic the loader pushed is the build's answer. Co-Authored-By: Claude Opus 5.5 --- .../src/frontend/lowering/lower-modules.ts | 44 +++++++++++++++++-- tests/harness/node-island.test.ts | 32 ++++++++++++++ 2 files changed, 72 insertions(+), 4 deletions(-) diff --git a/packages/compiler/src/frontend/lowering/lower-modules.ts b/packages/compiler/src/frontend/lowering/lower-modules.ts index 6091b8ce4e7..1cc10f28cb5 100644 --- a/packages/compiler/src/frontend/lowering/lower-modules.ts +++ b/packages/compiler/src/frontend/lowering/lower-modules.ts @@ -618,6 +618,37 @@ export function collectProgram(lowerer: Lowerer, parts: FileParts[]): void { markRequireEarlyBindings(lowerer); } +/** collectIslandImport inside the collection pass. A module the island's + * shipped tree cannot carry (a computed load, an unshippable resolution) + * has its SC8006 pushed and then throws PoisonError, which a statement + * fence around a body would catch. Collection runs outside those fences, so + * the throw is caught here: the diagnostic the loader pushed is the build's + * answer, and the import stays an island load of the module. */ +function collectIslandImportAtCollection( + lowerer: Lowerer, + sf: ts.SourceFile, + stmt: ts.Statement, + spec: string, +): boolean { + try { + return collectIslandImport(lowerer, sf, stmt, spec); + } catch (error) { + if (!(error instanceof PoisonError)) throw error; + return true; + } +} + +/** collectIslandRequires inside the collection pass, for the same reason as + * collectIslandImportAtCollection: a module the shipped tree cannot carry + * reports its SC8006 and throws, and the diagnostic is the build's answer. */ +function collectIslandRequiresAtCollection(lowerer: Lowerer, sf: ts.SourceFile): void { + try { + collectIslandRequires(lowerer, sf); + } catch (error) { + if (!(error instanceof PoisonError)) throw error; + } +} + export function collectNpmImports(lowerer: Lowerer, parts: FileParts[]): void { const entryPackageFile = entryPackageFilePredicate(lowerer.entry.fileName); for (const fp of parts) { @@ -668,14 +699,19 @@ export function collectNpmImports(lowerer: Lowerer, parts: FileParts[]): void { // 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; + if ( + lowerer.nodeIslands !== null && + collectIslandImportAtCollection(lowerer, fp.sf, stmt, spec) + ) + continue; const projectDep = isRelativeSpecifier(spec) ? null : resolveImport(lowerer.program, fp.sf, spec); if (projectDep !== null) { // A Node island package that tsconfig `paths` maps to its installed // directory resolves to its declarations: it loads in Node. - if (projectDep.isDeclarationFile) collectIslandImport(lowerer, fp.sf, stmt, spec); + if (projectDep.isDeclarationFile) + collectIslandImportAtCollection(lowerer, fp.sf, stmt, spec); continue; } const npm = resolveNpmImport(fp.sf.fileName, spec); @@ -729,7 +765,7 @@ export function collectNpmImports(lowerer: Lowerer, parts: FileParts[]): void { if (npm !== null && probeNodeImportRefusal(fp.sf.fileName, spec) !== null) continue; // A Node island package (scope-B NI-4): loaded in Node at this // module's position, its bindings are island values. - if (collectIslandImport(lowerer, fp.sf, stmt, spec)) continue; + if (collectIslandImportAtCollection(lowerer, fp.sf, stmt, spec)) continue; { lowerer.pushDiag(unsupportedPackageImportDiag(npm?.packageName ?? relPkg!, locOf(stmt))); continue; @@ -738,7 +774,7 @@ export function collectNpmImports(lowerer: Lowerer, parts: FileParts[]): void { } for (const fp of parts) { collectDynamicImports(lowerer, fp.sf); - collectIslandRequires(lowerer, fp.sf); + collectIslandRequiresAtCollection(lowerer, fp.sf); } } diff --git a/tests/harness/node-island.test.ts b/tests/harness/node-island.test.ts index 326f6530a84..e00211aa5ac 100644 --- a/tests/harness/node-island.test.ts +++ b/tests/harness/node-island.test.ts @@ -501,6 +501,38 @@ export const patched = Kind.A; expect(stripped).toBeDefined(); expect(stripped).toContain("enum"); }); + + // The module tree refuses a program module whose loads it cannot trace + // (they run in Node from the shipped tree, where nothing else provides + // them). Collection runs before any statement fence, so the refusal has + // to be a diagnostic — never a thrown PoisonError reaching the CLI. + test("a load the island tree cannot trace is SC8006", async () => { + const dir = copyCorpus( + "computed", + `import { loadPayload } from "./loader.ts"; +console.log(await loadPayload("payload-reader")); +`, + { + "loader.ts": `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; +export async function loadPayload(name: string): Promise { + return import(name); +} +`, + }, + ); + const result = await buildDir(dir, "main"); + expect(result.ok).toBe(false); + const untraced = messages(result).find( + (m) => m.startsWith("SC8006:") && m.includes("string-literal specifiers only"), + ); + expect(untraced).toBeDefined(); + expect(untraced).toContain("loader.ts"); + expect(untraced).toContain("import() with a computed specifier"); + }); + }); }); }); From f3758b3afb17e61f1315492159b0fce6fac5a517 Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Wed, 28 Oct 2026 22:08:46 -0700 Subject: [PATCH 10/13] Run a module that needs a host class in a Node island A user TypeScript module that extends a class only the JavaScript runtime has ('extending classes not declared in the program') needs the runtime itself: the base class exists in Node, and native code cannot inherit it. The island module tree carries such a module as written, so it is the one runtime need an automatic island satisfies today, and the pi census's openai-codex-responses site is exactly it. Under --node-islands=auto, loweringModules now grows an island module for that reason, as it already does for sloppy-only JavaScript modules. A TypeScript module joins only when Node can run it as written: the scan's stripProblem check refuses a file that needs a transform or resolves a relative import without its extension, and the refusal then stays the build's answer instead of a broken island. A load the shipped tree cannot trace stays refused too (SC8006), so nothing diverges silently. The other refusal reasons an island could serve are deliberately not grown: a computed specifier, a worker entry path or a program-owned CJS namespace needs the tree to ship what the module loads, which is a shipping decision, not a lowering one. Co-Authored-By: Claude Opus 5.5 --- packages/compiler/src/executable/prepare.ts | 65 +++++++-- packages/compiler/src/frontend/pipeline.ts | 9 +- packages/compiler/src/node-islands/scan.ts | 17 ++- .../6900-node-island-host-class/main.ts.json | 7 + .../6900-node-island-host-class/globals.d.ts | 12 ++ .../6900-node-island-host-class/main.ts | 7 + .../6900-node-island-host-class/package.json | 1 + .../6900-node-island-host-class/socket.ts | 12 ++ .../6900-node-island-host-class/main.ts.json | 1 + tests/harness/node-island.test.ts | 132 +++++++++++++++--- 10 files changed, 231 insertions(+), 32 deletions(-) create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6900-node-island-host-class/main.ts.json create mode 100644 tests/corpus/6900-node-island-host-class/globals.d.ts create mode 100644 tests/corpus/6900-node-island-host-class/main.ts create mode 100644 tests/corpus/6900-node-island-host-class/package.json create mode 100644 tests/corpus/6900-node-island-host-class/socket.ts create mode 100644 tests/harness/corpus-costs/6900-node-island-host-class/main.ts.json diff --git a/packages/compiler/src/executable/prepare.ts b/packages/compiler/src/executable/prepare.ts index fd71975c133..824c3dd4bed 100644 --- a/packages/compiler/src/executable/prepare.ts +++ b/packages/compiler/src/executable/prepare.ts @@ -42,7 +42,7 @@ import { isIslandReasonCode, } from "../node-islands/classify.js"; import { packageDirOf, packageVersionOf } from "../node-islands/origin.js"; -import { describe } from "../node-islands/summary.js"; +import { describe, type NodeIslandReason } 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"; @@ -194,7 +194,13 @@ export function prepareExecutableModule( // 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); + const grownModules = loweringModules( + entryPath, + opts, + lowered.diagnostics, + lowerModules, + (files) => fe.nodeIslandScan?.({ userJs: false, stripFor: files }) ?? null, + ); if (grown !== null || grownModules !== null) { fe.dispose(); frontendDisposed(); @@ -379,6 +385,14 @@ const SLOPPY_REASONS: ReadonlySet = new Set([ * builtin, so it runs in Node, where an island consumer sees it. */ const PATCH_REASONS: ReadonlySet = new Set(["builtin-patch"]); +/** The runtime reasons a TypeScript module turns into an island module for: + * the JavaScript runtime owns the construct's base class, so the module + * needs nothing shipped beside it. A load only the runtime can resolve (a + * computed specifier, a worker entry path) stays refused — the shipped tree + * cannot carry what the module loads, and an island would fail where the + * native build ran. */ +const ISLAND_RUN_REASONS: ReadonlySet = new Set(["host-class-base"]); + /** 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 @@ -744,27 +758,58 @@ function loweringModules( opts: CompileRequestOptions, diagnostics: readonly ScrDiagnostic[], known: ReadonlyMap, + scanForRun: (files: readonly string[]) => NodeIslandScan | null, ): Map | null { const policy = nodeIslandPolicy(readProjectConfig(entryPath), opts.nodeIslands); if (policy.mode !== "auto" || known.size > 16) return null; const grown = new Map(known); + // TypeScript modules the refusals name, with the reason that names them + // (a module joins once, with its first reason). + const runtimeReasons = new Map(); 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; + if (sameFile(file, entryPath) || grown.has(file) || runtimeReasons.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", - }); + if (isJsSourceFileName(file)) { + grown.set(file, { + file, + class: reason.class, + reason: describe(reason), + decision: "auto", + }); + continue; + } + // A TypeScript module runs in Node only for a need of the JavaScript + // runtime itself that the shipped tree satisfies (ISLAND_RUN_REASONS): + // Node then does what the native pipeline could not. An object-model + // need is a JavaScript module's sloppy semantics, and a refusal caused + // by another declaration is that declaration's cascade. + if (!ISLAND_RUN_REASONS.has(reason.reason) || d.cause !== undefined) continue; + if (!/\.(?:ts|mts|cts)$/.test(file) || /\.d\.[cm]?ts$/.test(file)) continue; + runtimeReasons.set(file, reason); + } + if (runtimeReasons.size > 0) { + const scan = scanForRun([...runtimeReasons.keys()]); + for (const [file, reason] of runtimeReasons) { + const module = scan?.modules.find((m) => m.file === file); + // Node runs TypeScript by erasing types in place: a module that needs + // a transform, or resolves a relative import only without its + // extension, cannot run in an island as written, and its refusal + // stays the build's answer. + if (module === undefined || module.stripProblem !== null) continue; + grown.set(file, { + file, + class: reason.class, + reason: describe(reason), + decision: "auto", + }); + } } return grown.size === known.size ? null : grown; } diff --git a/packages/compiler/src/frontend/pipeline.ts b/packages/compiler/src/frontend/pipeline.ts index cbd1fb23871..c27aad6a26c 100644 --- a/packages/compiler/src/frontend/pipeline.ts +++ b/packages/compiler/src/frontend/pipeline.ts @@ -131,7 +131,9 @@ export interface Frontend { /** The Node island detector's input (report-only): the program's * modules with their pre-scan markers, package import edges, and how * each refused package was refused. Call before dispose. */ - nodeIslandScan?: ((opts: { userJs: boolean }) => NodeIslandScan) | undefined; + nodeIslandScan?: + | ((opts: { userJs: boolean; stripFor?: readonly string[] | undefined }) => NodeIslandScan) + | undefined; /** Diagnostics that do not fail a build: unknown settings in the * project's scriptc configuration (SC8004), which builds do not use. */ warnings?: ScrDiagnostic[]; @@ -1120,7 +1122,7 @@ function loadFrontend( npmImportSites: npmSites, nodeIslandScan: !scanIslands ? undefined - : ({ userJs }) => { + : ({ userJs, stripFor }) => { const files = new Set([finalLoad.entry, ...finalLoad.moduleOrder]); addDynamicImportGraph(finalLoad, files); // Package edges matter only when the program has npm packages: one @@ -1133,6 +1135,9 @@ function loadFrontend( userJs, dropped: false, edges: npm, + ...(stripFor !== undefined && stripFor.length > 0 + ? { stripFor: new Set(stripFor) } + : {}), }); return { modules: [...droppedScan.modules, ...scanned.modules], diff --git a/packages/compiler/src/node-islands/scan.ts b/packages/compiler/src/node-islands/scan.ts index 07c43b93ec3..3de30bdbd66 100644 --- a/packages/compiler/src/node-islands/scan.ts +++ b/packages/compiler/src/node-islands/scan.ts @@ -140,11 +140,18 @@ function sloppyParsable(program: ts.Program, sf: ts.SourceFile): boolean { /** 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). */ + * no npm package has none to find). `stripFor` names user TypeScript modules + * a build is considering for an island: their `stripProblem` is computed so + * the build can refuse a file Node cannot run as written. */ export function scanFiles( program: ts.Program, files: readonly ts.SourceFile[], - opts: { userJs: boolean; dropped: boolean; edges: boolean }, + opts: { + userJs: boolean; + dropped: boolean; + edges: boolean; + stripFor?: ReadonlySet | undefined; + }, ): { modules: ScannedModule[]; edges: PackageEdge[] } { const modules: ScannedModule[] = []; const edges: PackageEdge[] = []; @@ -160,6 +167,10 @@ export function scanFiles( // (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; + // A module a build asks about for an island (H3.5's check, before it + // decides): the cost stays with the modules under consideration. + const forRun = + !js && origin === "user" && !sf.isDeclarationFile && opts.stripFor?.has(sf.fileName) === true; let facts: ReturnType = { markers: [], cjsReads: [] }; if ((js && (origin === "npm" || opts.userJs)) || userTs) facts = scanModuleFacts(sf); let markers = userTs @@ -173,7 +184,7 @@ export function scanFiles( packageName: at === null ? null : at.name, markers, cjsReads: facts.cjsReads, - stripProblem: userTs && markers.length > 0 ? stripProblem(program, sf) : null, + stripProblem: forRun || (userTs && markers.length > 0) ? stripProblem(program, sf) : null, dropped: opts.dropped, }); if (!opts.edges) continue; diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6900-node-island-host-class/main.ts.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6900-node-island-host-class/main.ts.json new file mode 100644 index 00000000000..182b1b29809 --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6900-node-island-host-class/main.ts.json @@ -0,0 +1,7 @@ +{ + "order": [ + "/tests/corpus/6900-node-island-host-class/socket.ts", + "/tests/corpus/6900-node-island-host-class/main.ts" + ], + "diags": [] +} diff --git a/tests/corpus/6900-node-island-host-class/globals.d.ts b/tests/corpus/6900-node-island-host-class/globals.d.ts new file mode 100644 index 00000000000..84fd4677e2b --- /dev/null +++ b/tests/corpus/6900-node-island-host-class/globals.d.ts @@ -0,0 +1,12 @@ +// The program declares the host class the way @types/node declares the +// WebSocket global: an interface plus a `var` of its constructor type. Only +// Node has the value itself. +interface HostSocket { + readonly readyState: number; + close(code?: number, reason?: string): void; +} + +declare var WebSocket: { + new (url: string): HostSocket; + readonly prototype: HostSocket; +}; diff --git a/tests/corpus/6900-node-island-host-class/main.ts b/tests/corpus/6900-node-island-host-class/main.ts new file mode 100644 index 00000000000..bc25e4724bf --- /dev/null +++ b/tests/corpus/6900-node-island-host-class/main.ts @@ -0,0 +1,7 @@ +// @requires-libnode +// socket.ts extends a host class the program only declares, and only Node +// has the class itself: the module runs in a Node island automatically +// (R1.6) while the entry stays native. +import { describeSocket } from "./socket.ts"; + +console.log("socket:", describeSocket()); diff --git a/tests/corpus/6900-node-island-host-class/package.json b/tests/corpus/6900-node-island-host-class/package.json new file mode 100644 index 00000000000..6d563eb9043 --- /dev/null +++ b/tests/corpus/6900-node-island-host-class/package.json @@ -0,0 +1 @@ +{ "name": "host-class-island", "private": true, "type": "module" } diff --git a/tests/corpus/6900-node-island-host-class/socket.ts b/tests/corpus/6900-node-island-host-class/socket.ts new file mode 100644 index 00000000000..b4b901c576f --- /dev/null +++ b/tests/corpus/6900-node-island-host-class/socket.ts @@ -0,0 +1,12 @@ +/// +// A class whose base only the JavaScript runtime has (the census's +// `class extends WebSocket` shape): native lowering cannot extend it, so +// this module runs in a Node island automatically (R1.6) and the entry +// stays native. Nothing connects: the program reads the class itself. +export class Socket extends WebSocket { + static tag = "socket"; +} + +export function describeSocket(): string { + return `${Socket.tag}:${Object.getPrototypeOf(Socket) === WebSocket}`; +} diff --git a/tests/harness/corpus-costs/6900-node-island-host-class/main.ts.json b/tests/harness/corpus-costs/6900-node-island-host-class/main.ts.json new file mode 100644 index 00000000000..d7dc7bf24d8 --- /dev/null +++ b/tests/harness/corpus-costs/6900-node-island-host-class/main.ts.json @@ -0,0 +1 @@ +{ "plain": 8000, "san": 45000 } diff --git a/tests/harness/node-island.test.ts b/tests/harness/node-island.test.ts index e00211aa5ac..e15847e6c3e 100644 --- a/tests/harness/node-island.test.ts +++ b/tests/harness/node-island.test.ts @@ -92,6 +92,26 @@ async function build(name: string, libnode?: string, entry = fixture): Promise(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 }); + } +} + +/** A build's diagnostics as "SC0000: message" lines. */ +function messages(result: { diagnostics?: { code: string; message: string }[] }): string[] { + return (result.diagnostics ?? []).map((d) => `${d.code}: ${d.message}`); +} + /** A bridge library that answers the ABI handshake and nothing else. */ async function fakePack(name: string, abi: string, version: string): Promise { const dir = join(outDir, name); @@ -369,23 +389,6 @@ describe.skipIf(skip)("Node islands in-process", () => { ); } - 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"); @@ -533,6 +536,101 @@ export async function loadPayload(name: string): Promise { expect(untraced).toContain("import() with a computed specifier"); }); }); + + // A module that needs the JavaScript runtime itself — it extends a host + // class the program only declares — runs in a Node island automatically + // (R1.6). The entry stays native. + describe("a module that needs a host class", () => { + const globals = `interface HostSocket { + readonly readyState: number; + close(code?: number, reason?: string): void; +} + +declare var WebSocket: { + new (url: string): HostSocket; + readonly prototype: HostSocket; +}; +`; + const socket = `/// +export class Socket extends WebSocket { + static tag = "socket"; +} + +export function describeSocket(): string { + return \`\${Socket.tag}:\${Object.getPrototypeOf(Socket) === WebSocket}\`; +} +`; + const main = `import { describeSocket } from "./socket.ts"; + +console.log("socket:", describeSocket()); +`; + + async function buildProgram(): Promise { + const dir = mkdtempSync(join(tmpdir(), "scriptc-host-class-")); + writeFileSync( + join(dir, "package.json"), + '{ "name": "host-class", "private": true, "type": "module" }\n', + ); + writeFileSync(join(dir, "globals.d.ts"), globals); + writeFileSync(join(dir, "socket.ts"), socket); + writeFileSync(join(dir, "main.ts"), main); + return dir; + } + + test("islands the module and runs it in Node", async () => { + const dir = await buildProgram(); + const result = await withFreshBuildCache(() => + compile(join(dir, "main.ts"), { + outPath: join(dir, "main"), + outDir: dir, + sanitize, + libnode: pack!, + }), + ); + expect(messages(result)).toEqual([]); + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.islands?.modules.map((file) => basename(file))).toEqual(["socket.ts"]); + const note = (result.warnings ?? []).find( + (d) => d.code === "SC8001" && d.message.includes("socket.ts"), + ); + expect(note?.message).toContain("Node island"); + // The island's module cache holds the TypeScript module, and Node + // refuses to strip types under node_modules, where this suite keeps + // its shared cache: this run gets a cache of its own. + const executed = await run(join(dir, "main"), [], { + ...process.env, + XDG_CACHE_HOME: join(dir, "cache"), + }); + expect(executed).toEqual({ stdout: "socket: socket:true\n", stderr: "", code: 0 }); + }, 600_000); + + // Node runs TypeScript by erasing types in place: a module that needs a + // transform cannot run in an island, so its refusal stays the build's + // answer. + test("a host class in a file Node cannot strip stays refused", async () => { + const dir = await buildProgram(); + writeFileSync( + join(dir, "socket.ts"), + `/// +enum Mode { + Socket, +} +export class Socket extends WebSocket { + static tag = Mode.Socket === 0 ? "socket" : "other"; +} + +export function describeSocket(): string { + return \`\${Socket.tag}:\${Object.getPrototypeOf(Socket) === WebSocket}\`; +} +`, + ); + const result = await withFreshBuildCache(() => + compile(join(dir, "main.ts"), { outPath: join(dir, "main"), outDir: dir, sanitize }), + ); + expect(result.ok).toBe(false); + expect(messages(result).some((m) => m.startsWith("SC1090: extending classes"))).toBe(true); + }, 600_000); }); }); From cd6c714aa7b826111437f3ddc1064c64b5b18513 Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Thu, 29 Oct 2026 07:57:58 -0700 Subject: [PATCH 11/13] Ship a Node island worker entry's subtree into the island tree A module that runs in a Node island starts Node's own Worker, in a thread with its own module instances and its own loader hooks. The island tree ships program modules the island also uses as placeholders that only the island thread's hooks answer, so a `new Worker(new URL("./worker.ts", import.meta.url))` in an island module could not load its entry at all. The tree now ships the entry and its static import subtree as real sources: the collector asks a new scan (frontend/island-worker-entry.ts) for the worker entries a module's shipped copy starts, and a fresh module is shipped as its source even where the program provides a placeholder (the worker thread never runs the island thread's hooks). Entries are accepted only in the form the tree keeps, a path relative to the module its own URL names; a computed entry, or a path that names the build machine, stays refused as SC8006. A TypeScript file that ships this way must pass stripProblem (a file the build never put in the program is parsed and checked syntactically), because Node erases types in a worker thread too, and refuses to below node_modules. Corpus 6901 covers it: an island module starts a worker on a program module that imports a helper, messages both ways, and the worker's helper state stays separate from the program's, identical under Node and native. Co-Authored-By: Claude Opus 5.5 --- .../limitations/native-worker-threads.mdx | 2 + packages/compiler/src/frontend/fork-target.ts | 51 ++++++ .../src/frontend/island-worker-entry.ts | 151 ++++++++++++++++++ .../compiler/src/frontend/lowering/lowerer.ts | 45 +++++- .../compiler/src/node-islands/modules.test.ts | 70 ++++++++ packages/compiler/src/node-islands/modules.ts | 69 +++++++- .../compiler/src/node-islands/type-strip.ts | 12 +- .../6901-island-worker-entry/main.ts.json | 8 + .../6901-island-worker-entry/globals.d.ts | 12 ++ .../6901-island-worker-entry/launcher.ts | 34 ++++ tests/corpus/6901-island-worker-entry/main.ts | 13 ++ .../6901-island-worker-entry/package.json | 1 + .../corpus/6901-island-worker-entry/state.ts | 9 ++ .../corpus/6901-island-worker-entry/worker.ts | 9 ++ .../6901-island-worker-entry/main.ts.json | 1 + tests/harness/node-island.test.ts | 122 ++++++++++++++ 16 files changed, 600 insertions(+), 9 deletions(-) create mode 100644 packages/compiler/src/frontend/island-worker-entry.ts create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6901-island-worker-entry/main.ts.json create mode 100644 tests/corpus/6901-island-worker-entry/globals.d.ts create mode 100644 tests/corpus/6901-island-worker-entry/launcher.ts create mode 100644 tests/corpus/6901-island-worker-entry/main.ts create mode 100644 tests/corpus/6901-island-worker-entry/package.json create mode 100644 tests/corpus/6901-island-worker-entry/state.ts create mode 100644 tests/corpus/6901-island-worker-entry/worker.ts create mode 100644 tests/harness/corpus-costs/6901-island-worker-entry/main.ts.json diff --git a/docs/content/limitations/native-worker-threads.mdx b/docs/content/limitations/native-worker-threads.mdx index 5bd259114b1..4ac71848835 100644 --- a/docs/content/limitations/native-worker-threads.mdx +++ b/docs/content/limitations/native-worker-threads.mdx @@ -1,5 +1,7 @@ `node:worker_threads` runs workers as native threads. Worker entry points must resolve at build time: use a source-relative `new URL("./worker.ts", import.meta.url)`, the current module URL, an absolute path, or CommonJS `__filename`. +A module that runs in a Node island starts Node's own Worker instead: its entry must be a source-relative `new URL("./worker.ts", import.meta.url)` built from that module's URL, the build ships the entry and the modules it imports beside the island's modules, and the worker thread evaluates its own copies of them. +The build refuses a Worker whose entry it cannot resolve to such a path, and a TypeScript entry, or a TypeScript module its imports reach, whose types Node cannot strip in a worker thread. Each worker initializes its own module state, event loop, environment snapshot, and reference-counted heap. Nested workers inherit their parent's environment snapshot. Supported constructor options are `workerData`, `argv`, a `transferList` of MessagePorts, `name` (reported by `threadName` and `worker.threadName`), `stdout`, `stderr` and `stdin`, and `resourceLimits.stackSizeMb`, which sets the worker thread's stack size (at least 192 KiB; without it a worker's stack is 8 MiB, where Node's is 4 MiB). diff --git a/packages/compiler/src/frontend/fork-target.ts b/packages/compiler/src/frontend/fork-target.ts index 398e1b0aaea..d157d73d70a 100644 --- a/packages/compiler/src/frontend/fork-target.ts +++ b/packages/compiler/src/frontend/fork-target.ts @@ -256,6 +256,57 @@ function staticString( return null; } +/** Whether an entry expression names its file relative to the module that + * starts it: `import.meta.url`, `__filename`, or a URL whose base is one of + * those (`new URL("./entry.ts", import.meta.url)`, through consts, + * templates and fileURLToPath). That relation is the one a shipped Node + * island tree keeps, because every collected file keeps its layout + * relative to one root; an absolute path or a `file:` URL built from + * anything else names the build machine, which the tree does not hold. */ +export function fromModuleUrl( + program: ts.Program, + expr: ts.Expression, + seen: Set = new Set(), +): boolean { + const current = strip(expr); + if (importMetaUrl(current)) return true; + if (ts.isIdentifier(current) && current.text === "__filename") + return isNodeGlobal(program, current); + const init = constInitializer(program, current); + if (init !== null && ts.isIdentifier(current)) { + const symbol = program.getTypeChecker().getSymbolAtLocation(current); + if (symbol === undefined || seen.has(symbol)) return false; + seen.add(symbol); + const value = fromModuleUrl(program, init, seen); + seen.delete(symbol); + return value; + } + if ( + ts.isCallExpression(current) && + !current.questionDotToken && + current.arguments.nodes.length === 1 && + ts.isIdentifier(current.expression) && + isBuiltinMemberImport(program, current.expression, "url", "fileURLToPath") + ) + return fromModuleUrl(program, current.arguments.nodes[0]!, seen); + if ( + ts.isNewExpression(current) && + !current.questionDotToken && + ts.isIdentifier(current.expression) && + (isBuiltinMemberImport(program, current.expression, "url", "URL") || + (current.expression.text === "URL" && isNodeGlobal(program, current.expression))) + ) { + const args = current.arguments?.nodes ?? []; + if (args.length !== 2) return false; + const input = staticString(program, args[0]!, seen); + // A reference relative to the base: an absolute one would not move + // with the module. + if (input === null || input.startsWith("/") || /^[a-z][a-z0-9+.-]*:/i.test(input)) return false; + return fromModuleUrl(program, args[1]!, seen); + } + return false; +} + /** Resolves the supported fork modulePath expression to an absolute source * path. Plain runtime strings are intentionally excluded: worker selection * is part of the compiled graph, not a runtime filesystem lookup. */ diff --git a/packages/compiler/src/frontend/island-worker-entry.ts b/packages/compiler/src/frontend/island-worker-entry.ts new file mode 100644 index 00000000000..e7448c5ee28 --- /dev/null +++ b/packages/compiler/src/frontend/island-worker-entry.ts @@ -0,0 +1,151 @@ +/* Worker entries of a module whose shipped copy Node runs (scope-B: an + * island module, or a module such an island loads). Node starts a worker + * thread with its own module instances: the thread re-evaluates the entry + * and everything the entry imports. So the module tree ships the entry and + * its static import subtree as real sources (node-islands/modules.ts), and + * the Worker keeps its path: the URL the module builds from its own URL at + * build time names the same file in the tree. + * + * Only a form the tree can keep is accepted: an entry relative to the + * module's own URL (`new URL("./entry.ts", import.meta.url)`, through + * consts, templates and fileURLToPath; `import.meta.url` itself; + * `__filename`). Every other form a build can see is refused, because the + * tree keeps relative layout and nothing else: + * - a computed or unbounded entry: the build cannot know what to ship; + * - an absolute path or a `file:` URL that is not built from the module's + * own URL: the tree never holds a build-machine path; + * - a path relative to the process's working directory (Node resolves a + * plain relative filename against `cwd`): the tree is not the cwd. + * A static entry that names no file at all (a data: URL, a package + * specifier, a missing path) is Node's own business and stays as it is. + * + * A TypeScript file that ships this way must pass stripProblem: Node + * erases types in place in a worker thread too, and it refuses to do so + * inside node_modules. */ +import { resolve } from "node:path"; +import { trackedFileExists, trackedReadFile } from "./input-tracker.js"; +import * as ts from "./ts7/adapter.js"; +import { parseSourceFile } from "./ts7/source-parser-node.js"; +import { tsgoPath } from "./dts-paths.js"; +import { fromModuleUrl, staticForkString } from "./fork-target.js"; +import { moduleSourceCandidates } from "./module-source-candidates.js"; +import { isWorkerConstructor, workerModulePath } from "./worker-target.js"; +import type { NodeIslandWorkerScan } from "../node-islands/modules.js"; +import { stripProblem } from "../node-islands/type-strip.js"; + +export interface IslandWorkerScan { + /** The worker entries `file`'s shipped copy starts, or the reason one + * cannot ship (node-islands/modules.ts asks for each traced module). */ + workers(file: string): NodeIslandWorkerScan; + /** Why Node cannot run `file`'s source as shipped, or null. */ + sourceProblem(file: string): string | null; +} + +const TYPESCRIPT = /\.[mc]?ts$/; + +const NONE: NodeIslandWorkerScan = { kind: "entries", files: [] }; + +interface IslandWorkerSource { + sf: ts.SourceFile; + /** False for a file the program never parsed: a worker entry's subtree is + * shipped, not compiled, so it need not be a file of the program. */ + inProgram: boolean; +} + +/** The scan over one program: `compiled` answers whether the program lowers + * a file natively, whose Worker constructions the native lowering answers + * (lowerWorkerNew) and which therefore never run from the shipped tree. It + * is asked on every call, not read once: the module order grows while + * lowering (worker targets join it) and the tree is collected after. */ +export function createIslandWorkerScan( + program: ts.Program, + compiled: (file: string) => boolean, +): IslandWorkerScan { + let programSources: Map | null = null; + const parsed = new Map(); + const key = (file: string): string => tsgoPath(resolve(file)); + const sourceOf = (file: string): IslandWorkerSource | null => { + const id = key(file); + const cached = parsed.get(id); + if (cached !== undefined) return cached; + programSources ??= new Map(); + if (programSources.size === 0) + for (const sf of program.getImplementationSourceFiles()) + programSources.set(key(sf.fileName), sf); + const inProgram = programSources.get(id); + let source: IslandWorkerSource | null = null; + if (inProgram !== undefined) source = { sf: inProgram, inProgram: true }; + else { + // Only the files a worker entry's subtree ships reach this: parse the + // text the tree carries, because the program never did. + const text = trackedReadFile(file); + if (text !== null) + source = { sf: parseSourceFile(file, text, kindOf(file)), inProgram: false }; + } + parsed.set(id, source); + return source; + }; + + const workers = (file: string): NodeIslandWorkerScan => { + if (compiled(key(file))) return NONE; + const source = sourceOf(file); + // A file a worker entry's subtree ships was never checked, so the + // checker cannot bind its names and a Worker it starts is not + // recognised; Node then reports that entry as a missing module in the + // worker thread. A file of the program is the case this serves. + if (source === null || !source.inProgram) return NONE; + const sf = source.sf; + const entries = new Set(); + for (const construction of moduleSourceCandidates(program, sf).constructions) { + const args = construction.arguments?.nodes ?? []; + // An arity the constructor rejects is the runtime's error, in the + // island exactly as under Node. + if (args.length === 0 || args.length > 2 || args.some(ts.isSpreadElement)) continue; + if (!isWorkerConstructor(program, construction.expression)) continue; + const entry = args[0]!; + const path = workerModulePath(program, entry); + if (path === null) { + // A static entry that names no file of the build (a data: URL a + // package specifier, a filename relative to the working directory) + // is Node's to resolve; only an entry the build cannot even see is + // refused. + if (staticForkString(program, entry) !== null) continue; + return { kind: "refused", reason: refusal(sf, construction, "cannot resolve") }; + } + if (!fromModuleUrl(program, entry)) + return { + kind: "refused", + reason: refusal(sf, construction, `names the build-machine path '${path}'`), + }; + // A file that does not exist is loaded (and fails) exactly as under + // Node: there is nothing for the tree to carry. + if (trackedFileExists(path)) entries.add(path); + } + return entries.size === 0 ? NONE : { kind: "entries", files: [...entries].sort() }; + }; + + const sourceProblem = (file: string): string | null => { + if (!TYPESCRIPT.test(file)) return null; + if (/[\\/]node_modules[\\/]/.test(file)) + return `'${file}' runs in a worker thread of a Node island, but Node refuses to strip types inside node_modules`; + const source = sourceOf(file); + if (source === null) return null; + const problem = stripProblem(program, source.sf, source.inProgram); + if (problem === null) return null; + return `'${file}' runs in a worker thread of a Node island, but Node cannot strip its types: ${problem.construct}`; + }; + + return { workers, sourceProblem }; +} + +/** The syntax a parse of `file` needs, from its extension. */ +function kindOf(file: string): "ts" | "tsx" { + return /\.[mc]?tsx$/.test(file) ? "tsx" : "ts"; +} + +/** A one-line description of the construction, anchored at its source: the + * collector's diagnostics carry the reason as a message. */ +function refusal(sf: ts.SourceFile, node: ts.Node, what: string): string { + const line = sf.text.slice(0, ts.tokenStartOfNode(node)).split("\n").length; + return `${sf.fileName}:${line} starts a Worker whose entry the build ${what}; a worker entry that runs in a Node island is new URL("./entry.ts", import.meta.url), so the build ships the entry and the modules it imports beside the module`; +} diff --git a/packages/compiler/src/frontend/lowering/lowerer.ts b/packages/compiler/src/frontend/lowering/lowerer.ts index ef9d5c31c41..f6a65b1635a 100644 --- a/packages/compiler/src/frontend/lowering/lowerer.ts +++ b/packages/compiler/src/frontend/lowering/lowerer.ts @@ -786,7 +786,11 @@ import { maybePromiseUnionOf, promiseArmedUnionOf, } from "./maybe-promise.js"; -import { NodeIslandModuleCollector } from "../../node-islands/modules.js"; +import { + NodeIslandModuleCollector, + type NodeIslandWorkerScan, +} from "../../node-islands/modules.js"; +import { createIslandWorkerScan, type IslandWorkerScan } from "../island-worker-entry.js"; /** Entry function name. '%' cannot appear in a TS identifier, so a user * function can never collide with it (mangling is injective per prefix). */ @@ -3744,8 +3748,13 @@ export class Lowerer { /** Island module loads, by the importing file and specifier: the module * global holding what the import form sees (lower-node-island.ts). */ readonly islandModules = new Map(); - /** The modules Node island calls load (lower-node-island.ts). */ - readonly nodeIslandModules = new NodeIslandModuleCollector(); + /** The modules Node island calls load (lower-node-island.ts). The hooks + * answer what a module's shipped copy runs that is not an import: the + * worker entries it starts (frontend/island-worker-entry.ts) and whether + * Node can run its source (type stripping). */ + readonly nodeIslandModules: NodeIslandModuleCollector; + private islandWorkerScans: IslandWorkerScan | null = null; + private islandWorkerCompiled: { size: number; set: ReadonlySet } | null = null; /** Each call's caller literal, set to its tree path when lowering ends. */ readonly nodeIslandCallers: { literal: IrExpr & { kind: "strLit" }; caller: string }[] = []; /** Absolute specifiers, rewritten to the shipped file relative to their caller. */ @@ -3756,6 +3765,32 @@ export class Lowerer { }[] = []; /** Each island module load's path literal, set to its tree path when lowering ends. */ readonly nodeIslandLoads: { literal: IrExpr & { kind: "strLit" }; path: string }[] = []; + /** The worker entries a module its shipped copy runs starts, or why one + * cannot ship; null when the program has no Node islands. */ + private islandWorkerEntries(file: string): NodeIslandWorkerScan { + return this.islandWorkerScan().workers(file); + } + /** Why Node cannot run a module's source as shipped (type stripping). */ + private islandWorkerSourceProblem(file: string): string | null { + return this.islandWorkerScan().sourceProblem(file); + } + private islandWorkerScan(): IslandWorkerScan { + this.islandWorkerScans ??= createIslandWorkerScan(this.program, (file) => + this.compiledModulePaths().has(file), + ); + return this.islandWorkerScans; + } + /** The modules the program lowers natively, by tsgo path: rebuilt when + * the order grows (worker targets join it while lowering). */ + private compiledModulePaths(): ReadonlySet { + if (this.islandWorkerCompiled?.size !== this.moduleOrder.length) { + this.islandWorkerCompiled = { + size: this.moduleOrder.length, + set: new Set(this.moduleOrder.map((sf) => tsgoPath(resolve(sf.fileName)))), + }; + } + return this.islandWorkerCompiled.set; + } /** Island handle bindings by symbol (lower-node-island.ts). */ readonly islandHandleBindings = new Map(); /** `instanceof` expressions the island lowering already handles. */ @@ -5057,6 +5092,10 @@ export class Lowerer { this.generalNodeStreams = mode.generalNodeStreams ?? false; this.dynNodeStreams = this.generalNodeStreams && (mode.dynNodeStreams ?? false); this.workerTargets = mode.workerTargets ?? []; + this.nodeIslandModules = new NodeIslandModuleCollector({ + workers: (file) => this.islandWorkerEntries(file), + sourceProblem: (file) => this.islandWorkerSourceProblem(file), + }); this.workerTargets.forEach((sf, id) => { this.workerTargetIdByPath.set(tsgoPath(resolve(sf.fileName)), id); }); diff --git a/packages/compiler/src/node-islands/modules.test.ts b/packages/compiler/src/node-islands/modules.test.ts index 8f443a5b57e..4a4d3bd755c 100644 --- a/packages/compiler/src/node-islands/modules.test.ts +++ b/packages/compiler/src/node-islands/modules.test.ts @@ -318,6 +318,76 @@ describe("modules the program provides", () => { expect(files.some((f) => f.path.endsWith("internal.mjs"))).toBe(false); }); + test("a worker entry a module starts ships as source, and so does what it imports", () => { + const root = project({ + "src/main.ts": "", + "src/launcher.mjs": [ + 'import { Worker } from "node:worker_threads";', + 'export const start = () => new Worker(new URL("./worker.mjs", import.meta.url));', + ].join("\n"), + "src/worker.mjs": 'import { count } from "./state.mjs";\ncount();', + "src/state.mjs": "export let count = 0;\nexport const internal = 7;\n", + }); + const collector = new NodeIslandModuleCollector({ + workers: (file) => + file.endsWith("launcher.mjs") + ? { kind: "entries", files: [join(root, "src/worker.mjs")] } + : { kind: "entries", files: [] }, + }); + // A worker thread has its own module instances, so the program module + // the entry imports ships as source there even though the island thread + // reads a placeholder for it. + collector.addProvided(join(root, "src/state.mjs")); + expect(collector.add(join(root, "src/main.ts"), "./launcher.mjs")).toBeNull(); + const tree = collector.tree()!; + expect(tree.root).toBe(join(root, "src")); + expect(tree.files.map((f) => f.path)).toEqual(["launcher.mjs", "state.mjs", "worker.mjs"]); + expect(tree.files.every((f) => f.placeholder !== true)).toBe(true); + const data = readNodeIslandModules(tree); + if (typeof data === "string") throw new Error(data); + expect(new TextDecoder().decode(data)).toContain("export const internal = 7;"); + }); + + test("a worker entry traced after the placeholder ships the real source instead", () => { + const root = project({ + "src/main.ts": "", + "src/plugin.mjs": [ + 'import { s } from "./shared.mjs";', + 'new Worker(new URL("./worker.mjs", import.meta.url));', + ].join("\n"), + "src/worker.mjs": 'import { s } from "./shared.mjs";', + "src/shared.mjs": "export const s = 42;\n", + }); + const collector = new NodeIslandModuleCollector({ + workers: (file) => + file.endsWith("plugin.mjs") + ? { kind: "entries", files: [join(root, "src/worker.mjs")] } + : { kind: "entries", files: [] }, + }); + collector.addProvided(join(root, "src/shared.mjs")); + expect(collector.add(join(root, "src/main.ts"), "./plugin.mjs")).toBeNull(); + const shared = collector.tree()!.files.find((f) => f.path === "shared.mjs")!; + expect(shared.placeholder).toBeUndefined(); + }); + + test("a worker entry the build refuses stops the collection", () => { + const root = project({ + "src/main.ts": "", + "src/plugin.mjs": 'new Worker(new URL("./worker.ts", import.meta.url));', + "src/worker.ts": "enum E {}\n", + }); + const collector = new NodeIslandModuleCollector({ + workers: (file) => + file.endsWith("plugin.mjs") + ? { kind: "entries", files: [join(root, "src/worker.ts")] } + : { kind: "entries", files: [] }, + sourceProblem: (file) => `${file}: Node cannot strip its types: an enum`, + }); + expect(collector.add(join(root, "src/main.ts"), "./plugin.mjs")).toBe( + `${join(root, "src/worker.ts")}: Node cannot strip its types: an enum`, + ); + }); + test("requireModule specifiers come from code, not comments or strings", () => { expect( requireModuleSpecifiers(` diff --git a/packages/compiler/src/node-islands/modules.ts b/packages/compiler/src/node-islands/modules.ts index ead084088d6..e6e94f54fc7 100644 --- a/packages/compiler/src/node-islands/modules.ts +++ b/packages/compiler/src/node-islands/modules.ts @@ -163,6 +163,24 @@ function portable(path: string): string { return sep === "/" ? path : path.split(sep).join("/"); } +/** What a module whose shipped copy Node runs starts, or why it cannot + * ship: the worker entries it resolves to a file (worker threads, Node's + * own: the worker re-evaluates its entry in its own thread, so the tree + * ships the entry and its static import subtree as real sources). */ +export type NodeIslandWorkerScan = + | { kind: "entries"; files: readonly string[] } + | { kind: "refused"; reason: string }; + +export interface NodeIslandModuleHooks { + /** The worker entries `file`'s source starts, or the reason one cannot + * ship. Only asked for modules whose shipped copy is the only copy: a + * module the program compiles lowers its Worker constructions natively. */ + workers?: ((file: string) => NodeIslandWorkerScan) | undefined; + /** Why Node cannot run `file`'s source as shipped (type stripping), or + * null. Asked for every file a worker entry's subtree ships. */ + sourceProblem?: ((file: string) => string | null) | undefined; +} + export class NodeIslandModuleCollector { /** Real paths of collected files. */ private readonly files = new Set(); @@ -176,6 +194,14 @@ export class NodeIslandModuleCollector { private readonly loaded = new Set(); /** Program modules whose source the program provides (addProvided). */ private readonly provided = new Set(); + /** Files a worker entry's static import subtree runs: shipped as their + * real sources, even where the program provides a placeholder. */ + private readonly fresh = new Set(); + /** Fresh files whose subtree is already traced (fresh reaches further + * than the island-side trace: it also runs through placeholders). */ + private readonly freshTraced = new Set(); + + constructor(private readonly hooks: NodeIslandModuleHooks = {}) {} /** Collects the module `specifier` names from `caller` (a real path). * Returns null, or the reason the build cannot ship it. */ @@ -233,7 +259,11 @@ export class NodeIslandModuleCollector { const files: NodeIslandModuleFile[] = []; for (const file of [...this.files].sort()) { const path = portable(relative(root, file)); - if (this.provided.has(file)) { + // A worker entry's subtree runs in the worker's own thread, with its + // own instances: its real source ships even where the program's copy + // is a placeholder (the island thread never reads the placeholder + // anyway; the loader hooks answer it). + if (this.provided.has(file) && !this.fresh.has(file)) { const bytes = providedModulePlaceholder(path); files.push({ path, size: bytes.length, sha256: sha256(bytes), placeholder: true }); continue; @@ -262,7 +292,7 @@ export class NodeIslandModuleCollector { return tree === null ? file : `./${portable(relative(tree.root, file))}`; } - private resolveFrom(fromFile: string, specifier: string): string | null { + private resolveFrom(fromFile: string, specifier: string, fresh = false): string | null { if (isBuiltin(specifier)) return null; if (specifier.startsWith("#")) return `'${specifier}' is a package import ("imports" in package.json), which Node island modules of the program cannot use`; @@ -273,7 +303,8 @@ export class NodeIslandModuleCollector { // optional require in a try block keeps working). if (isPathSpecifier(specifier)) { const target = resolveFile(resolve(dirname(fromFile), specifier)); - return target === null ? null : this.addModule(this.placed(target)); + if (target === null) return null; + return fresh ? this.addFresh(this.placed(target)) : this.addModule(this.placed(target)); } const dir = packageDirectory(dirname(fromFile), packageName(specifier)); if (dir !== null) this.addPackage(dir); @@ -319,6 +350,29 @@ export class NodeIslandModuleCollector { // The program provides its source: island code never runs this file, // so what it imports is not shipped for it. if (this.provided.has(file)) return null; + return this.traceSource(file, false); + } + + /** A worker entry and, through its imports, the subtree the worker's own + * thread evaluates: shipped as its real sources, placeholders or not, + * because the worker has its own module instances (Node starts a + * worker's module graph afresh, in a thread whose loader hooks are its + * own). */ + private addFresh(file: string): string | null { + this.files.add(file); + this.fresh.add(file); + if (this.freshTraced.has(file)) return null; + this.freshTraced.add(file); + this.traced.add(file); + const problem = this.hooks.sourceProblem?.(file) ?? null; + if (problem !== null) return problem; + return this.traceSource(file, true); + } + + /** The source of a collected program module: its package scope, the data + * files it names, its module specifiers (`fresh` on a worker entry's + * subtree) and the worker entries it starts. */ + private traceSource(file: string, fresh: boolean): string | null { const scope = packageScope(dirname(file)); if (scope !== null) this.files.add(scope); if (!TRACED.test(file)) return null; @@ -332,7 +386,14 @@ export class NodeIslandModuleCollector { return `${file}:${line} loads a module through ${first.form}; a module that runs in a Node island loads modules by string-literal specifiers only`; } for (const specifier of scan.specifiers) { - const problem = this.resolveFrom(file, specifier); + const problem = this.resolveFrom(file, specifier, fresh); + if (problem !== null) return problem; + } + const workers = this.hooks.workers?.(file); + if (workers === undefined) return null; + if (workers.kind === "refused") return workers.reason; + for (const entry of workers.files) { + const problem = this.addFresh(this.placed(entry)); if (problem !== null) return problem; } return null; diff --git a/packages/compiler/src/node-islands/type-strip.ts b/packages/compiler/src/node-islands/type-strip.ts index 4e8c61b45be..6f6c4d42bf4 100644 --- a/packages/compiler/src/node-islands/type-strip.ts +++ b/packages/compiler/src/node-islands/type-strip.ts @@ -70,8 +70,15 @@ function importedValue(checker: ts.TypeChecker, element: ts.ImportSpecifier): bo /** 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 { + * marked as patching a builtin, so the walk costs nothing elsewhere. + * `checkImports` is false for a file the build never put in the program + * (a shipped worker entry's subtree): every other check is syntactic, and + * the one that is not needs the checker. */ +export function stripProblem( + program: ts.Program, + sf: ts.SourceFile, + checkImports = true, +): StripProblem | null { let checker: ts.TypeChecker | null = null; let found: StripProblem | null = null; ts.walkPreorder(sf, (node) => { @@ -131,6 +138,7 @@ export function stripProblem(program: ts.Program, sf: ts.SourceFile): StripProbl } const clause = node.importClause; if ( + !checkImports || clause === undefined || clause.phaseModifier === ts.SyntaxKind.TypeKeyword || clause.namedBindings === undefined || diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6901-island-worker-entry/main.ts.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6901-island-worker-entry/main.ts.json new file mode 100644 index 00000000000..aa7d166e724 --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6901-island-worker-entry/main.ts.json @@ -0,0 +1,8 @@ +{ + "order": [ + "/tests/corpus/6901-island-worker-entry/state.ts", + "/tests/corpus/6901-island-worker-entry/launcher.ts", + "/tests/corpus/6901-island-worker-entry/main.ts" + ], + "diags": [] +} diff --git a/tests/corpus/6901-island-worker-entry/globals.d.ts b/tests/corpus/6901-island-worker-entry/globals.d.ts new file mode 100644 index 00000000000..84fd4677e2b --- /dev/null +++ b/tests/corpus/6901-island-worker-entry/globals.d.ts @@ -0,0 +1,12 @@ +// The program declares the host class the way @types/node declares the +// WebSocket global: an interface plus a `var` of its constructor type. Only +// Node has the value itself. +interface HostSocket { + readonly readyState: number; + close(code?: number, reason?: string): void; +} + +declare var WebSocket: { + new (url: string): HostSocket; + readonly prototype: HostSocket; +}; diff --git a/tests/corpus/6901-island-worker-entry/launcher.ts b/tests/corpus/6901-island-worker-entry/launcher.ts new file mode 100644 index 00000000000..8aac4bf74dc --- /dev/null +++ b/tests/corpus/6901-island-worker-entry/launcher.ts @@ -0,0 +1,34 @@ +/// +// Runs in a Node island: it extends a host class only Node has, so the +// native lowering refuses the module and the build runs it in Node (the +// 6900 mechanism). The Worker it starts is Node's own: the build ships +// worker.ts and what it imports beside this module, and the URL this module +// builds at run time names the same file in the shipped tree. +import { Worker } from "node:worker_threads"; + +export class Socket extends WebSocket { + static tag = "socket"; +} + +// The program reads the class, so the refusable base is live code: the +// build islanded this module rather than inventing a native base. +export function socketTag(): string { + return `${Socket.tag}:${Object.getPrototypeOf(Socket) === WebSocket}`; +} + +export async function roundTrip(value: number): Promise { + const worker = new Worker(new URL("./worker.ts", import.meta.url)); + const reply = (): Promise => + new Promise((resolve, reject) => { + worker.once("message", resolve); + worker.once("error", reject); + }); + const firstMessage = reply(); + worker.postMessage(value); + const first = await firstMessage; + const secondMessage = reply(); + worker.postMessage(value + 1); + const second = await secondMessage; + await worker.terminate(); + return `${first} ${second}`; +} diff --git a/tests/corpus/6901-island-worker-entry/main.ts b/tests/corpus/6901-island-worker-entry/main.ts new file mode 100644 index 00000000000..077216cdf5d --- /dev/null +++ b/tests/corpus/6901-island-worker-entry/main.ts @@ -0,0 +1,13 @@ +// @requires-libnode +// A Worker the program starts from a module that runs in a Node island: +// its entry is a module of the program, so the island tree ships the entry +// and what it imports as real sources, and the worker thread evaluates its +// own copy (Node gives every thread its own module instances). The program's +// copy of state.ts and the worker's copy count separately. +import { bump, count } from "./state.ts"; +import { roundTrip, socketTag } from "./launcher.ts"; + +console.log("socket:", socketTag()); +console.log("main:", bump(), bump(), count); +console.log("worker:", await roundTrip(21)); +console.log("main after:", count); diff --git a/tests/corpus/6901-island-worker-entry/package.json b/tests/corpus/6901-island-worker-entry/package.json new file mode 100644 index 00000000000..76a7f3fc582 --- /dev/null +++ b/tests/corpus/6901-island-worker-entry/package.json @@ -0,0 +1 @@ +{ "name": "island-worker-entry", "private": true, "type": "module" } diff --git a/tests/corpus/6901-island-worker-entry/state.ts b/tests/corpus/6901-island-worker-entry/state.ts new file mode 100644 index 00000000000..f11378093b2 --- /dev/null +++ b/tests/corpus/6901-island-worker-entry/state.ts @@ -0,0 +1,9 @@ +// One module, two threads: the program compiles this copy for its own +// thread, and the worker's thread evaluates the island tree's copy, because +// Node gives every thread its own module instances. +export let count = 0; + +export function bump(): number { + count++; + return count; +} diff --git a/tests/corpus/6901-island-worker-entry/worker.ts b/tests/corpus/6901-island-worker-entry/worker.ts new file mode 100644 index 00000000000..9192e7d13ef --- /dev/null +++ b/tests/corpus/6901-island-worker-entry/worker.ts @@ -0,0 +1,9 @@ +// The worker entry, shipped as real source with the island tree: this +// thread's copy of state.ts starts at zero, whatever the program's copy +// holds. +import { parentPort } from "node:worker_threads"; +import { bump, count } from "./state.ts"; + +parentPort?.on("message", (value: number) => { + parentPort?.postMessage(`echo=${value} worker=${bump()},${bump()},${bump()} count=${count}`); +}); diff --git a/tests/harness/corpus-costs/6901-island-worker-entry/main.ts.json b/tests/harness/corpus-costs/6901-island-worker-entry/main.ts.json new file mode 100644 index 00000000000..0fd8341bf23 --- /dev/null +++ b/tests/harness/corpus-costs/6901-island-worker-entry/main.ts.json @@ -0,0 +1 @@ +{ "plain": 8000, "san": 50000 } diff --git a/tests/harness/node-island.test.ts b/tests/harness/node-island.test.ts index e15847e6c3e..7aae9cbe07c 100644 --- a/tests/harness/node-island.test.ts +++ b/tests/harness/node-island.test.ts @@ -632,6 +632,128 @@ export function describeSocket(): string { expect(messages(result).some((m) => m.startsWith("SC1090: extending classes"))).toBe(true); }, 600_000); }); + + // A Worker a module of the program starts from inside a Node island: its + // entry is a module of the program too, so the module tree ships the entry + // and what it imports as real sources, and the worker thread evaluates its + // own copies (Node gives every thread its own module instances). The corpus + // program 6901 pins the behavior against Node; these cases read what the + // build ships and what the island refuses. + describe("a worker entry a Node island starts", () => { + const corpus = join(repoRoot, "tests/corpus/6901-island-worker-entry"); + + /** A scratch copy of the corpus outside node_modules: the worker entry + * ships as TypeScript and Node refuses to strip types below one. */ + function copyCorpus(name: string, extra: Record = {}): string { + const dir = mkdtempSync(join(tmpdir(), `scriptc-island-worker-${name}-`)); + cpSync(corpus, dir, { recursive: true }); + for (const [file, text] of Object.entries(extra)) writeFileSync(join(dir, file), text); + return dir; + } + + async function buildDir(dir: string, libnode: string = pack!) { + return withFreshBuildCache(() => + compile(join(dir, "main.ts"), { + outPath: join(dir, "main"), + outDir: dir, + sanitize, + libnode, + }), + ); + } + + test("ships the entry and what it imports as real sources", async () => { + const dir = copyCorpus("tree"); + const result = await buildDir(dir, "beside"); + expect(messages(result)).toEqual([]); + expect(result.ok).toBe(true); + if (!result.ok) return; + const modules = join(dir, "main.libnode/modules"); + const shipped = (file: string) => readFileSync(join(modules, file), "utf8"); + expect(shipped("worker.ts")).toContain("parentPort?.postMessage"); + // state.ts is the program's own module: the program compiles its copy + // for its own thread, and the worker thread reads this source. + expect(shipped("state.ts")).toContain("export let count = 0;"); + expect(shipped("launcher.ts")).toContain("new Worker(new URL"); + }, 600_000); + + test("runs the worker on its own module instances", async () => { + const dir = copyCorpus("run"); + const result = await buildDir(dir); + expect(messages(result)).toEqual([]); + expect(result.ok).toBe(true); + if (!result.ok) return; + // The island's module cache cannot live under node_modules, where this + // suite keeps its shared one. + const executed = await run(join(dir, "main"), [], { + ...process.env, + XDG_CACHE_HOME: join(dir, "cache"), + }); + expect(executed).toEqual({ + stdout: + "socket: socket:true\nmain: 1 2 2\nworker: echo=21 worker=1,2,3 count=3 echo=22 worker=4,5,6 count=6\nmain after: 2\n", + stderr: "", + code: 0, + }); + }, 600_000); + + // The tree keeps the program's relative layout and no other: an entry the + // build cannot name is refused instead of shipped. + test("a computed entry is SC8006", async () => { + const dir = copyCorpus("computed", { + "launcher.ts": `/// +import { Worker } from "node:worker_threads"; + +export class Socket extends WebSocket { + static tag = "socket"; +} + +export function socketTag(): string { + return \`\${Socket.tag}:\${Object.getPrototypeOf(Socket) === WebSocket}\`; +} + +export function start(name: string): number { + return new Worker(new URL(name, import.meta.url)).threadId; +} +`, + "main.ts": `import { socketTag, start } from "./launcher.ts"; + +console.log(socketTag(), "started", start("worker.ts")); +`, + }); + const result = await buildDir(dir); + expect(result.ok).toBe(false); + const refused = messages(result).find( + (m) => + m.startsWith("SC8006:") && + m.includes("starts a Worker whose entry the build cannot resolve"), + ); + expect(refused).toBeDefined(); + expect(refused).toContain("launcher.ts"); + expect(refused).toContain('new URL("./entry.ts", import.meta.url)'); + }, 600_000); + + test("a worker entry Node cannot strip stays refused", async () => { + const dir = copyCorpus("enum", { + "worker.ts": `import { parentPort } from "node:worker_threads"; + +enum Kind { + A, +} + +parentPort?.postMessage(Kind.A); +`, + }); + const result = await buildDir(dir); + 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("worker.ts"); + expect(stripped).toContain("enum"); + }, 600_000); + }); }); // Lowering checks every argument and result type for JSON data; IR output From 854ffaebe154f9a521207a7987afcbfcb8db60c7 Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Thu, 29 Oct 2026 08:53:37 -0700 Subject: [PATCH 12/13] Read the worker scan from the program, not the TypeScript parser The scan parsed a file the program never held with the parser island's rpc-api module. The compiler is itself compiled natively, where rpc-api and rpc-process cannot lower, so the self-hosted build failed on surface those modules use (process.report, a computed createRequire, a spawn stdio option). The scan reads only the program's own sources now: a worker entry is a program root and its import graph is in the program, so the files the tree ships are the ones the checker bound. A module outside the program has no checked source, and its Worker stays Node's own business, as before the tree carried it; the limitations page says so beside the refusal. Co-Authored-By: Claude Opus 5.5 --- .../limitations/native-worker-threads.mdx | 3 +- .../src/frontend/island-worker-entry.ts | 63 +++++++------------ .../compiler/src/node-islands/type-strip.ts | 12 +--- 3 files changed, 25 insertions(+), 53 deletions(-) diff --git a/docs/content/limitations/native-worker-threads.mdx b/docs/content/limitations/native-worker-threads.mdx index 4ac71848835..de01ac7d0da 100644 --- a/docs/content/limitations/native-worker-threads.mdx +++ b/docs/content/limitations/native-worker-threads.mdx @@ -1,7 +1,8 @@ `node:worker_threads` runs workers as native threads. Worker entry points must resolve at build time: use a source-relative `new URL("./worker.ts", import.meta.url)`, the current module URL, an absolute path, or CommonJS `__filename`. A module that runs in a Node island starts Node's own Worker instead: its entry must be a source-relative `new URL("./worker.ts", import.meta.url)` built from that module's URL, the build ships the entry and the modules it imports beside the island's modules, and the worker thread evaluates its own copies of them. -The build refuses a Worker whose entry it cannot resolve to such a path, and a TypeScript entry, or a TypeScript module its imports reach, whose types Node cannot strip in a worker thread. +The build refuses a Worker whose entry it cannot resolve to such a path, and a TypeScript module of the program that ships this way whose types Node cannot strip in a worker thread. +A TypeScript module the worker's entry imports from outside the program ships unchecked, and Node reports its own error there if it cannot strip it. Each worker initializes its own module state, event loop, environment snapshot, and reference-counted heap. Nested workers inherit their parent's environment snapshot. Supported constructor options are `workerData`, `argv`, a `transferList` of MessagePorts, `name` (reported by `threadName` and `worker.threadName`), `stdout`, `stderr` and `stdin`, and `resourceLimits.stackSizeMb`, which sets the worker thread's stack size (at least 192 KiB; without it a worker's stack is 8 MiB, where Node's is 4 MiB). diff --git a/packages/compiler/src/frontend/island-worker-entry.ts b/packages/compiler/src/frontend/island-worker-entry.ts index e7448c5ee28..d351060e779 100644 --- a/packages/compiler/src/frontend/island-worker-entry.ts +++ b/packages/compiler/src/frontend/island-worker-entry.ts @@ -21,11 +21,19 @@ * * A TypeScript file that ships this way must pass stripProblem: Node * erases types in place in a worker thread too, and it refuses to do so - * inside node_modules. */ + * inside node_modules. + * + * The scan reads only the program's own sources, and never the TypeScript + * parser: a worker entry is a program root and its import graph is in the + * program (worker-target.ts), so the files this ships are exactly the ones + * the checker bound. A module of the tree the program does not hold has no + * checked source, and its Worker stays Node's own business, as it was before + * the tree carried it. The compiler itself is compiled natively, where the + * parser island's rpc-api/rpc-process modules cannot lower — importing them + * from a lowering-side module breaks the self-hosted build. */ import { resolve } from "node:path"; -import { trackedFileExists, trackedReadFile } from "./input-tracker.js"; +import { trackedFileExists } from "./input-tracker.js"; import * as ts from "./ts7/adapter.js"; -import { parseSourceFile } from "./ts7/source-parser-node.js"; import { tsgoPath } from "./dts-paths.js"; import { fromModuleUrl, staticForkString } from "./fork-target.js"; import { moduleSourceCandidates } from "./module-source-candidates.js"; @@ -45,13 +53,6 @@ const TYPESCRIPT = /\.[mc]?ts$/; const NONE: NodeIslandWorkerScan = { kind: "entries", files: [] }; -interface IslandWorkerSource { - sf: ts.SourceFile; - /** False for a file the program never parsed: a worker entry's subtree is - * shipped, not compiled, so it need not be a file of the program. */ - inProgram: boolean; -} - /** The scan over one program: `compiled` answers whether the program lowers * a file natively, whose Worker constructions the native lowering answers * (lowerWorkerNew) and which therefore never run from the shipped tree. It @@ -62,39 +63,22 @@ export function createIslandWorkerScan( compiled: (file: string) => boolean, ): IslandWorkerScan { let programSources: Map | null = null; - const parsed = new Map(); const key = (file: string): string => tsgoPath(resolve(file)); - const sourceOf = (file: string): IslandWorkerSource | null => { - const id = key(file); - const cached = parsed.get(id); - if (cached !== undefined) return cached; + const sourceOf = (file: string): ts.SourceFile | null => { programSources ??= new Map(); if (programSources.size === 0) for (const sf of program.getImplementationSourceFiles()) programSources.set(key(sf.fileName), sf); - const inProgram = programSources.get(id); - let source: IslandWorkerSource | null = null; - if (inProgram !== undefined) source = { sf: inProgram, inProgram: true }; - else { - // Only the files a worker entry's subtree ships reach this: parse the - // text the tree carries, because the program never did. - const text = trackedReadFile(file); - if (text !== null) - source = { sf: parseSourceFile(file, text, kindOf(file)), inProgram: false }; - } - parsed.set(id, source); - return source; + return programSources.get(key(file)) ?? null; }; const workers = (file: string): NodeIslandWorkerScan => { if (compiled(key(file))) return NONE; - const source = sourceOf(file); - // A file a worker entry's subtree ships was never checked, so the - // checker cannot bind its names and a Worker it starts is not - // recognised; Node then reports that entry as a missing module in the - // worker thread. A file of the program is the case this serves. - if (source === null || !source.inProgram) return NONE; - const sf = source.sf; + // A file outside the program has no checked source: its Worker cannot be + // recognised, and Node reports its entry as a missing module in the + // worker thread exactly as it did before the tree carried the file. + const sf = sourceOf(file); + if (sf === null) return NONE; const entries = new Set(); for (const construction of moduleSourceCandidates(program, sf).constructions) { const args = construction.arguments?.nodes ?? []; @@ -128,9 +112,9 @@ export function createIslandWorkerScan( if (!TYPESCRIPT.test(file)) return null; if (/[\\/]node_modules[\\/]/.test(file)) return `'${file}' runs in a worker thread of a Node island, but Node refuses to strip types inside node_modules`; - const source = sourceOf(file); - if (source === null) return null; - const problem = stripProblem(program, source.sf, source.inProgram); + const sf = sourceOf(file); + if (sf === null) return null; + const problem = stripProblem(program, sf); if (problem === null) return null; return `'${file}' runs in a worker thread of a Node island, but Node cannot strip its types: ${problem.construct}`; }; @@ -138,11 +122,6 @@ export function createIslandWorkerScan( return { workers, sourceProblem }; } -/** The syntax a parse of `file` needs, from its extension. */ -function kindOf(file: string): "ts" | "tsx" { - return /\.[mc]?tsx$/.test(file) ? "tsx" : "ts"; -} - /** A one-line description of the construction, anchored at its source: the * collector's diagnostics carry the reason as a message. */ function refusal(sf: ts.SourceFile, node: ts.Node, what: string): string { diff --git a/packages/compiler/src/node-islands/type-strip.ts b/packages/compiler/src/node-islands/type-strip.ts index 6f6c4d42bf4..4e8c61b45be 100644 --- a/packages/compiler/src/node-islands/type-strip.ts +++ b/packages/compiler/src/node-islands/type-strip.ts @@ -70,15 +70,8 @@ function importedValue(checker: ts.TypeChecker, element: ts.ImportSpecifier): bo /** 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. - * `checkImports` is false for a file the build never put in the program - * (a shipped worker entry's subtree): every other check is syntactic, and - * the one that is not needs the checker. */ -export function stripProblem( - program: ts.Program, - sf: ts.SourceFile, - checkImports = true, -): StripProblem | null { + * 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) => { @@ -138,7 +131,6 @@ export function stripProblem( } const clause = node.importClause; if ( - !checkImports || clause === undefined || clause.phaseModifier === ts.SyntaxKind.TypeKeyword || clause.namedBindings === undefined || From 6706417e36498512b3b34901facf3896dd7dcdaf Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Fri, 30 Oct 2026 03:15:10 -0500 Subject: [PATCH 13/13] Check destructured builtin reads, load .mts islands as ES modules, ship nested worker entries, and keep one compareArray - A native module that destructures a patched builtin's CommonJS object (`const { readFileSync } = require("fs")`, nested patterns, an alias of the object) or reads it through require() directly (`require("fs").readFileSync`) now records those member reads, so the patch check reports SC8020 for them as it does for `fs.readFileSync`. - An island module ending in .mts is an ES module and one ending in .cts is CommonJS, whatever the nearest package.json says (isEsmFile and the effect gate's copy), so a default import of an islanded .mts file in a CommonJS package is its default export. - A module of a worker entry's subtree runs in the worker thread from the shipped tree even when the program also compiles it, so its Worker constructions are scanned and their entries ship. The compiled-file guard now applies only outside a worker subtree. - The Test262 harness exported assert's compareArray as `compareArray` beside assert.js's bare `compareArray` function, a duplicate identifier that made every harness compile refuse. Assert's member is now `assertCompareArray`, and the island lane binds the bare function too. detect.test.ts covers the destructured and direct reads; node-island.test.ts covers SC8020 through destructuring, the .mts default export run against Node's output, and the nested worker entry in the shipped tree. --- .../src/frontend/island-worker-entry.ts | 19 +++-- .../frontend/lowering/lower-node-island.ts | 8 +- .../compiler/src/frontend/lowering/lowerer.ts | 6 +- .../compiler/src/node-islands/detect.test.ts | 11 +++ packages/compiler/src/node-islands/detect.ts | 49 +++++++++-- .../compiler/src/node-islands/effect-gate.ts | 4 +- packages/compiler/src/node-islands/modules.ts | 10 ++- tests/harness/node-island.test.ts | 84 +++++++++++++++++++ tests/test262/harness.ts | 6 +- tests/test262/support.mjs | 9 +- 10 files changed, 177 insertions(+), 29 deletions(-) diff --git a/packages/compiler/src/frontend/island-worker-entry.ts b/packages/compiler/src/frontend/island-worker-entry.ts index d351060e779..bd2cdc049e3 100644 --- a/packages/compiler/src/frontend/island-worker-entry.ts +++ b/packages/compiler/src/frontend/island-worker-entry.ts @@ -43,8 +43,10 @@ import { stripProblem } from "../node-islands/type-strip.js"; export interface IslandWorkerScan { /** The worker entries `file`'s shipped copy starts, or the reason one - * cannot ship (node-islands/modules.ts asks for each traced module). */ - workers(file: string): NodeIslandWorkerScan; + * cannot ship (node-islands/modules.ts asks for each traced module). + * `fresh`: the file belongs to a worker entry's subtree, so its shipped + * copy runs in the worker thread even when the program compiles it. */ + workers(file: string, fresh: boolean): NodeIslandWorkerScan; /** Why Node cannot run `file`'s source as shipped, or null. */ sourceProblem(file: string): string | null; } @@ -55,9 +57,12 @@ const NONE: NodeIslandWorkerScan = { kind: "entries", files: [] }; /** The scan over one program: `compiled` answers whether the program lowers * a file natively, whose Worker constructions the native lowering answers - * (lowerWorkerNew) and which therefore never run from the shipped tree. It - * is asked on every call, not read once: the module order grows while - * lowering (worker targets join it) and the tree is collected after. */ + * (lowerWorkerNew), so outside a worker entry's subtree they never run from + * the shipped tree. Inside one (`fresh`), the worker thread evaluates the + * file's shipped copy, natively compiled or not, and its Worker + * constructions are scanned. `compiled` is asked on every call, not read + * once: the module order grows while lowering (worker targets join it) and + * the tree is collected after. */ export function createIslandWorkerScan( program: ts.Program, compiled: (file: string) => boolean, @@ -72,8 +77,8 @@ export function createIslandWorkerScan( return programSources.get(key(file)) ?? null; }; - const workers = (file: string): NodeIslandWorkerScan => { - if (compiled(key(file))) return NONE; + const workers = (file: string, fresh: boolean): NodeIslandWorkerScan => { + if (!fresh && compiled(key(file))) return NONE; // A file outside the program has no checked source: its Worker cannot be // recognised, and Node reports its entry as a missing module in the // worker thread exactly as it did before the tree carried the file. diff --git a/packages/compiler/src/frontend/lowering/lower-node-island.ts b/packages/compiler/src/frontend/lowering/lower-node-island.ts index 742b47d4bb9..f49a74a3458 100644 --- a/packages/compiler/src/frontend/lowering/lower-node-island.ts +++ b/packages/compiler/src/frontend/lowering/lower-node-island.ts @@ -270,10 +270,12 @@ function boundaryFail(lowerer: Lowerer, what: string, node: ts.Node): never { throw new PoisonError(); } -/** Whether the JavaScript file is an ES module by Node's rules. */ +/** Whether the file is an ES module by Node's rules: `.mjs` and `.mts` + * always are, `.cjs` and `.cts` never are, and any other file follows the + * nearest package.json's `type`. */ export function isEsmFile(file: string): boolean { - if (file.endsWith(".mjs")) return true; - if (file.endsWith(".cjs")) return false; + if (file.endsWith(".mjs") || file.endsWith(".mts")) return true; + if (file.endsWith(".cjs") || file.endsWith(".cts")) return false; for (let dir = dirname(file); ;) { try { const pkg = JSON.parse(readFileSync(join(dir, "package.json"), "utf8")) as { diff --git a/packages/compiler/src/frontend/lowering/lowerer.ts b/packages/compiler/src/frontend/lowering/lowerer.ts index f6a65b1635a..5b6bf2baab7 100644 --- a/packages/compiler/src/frontend/lowering/lowerer.ts +++ b/packages/compiler/src/frontend/lowering/lowerer.ts @@ -3767,8 +3767,8 @@ export class Lowerer { readonly nodeIslandLoads: { literal: IrExpr & { kind: "strLit" }; path: string }[] = []; /** The worker entries a module its shipped copy runs starts, or why one * cannot ship; null when the program has no Node islands. */ - private islandWorkerEntries(file: string): NodeIslandWorkerScan { - return this.islandWorkerScan().workers(file); + private islandWorkerEntries(file: string, fresh: boolean): NodeIslandWorkerScan { + return this.islandWorkerScan().workers(file, fresh); } /** Why Node cannot run a module's source as shipped (type stripping). */ private islandWorkerSourceProblem(file: string): string | null { @@ -5093,7 +5093,7 @@ export class Lowerer { this.dynNodeStreams = this.generalNodeStreams && (mode.dynNodeStreams ?? false); this.workerTargets = mode.workerTargets ?? []; this.nodeIslandModules = new NodeIslandModuleCollector({ - workers: (file) => this.islandWorkerEntries(file), + workers: (file, fresh) => this.islandWorkerEntries(file, fresh), sourceProblem: (file) => this.islandWorkerSourceProblem(file), }); this.workerTargets.forEach((sf, id) => { diff --git a/packages/compiler/src/node-islands/detect.test.ts b/packages/compiler/src/node-islands/detect.test.ts index 52b577da860..b305f4ee244 100644 --- a/packages/compiler/src/node-islands/detect.test.ts +++ b/packages/compiler/src/node-islands/detect.test.ts @@ -302,6 +302,17 @@ 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([]); + // Destructuring the CommonJS object reads each member it binds, and a + // require() read directly is a read through the object too. + expect(reads("const { readFileSync } = require('fs'); readFileSync(p);")).toEqual([ + "fs.readFileSync", + ]); + expect( + reads("const { readFileSync: read, promises: { readFile } } = require('node:fs');"), + ).toEqual(["fs.readFileSync", "fs.promises", "fs.promises.readFile"]); + expect(reads("const fs = require('fs'); const { statSync } = fs;")).toEqual(["fs.statSync"]); + expect(reads("require('fs').readFileSync(p);")).toEqual(["fs.readFileSync"]); + expect(reads("const { helper } = require('./local');")).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 926e1d6b815..8bd21b40667 100644 --- a/packages/compiler/src/node-islands/detect.ts +++ b/packages/compiler/src/node-islands/detect.ts @@ -385,6 +385,10 @@ class ModuleScanner { } if (ts.isObjectBindingPattern(name)) { const base = init === undefined ? null : this.builtinOf(init); + // Destructuring a builtin's CommonJS object reads each member it + // binds (H3.4), as `fs.readFileSync` does. + if (base !== null && !base.includes(".") && this.isCjsObjectValue(stripWrappers(init!))) + this.recordPatternReads(name, base); for (const element of name.elements.nodes) { if (!ts.isBindingElement(element)) continue; const local = element.name; @@ -648,16 +652,32 @@ class ModuleScanner { } } + /** Records the members an object binding pattern reads from `base`, a + * builtin module's CommonJS object or a member of one: each element's + * property, and the members a nested pattern reads from it. */ + private recordPatternReads(pattern: ts.ObjectBindingPattern, base: string): void { + for (const element of pattern.elements.nodes) { + if (!ts.isBindingElement(element) || element.dotDotDotToken !== undefined) continue; + const key = element.propertyName ?? element.name; + const member = + key !== undefined && (ts.isIdentifier(key) || ts.isStringLiteral(key)) ? key.text : null; + if (member === null) continue; + this.cjsReads.push({ + member: `${base}.${member}`, + loc: { file: this.sf.fileName, start: ts.tokenStartOfNode(element), end: element.end }, + }); + if (element.name !== undefined && ts.isObjectBindingPattern(element.name)) + this.recordPatternReads(element.name, `${base}.${member}`); + } + } + /** 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 member = this.cjsMemberOf(node); + if (member === null) return; const parent = node.parent; if (parent !== undefined) { if ( @@ -674,11 +694,28 @@ class ModuleScanner { return; } this.cjsReads.push({ - member: module + text.slice(root.text.length), + member, loc: { file: this.sf.fileName, start: ts.tokenStartOfNode(node), end: node.end }, }); } + /** The `module.member` a property read names through a builtin module's + * CommonJS object: a name bound to one, or a require() of the builtin + * read directly (`require("fs").readFileSync`). Null for any other read. */ + private cjsMemberOf(node: ts.PropertyAccessExpression): string | null { + const root = chainRoot(node); + if (root !== null) { + if (!this.cjsBuiltinObjects.has(root.text)) return null; + const module = this.builtinBindings.get(root.text); + const text = chainText(node); + return module === undefined || text === null ? null : module + text.slice(root.text.length); + } + let base = stripWrappers(node.expression); + while (ts.isPropertyAccessExpression(base) || ts.isElementAccessExpression(base)) + base = stripWrappers(base.expression); + return ts.isCallExpression(base) && this.isCjsObjectValue(base) ? this.builtinOf(node) : null; + } + 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/effect-gate.ts b/packages/compiler/src/node-islands/effect-gate.ts index d99d831cc34..fa6d8527e1f 100644 --- a/packages/compiler/src/node-islands/effect-gate.ts +++ b/packages/compiler/src/node-islands/effect-gate.ts @@ -181,8 +181,8 @@ export type EffectGateResult = | { ok: false; reason: string; file: string }; function isEsm(file: string): boolean { - if (file.endsWith(".mjs")) return true; - if (file.endsWith(".cjs")) return false; + if (file.endsWith(".mjs") || file.endsWith(".mts")) return true; + if (file.endsWith(".cjs") || file.endsWith(".cts")) return false; for (let dir = dirname(file); ;) { try { const pkg = JSON.parse(readFileSync(resolve(dir, "package.json"), "utf8")) as { diff --git a/packages/compiler/src/node-islands/modules.ts b/packages/compiler/src/node-islands/modules.ts index e6e94f54fc7..d6e787e041d 100644 --- a/packages/compiler/src/node-islands/modules.ts +++ b/packages/compiler/src/node-islands/modules.ts @@ -173,9 +173,11 @@ export type NodeIslandWorkerScan = export interface NodeIslandModuleHooks { /** The worker entries `file`'s source starts, or the reason one cannot - * ship. Only asked for modules whose shipped copy is the only copy: a - * module the program compiles lowers its Worker constructions natively. */ - workers?: ((file: string) => NodeIslandWorkerScan) | undefined; + * ship. `fresh` is true for a file of a worker entry's subtree, whose real + * source runs in the worker's own thread even when the program compiles + * the file too; otherwise a module the program compiles lowers its Worker + * constructions natively and its shipped copy starts none. */ + workers?: ((file: string, fresh: boolean) => NodeIslandWorkerScan) | undefined; /** Why Node cannot run `file`'s source as shipped (type stripping), or * null. Asked for every file a worker entry's subtree ships. */ sourceProblem?: ((file: string) => string | null) | undefined; @@ -389,7 +391,7 @@ export class NodeIslandModuleCollector { const problem = this.resolveFrom(file, specifier, fresh); if (problem !== null) return problem; } - const workers = this.hooks.workers?.(file); + const workers = this.hooks.workers?.(file, fresh); if (workers === undefined) return null; if (workers.kind === "refused") return workers.reason; for (const entry of workers.files) { diff --git a/tests/harness/node-island.test.ts b/tests/harness/node-island.test.ts index 7aae9cbe07c..fd513e14d9f 100644 --- a/tests/harness/node-island.test.ts +++ b/tests/harness/node-island.test.ts @@ -423,6 +423,25 @@ console.log(result.text, fs.readFileSync("payload.txt", "utf8")); expect(read).toContain("loader.ts"); }, 300_000); + test("a native read of the patched member through destructuring is SC8020", async () => { + const dir = copyCorpus( + "read-destructured", + `import { createRequire } from "node:module"; +import { loadPayload } from "./loader.ts"; +const require = createRequire(import.meta.url); +const { readFileSync } = require("fs") as typeof import("fs"); +const result = await loadPayload(); +console.log(result.text, readFileSync("payload.txt", "utf8")); +`, + ); + const result = await buildDir(dir, "main"); + expect(result.ok).toBe(false); + const read = messages(result).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", @@ -677,6 +696,37 @@ export function describeSocket(): string { expect(shipped("launcher.ts")).toContain("new Worker(new URL"); }, 600_000); + // state.ts is compiled for the program's thread and also runs in the + // worker's thread from the tree: a Worker it starts is the worker + // thread's own, so its entry ships too. + test("ships the worker entries of a compiled module a worker imports", async () => { + const dir = copyCorpus("nested", { + "state.ts": `import { Worker } from "node:worker_threads"; + +export let count = 0; + +export function bump(): number { + count++; + return count; +} + +export function startNested(): Worker { + return new Worker(new URL("./nested.ts", import.meta.url)); +} +`, + "nested.ts": `import { parentPort } from "node:worker_threads"; + +parentPort?.postMessage("nested"); +`, + }); + const result = await buildDir(dir, "beside"); + expect(messages(result)).toEqual([]); + expect(result.ok).toBe(true); + if (!result.ok) return; + const modules = join(dir, "main.libnode/modules"); + expect(readFileSync(join(modules, "nested.ts"), "utf8")).toContain('postMessage("nested")'); + }, 600_000); + test("runs the worker on its own module instances", async () => { const dir = copyCorpus("run"); const result = await buildDir(dir); @@ -697,6 +747,40 @@ export function describeSocket(): string { }); }, 600_000); + // A .mts module is an ES module whatever its package.json says, so the + // island loads it as one: a default import is its default export. + test("an islanded .mts module in a CommonJS package keeps its default export", async () => { + const dir = copyCorpus("mts", { + "package.json": '{ "name": "island-mts", "private": true }\n', + "launcher.mts": `/// +export class Socket extends WebSocket { + static tag = "socket"; +} + +export default function greet(name: string): string { + return \`hello \${name} from \${Socket.tag}:\${Object.getPrototypeOf(Socket) === WebSocket}\`; +} +`, + "main.ts": `import greet from "./launcher.mts"; + +console.log(greet("island")); +`, + }); + const result = await buildDir(dir); + expect(messages(result)).toEqual([]); + expect(result.ok).toBe(true); + if (!result.ok) return; + const executed = await run(join(dir, "main"), [], { + ...process.env, + XDG_CACHE_HOME: join(dir, "cache"), + }); + expect(executed).toEqual({ + stdout: "hello island from socket:true\n", + stderr: "", + code: 0, + }); + }, 600_000); + // The tree keeps the program's relative layout and no other: an entry the // build cannot name is refused instead of shipped. test("a computed entry is SC8006", async () => { diff --git a/tests/test262/harness.ts b/tests/test262/harness.ts index ab47fc96f08..9300454e865 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 bare +// `compareArray` above is assert.js's own function, so assert's member +// exports as `assertCompareArray`. 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 f4d8eb32c1f..9d25b08f029 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,11 @@ 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;", + ]; return [ 'const { readFileSync } = require("node:fs");', 'const vm = require("node:vm");',