Skip to content

feat: port ncc bundling pipeline to rspack - #1304

Open
ScriptedAlchemy wants to merge 24 commits into
vercel:mainfrom
ScriptedAlchemy:llms-module-rspack-port-78c2
Open

feat: port ncc bundling pipeline to rspack#1304
ScriptedAlchemy wants to merge 24 commits into
vercel:mainfrom
ScriptedAlchemy:llms-module-rspack-port-78c2

Conversation

@ScriptedAlchemy

@ScriptedAlchemy ScriptedAlchemy commented Feb 9, 2026

Copy link
Copy Markdown

Summary

  • Port ncc's internal bundling pipeline from webpack to rspack while preserving CLI behavior.
  • Keep resolver fallback, native asset relocation, and TypeScript loader behavior aligned with existing edge cases.
  • Update unit output fixtures for rspack-generated artifacts and refresh dependencies/lockfile changes.

Testing

  • Updated fixture outputs across affected unit cases.
  • CI on this PR validates behavior against the existing test suite.

@ScriptedAlchemy ScriptedAlchemy changed the title feat: migrate to rspac feat: port ncc bundling pipeline to rspack Feb 9, 2026
Comment thread scripts/build.js
Comment thread src/loaders/ts-loader.js
cursoragent and others added 8 commits June 1, 2026 19:45
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
Handle both 4-argument and 5-argument watch callback signatures so changed and removed file sets are forwarded correctly and callbackUndelayed receives file metadata.

Co-authored-by: Cursor <cursoragent@cursor.com>
@ScriptedAlchemy
ScriptedAlchemy force-pushed the llms-module-rspack-port-78c2 branch from 45bfde1 to a764e3c Compare June 1, 2026 17:49
@ScriptedAlchemy
ScriptedAlchemy requested a review from styfle June 1, 2026 20:48
Comment thread package.json Outdated
Comment thread package.json
Comment thread src/loaders/relocate-loader.js Outdated
Comment thread src/index.js Outdated
Comment thread src/index.js Outdated

@styfle styfle left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Each TS file rebuilds and type-checks the entire dependency graph + lib files. The old ts-loader kept a single shared Program/instance and type-checked once per build. This is a clear regression. TS modules are wrapped by uncacheable.js (this.cacheable(false)), so rspack's persistent cache can't amortize any of it — every build and every watch rebuild pays full cost. This cost applies to all TS builds, not just ones with errors.
  2. Duplicate diagnostics (correctness/UX). getPreEmitDiagnostics returns whole-program diagnostics, so one error in a shared module is reported once per importer. Verified through real ncc — a single shared.ts error printed 3× (once per program that includes it). ts-loader on main de-duplicates via its shared instance.
  3. Fragile emit invariant. program.emit() is called with no target file (ts-loader.js:80), so it emits every file in the program; outputText keeps only the last writeFile. This happens to be correct because the root file is emitted last (I verified across chained and circular imports), but it's an undocumented invariant and it serializes every other module's output for nothing.
  4. Double module resolution in beforeResolve — src/index.js:246-298. For every request, the hook calls resolver.resolve(...) to test existence, then on success returns control so rspack resolves the same request again — i.e. two full resolves per resolved module. The old webpack resolver-plugin wrapped the single resolve call, so it resolved once. Scales with graph size (the integration builds still pass, so it's not catastrophic, but it's measurable overhead on large graphs). Hooking the failed-resolution path instead of pre-resolving everything would avoid the duplication.

@ScriptedAlchemy

ScriptedAlchemy commented Jun 30, 2026

Copy link
Copy Markdown
Author

@styfle I pushed 6aaddc2 with the follow-up fixes:

  • Fixed the Windows ESM runtime path issue by decoding the asset base path, which should address the RUNNER%7E1 create-require-runtime failure.
  • Updated the TS loader so per-file output uses transpileModule, while full type-check diagnostics are collected once per compilation via finishModules. That removes the per-file whole-program emit/typecheck path and avoids duplicate shared-module diagnostics.
  • Updated the affected ESM output fixtures.

Local verification:

pnpm build
pnpm exec jest test/unit.test.js --runInBand
pnpm exec jest test/integration.test.js --runInBand --testNamePattern="create-require-runtime|request-ts|test.ts"
pnpm exec jest test/cli.test.js --runInBand --testNamePattern="ts-error|type"
git diff --check

For the resolver double-resolve point: I dug through Rspack's normal module factory path. I agree that the ideal shape is to handle this in the failed-resolution path rather than pre-resolving in beforeResolve, because the current hook probes the request once and then lets Rspack resolve successful requests a second time.

The blocker is that Rspack does not appear to expose that failed-resolution recovery point to JS plugins today. beforeResolve is pre-resolution and can only ignore or mutate the request, afterResolve/createModule only run after a successful resolution, and the JS bridge for normalModuleFactory.hooks.resolve currently maps a successful JS tap back to Ok(None), so JS cannot bail out with a recovered module/request there. The native failure branch that would be ideal is the Err(err) path in resolve_normal_module, after Rspack has collected file/missing dependencies and before it returns the resolver error.

So I left the existing beforeResolve behavior in place for this PR to preserve runtime-notfound semantics. The cleaner long-term fix would be a small Rspack API addition, such as a JS-exposed failedResolve/resolveError hook that can see NotFound, mutate the request to ncc's @@notfound.js?... module, and ask Rspack to retry. That would avoid double-resolution for successful modules while keeping ncc's runtime missing-module behavior.

I also refreshed checks: lint and both Socket checks are passing, and the Node CI matrix is now pending.

Comment thread package.json Outdated

@styfle styfle left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@styfle styfle left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  [
    {
      "file": "src/cli.js",
      "line": 377,
      "summary": "New .catch on the non-watch build path mishandles `ncc run` failures: it stringifies the rejection to '[object Object]', drops the child's real exit code, and violates
  the silent flag.",
      "failure_scenario": "`ncc run script.js` where script.js exits non-zero (e.g. throws) → handler's inner promise rejects with `{silent:true, exitCode:2}` → this catch runs
  `stderr.write(err+'\\n')` printing '[object Object]' despite silent, then `process.exit(1)` losing exit code 2. In api mode it also overwrites `err.exitCode = 1`, so callers/tests see 1
  instead of 2. Previously the rejection propagated to the top-level handler which honored `silent` and exited with `e.exitCode`."
    },
    {
      "file": "src/index.js",
      "line": 223,
      "summary": "The beforeResolve notfound-probe resolves with the generic `getResolver(\"normal\")`, which does not carry the per-dependency `conditionNames` (import/require) that only
  exist under `resolve.byDependency`.",
      "failure_scenario": "An ESM build imports a package whose package.json exposes its entry only via `exports: { import: ... }` (no `main`/default). rspack's real esm resolver would
  resolve it, but the probe's 'normal' resolver lacks the `import` condition, so the probe fails → `handleMissing` rewrites the request to `@@notfound.js?<req>` and rspack never attempts
  its own (correct) resolution  the module becomes a runtime MODULE_NOT_FOUND."
    },
    {
      "file": "src/index.js",
      "line": 234,
      "summary": "`isNotFoundError` classifies resolver failures purely by substring-matching the message ('NotFound' / \"Can't resolve\" / \"Cannot resolve\"); any other phrasing is
  treated as a hard error.",
      "failure_scenario": "If rspack's resolver returns an unresolvable-module error worded differently (e.g. 'Failed to resolve X' or a localized message), `isNotFoundError` returns false
  and both probe paths call `callback(err)`, turning what ncc intends to be a runtime not-found (`__non_webpack_require__`) into a hard build failure  a regression of ncc's core 'make
  not-found errors runtime errors' semantics."
    },
    {
      "file": "src/index.js",
      "line": 252,
      "summary": "beforeResolve resolves every non-builtin/non-external request an extra time (probe) before rspack performs the real resolution, doubling resolver work for all modules on
  all builds.",
      "failure_scenario": "For a large dependency graph, each import triggers a full `resolver.resolve` in the probe (only to decide notfound), then rspack resolves the same request again
  to actually build the module. The webpack implementation intercepted the single real resolution; the rspack workaround cannot, so resolution cost is ~2x on every build."
    },
    {
      "file": "src/index.js",
      "line": 513,
      "summary": "Error-message assembly strips every line whose trimmed text begins with 'at ', so an error message composed solely of stack-like lines collapses to an empty string.",
      "failure_scenario": "A compilation error whose `.message` is only stack frames (each line starting with 'at ') is filtered down to '', so `reject(new Error(errLog))` surfaces a blank
  error and the user sees a failed build with no explanation."
    },
    {
      "file": "src/loaders/ts-loader.js",
      "line": 33,
      "summary": "`stateKey = JSON.stringify(parsedOptions)` keys the shared type-check state on options resolved per-file via `convertCompilerOptionsFromJson(...,
  path.dirname(fileName))`, so relative tsconfig paths yield a different key per source directory.",
      "failure_scenario": "With a tsconfig using relative `baseUrl`/`paths`/`rootDir` and .ts files spread across directories, each directory produces distinct absolute-path options →
  distinct stateKey → `getTypeCheckState` builds a separate full `ts.Program` per directory in `finishModules` instead of one, multiplying type-checking CPU across the build."
    },
    {
      "file": "src/loaders/relocate-loader.js",
      "line": 32,
      "summary": "The `.json` branch in `wrappedRelocateLoader` is unreachable dead code and would bypass asset relocation if ever reached.",
      "failure_scenario": "The module rule test `/\\.(js|mjs|tsx?|node)$/` excludes `.json`, and rspack parses JSON natively, so this loader is never invoked for `.json`. The branch adds
  complexity for no effect; if a future rule change routed json here, it would return the file verbatim and skip the asset relocator entirely."
    },
    {
      "file": "src/loaders/relocate-loader.js",
      "line": 4,
      "summary": "`ensureMainTemplate` installs a fake `compilation.mainTemplate.hooks.requireExtensions` whose tapped functions are never executed; the pinned relocator (1.10) never reads
  `mainTemplate`.",
      "failure_scenario": "`@vercel/webpack-asset-relocator-loader@^1.10.0` uses `runtimeRequirementInTree`/`addRuntimeModule`, not `mainTemplate` (verified: no reference in the package).
  The shim collects taps into an array that is never invoked, so it is dead code that can mislead maintainers into thinking a `requireExtensions` hook is active."
    }
  ]

Rspack resolves in Rust and only reports the paths a resolver touched when a
request resolves, so requests ncc rewrites to @@notfound.js left nothing for
watch mode or the persistent cache to invalidate on. Derive the paths whose
creation could satisfy a missed request and register them instead, stopping
each candidate one segment past the deepest existing directory so rspack does
not end up watching a far ancestor recursively.

Also probe with the conditions of the dependency being resolved rather than a
single default resolver, classify resolver misses from error codes instead of
message substrings, keep compilation messages that look like stack frames,
share one TypeScript program across source directories, and let ncc run
child exit codes reach the CLI.
Track the bounded files that can satisfy rewritten resolver misses so watch mode and persistent cache recover when dependencies appear, including package mappings and TypeScript fallbacks. Restore syntax diagnostics, diagnostic deduplication, and declaration emission in the shared TypeScript program while removing an unreachable webpack parser shim.
@ScriptedAlchemy

ScriptedAlchemy commented Jul 24, 2026

Copy link
Copy Markdown
Author

Addressed styfle's latest requested changes and the follow-up parity audit in a40830e, b61042e, 9531676, 00f0dcf, and 9cfc8fb.

  1. Removed the non-watch .catch that overwrote ncc run child exit details; silent and the original exitCode now propagate.
  2. The resolver probe now uses dependency-aware ESM/CommonJS/URL/worker modes, with regression coverage for conditional exports, dynamic imports, TS requires, and URL assets.
  3. Replaced broad error-message matching with classification of known resolver miss codes/names/variants; non-missing errors defer to Rspack.
  4. The extra probe remains the accepted Rspack 1.7 limitation. Rspack's JS bridge cannot recover from failed resolution (resolve/factorize return Ok(None)). Measured overhead is ~5%; details are in [Feature]: expose a JS hook to recover from NormalModuleFactory resolution failures web-infra-dev/rspack#14640. This PR does not depend on an unreleased Rspack API.
  5. Compilation formatting preserves the original error when stack filtering would make it blank.
  6. TypeScript now uses one shared/reused ts.Program for type-aware output, diagnostics, and declarations. This restores ambient const-enum inlining, type-only re-export elision, declaration directories, syntax diagnostics in transpile-only mode, and diagnostic de-duplication. Tsconfig discovery uses the resolved entry, honors TS_NODE_PROJECT, and registers the config as a watched dependency.
  7. Retained the JSON guard after reproducing the pinned relocator's TDZ bug at asset-relocator.js:410; the comment now states that exact reason.
  8. Removed the unused mainTemplate shim and the unreachable webpack parser shim (Rspack's NormalModuleFactory has no parser hook).

Additional parity fixes:

  • Restored Node CommonJS failed-module eviction despite Rspack mapping strictModuleExceptionHandling to its ESM-style error cache. The rewrite is scoped to Rspack's generated require runtime, with a collision regression proving matching user code is untouched.
  • Watch mode no longer ignores bundled node_modules, and rewritten runtime-notfound requests register bounded file/missing dependencies for relative requests, TS fallback, mainFields, and supported imports/exports targets.
  • TypeScript diagnostic categories now preserve severity: only errors fail compilation, while warnings use warning channels.
  • Kept the .node reread after an attempted removal caused invalid ELF output for binary-require, canvas, and sharp in the integration suite.

Fresh verification on 9cfc8fb:

pnpm build
pnpm test --runInBand
Test Suites: 5 passed, 5 total
Tests:       144 passed, 144 total

The unit suite also passes twice consecutively with the persistent cache warm. @styfle could you please re-review when you have a chance? The CI/PR workflows are waiting for maintainer approval.

Preserve Node CommonJS retry semantics despite Rspack's strict-error option alias, and use a shared TypeScript Program for type-aware output, declarations, and diagnostics. Track discovered tsconfig files and update generated runtime fixtures to reflect the intentional cache-eviction behavior.
Route only error-category diagnostics to compilation failures while reporting warnings through the warning channels, matching ts-loader behavior without weakening syntax or config error handling.
Reuse package candidate and diagnostic reporting helpers, cache negative external lookups and filesystem metadata, and centralize watch-test plumbing without changing behavior.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants