Repository navigation
feat(build): restore string externals and add build.ignore option - #82
Conversation
Fixes a regression from v0.6.4 in which user-configured string externals were silently ignored because leading the externals array with a function caused rspack to stop consulting the trailing string entries. The entire externalization decision is now folded into a single function. Adds a new build.ignore config option: a string array of module names to exclude from the bundle and replace with a throwing ESM stub. Use it for optional transitive dependencies the action never exercises (e.g. native modules pulled in as optional plugins). Modules in build.ignore throw at runtime so try/catch-guarded optional require calls detect them as absent. build.ignore takes precedence over build.externals. Closes #81. Signed-off-by: C. Spencer Beggs <spencer@beggs.codes>
![savvy-web-bot[bot]](https://avatars.githubusercontent.com/in/1855974?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
Current Commit: a4f3e01
Summary
This is a clean, well-scoped PR. The regression fix is correct, the new feature is semantically well-designed, and the integration tests are exactly the right kind of evidence for both. All CI checks pass (Tests, Code Quality, Conventional Commits, PR Title Validation, Markdown, CodeQL — all ✅).
Checks
| Check | Status |
|---|---|
| Tests | ✅ success |
| Code Quality | ✅ success |
| Conventional Commits | ✅ success |
| PR Title Validation | ✅ success |
| Markdown | ✅ success |
| CodeQL | ✅ success |
The externals regression fix
The root cause analysis is correct: when rspack externals is an array and the first entry is a function, rspack's array fall-through stops after the function returns a non-false value for any request — trailing string entries never get consulted. Folding everything into a single function eliminates the dependency on that undocumented array-traversal behavior entirely. The fix is minimal and clearly explained in the inline comment.
The guard !config.build.ignore.includes(request) on line 174 is necessary and correct. Because rspack's externals check runs before alias resolution, without that guard a module in both ignore and externals would be externalized (skipping alias substitution entirely), rather than stubbed. The behavior matches what the docs describe.
The build.ignore feature
The semantic distinction between externals (available at runtime) and ignore (absent at runtime, replaced with a throwing stub) is well-drawn and the docs' comparison table makes it immediately clear. The fixture vendor/optional-loader.cjs mirrors the real-world CycloneDX/optional-native-dep pattern closely, making the integration test a meaningful proof of the end-to-end behavior. The stub error message ("A module excluded via the build 'ignore' option was loaded at runtime.") is descriptive and actionable.
Changeset is correctly typed as minor — regression fix + new feature, no breaking changes.
Issues
Two inline comments are attached. In brief:
-
Subpath imports (line 132) — the
$-suffixed alias only stubs exact module-name imports;require("bufferutil/fallback")would still fail the build. The stated use cases (top-level optional native modules) almost certainly never use subpath imports, so this may never bite anyone in practice. But the docs should at least mention the constraint so users who hit it aren't left guessing. -
Stub written per entry (line 135) — minor I/O redundancy in multi-entry builds; nit-level and safe to address in a follow-up if desired.
There was a problem hiding this comment.
Pull request overview
This PR addresses a bundling regression where string entries in build.externals were ignored after the node: interop fix, and introduces a new build.ignore option to intentionally stub out optional (runtime-absent) dependencies while keeping builds resolvable.
Changes:
- Fixes
build.externalshandling by consolidating externalization logic into a single rspack externals function (restoring string externals behavior). - Adds
build.ignore: string[]to config schema + docs, implemented viaresolve.aliasto a throwing stub module (withignoretaking precedence overexternals). - Adds integration + unit tests and updates documentation/spec artifacts and release changeset.
Reviewed changes
Copilot reviewed 15 out of 18 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/services/build-live.ts |
Implements consolidated externals decision function and build.ignore stub/alias mechanism. |
src/schemas/config.ts |
Adds build.ignore to resolved and input config schemas; updates example. |
src/schemas/config.test.ts |
Unit tests for build.ignore defaults and decoding. |
README.md |
Updates feature bullets/headings and mentions externals/ignore. |
docs/superpowers/specs/2026-05-15-build-ignore-option-design.md |
New/updated design spec describing build.ignore semantics and mechanism. |
docs/superpowers/plans/2026-05-15-build-ignore-option.md |
Implementation plan write-up for the feature and regression fix. |
docs/configuration.md |
Documents build.ignore and clarifies externals semantics; adds examples and comparison table. |
.claude/design/github-action-builder/architecture.md |
Updates architecture/config documentation to include ignore and the new bundler behavior. |
.changeset/quiet-roads-return.md |
Changeset for the externals regression fix + new build.ignore feature. |
__test__/integration/string-externals.int.test.ts |
New integration regression test for issue #81 (string externals). |
__test__/integration/ignore-modules.int.test.ts |
New integration test verifying build.ignore stubbing behavior end-to-end. |
__test__/integration/fixtures/string-externals/action.yml |
Fixture manifest for string externals integration test. |
__test__/integration/fixtures/string-externals/src/main.ts |
Fixture source that imports a missing external module to force the regression case. |
__test__/integration/fixtures/string-externals/uninstalled-peer-dep.d.ts |
Ambient type declaration for the missing external module used by the fixture. |
__test__/integration/fixtures/ignore-modules/action.yml |
Fixture manifest for ignore-modules integration test. |
__test__/integration/fixtures/ignore-modules/src/main.ts |
Fixture entry importing a CJS loader that conditionally requires an ignored module. |
__test__/integration/fixtures/ignore-modules/vendor/optional-loader.cjs |
CJS optional-dependency loader using try/catch around require(). |
__test__/integration/fixtures/ignore-modules/vendor/optional-loader.d.cts |
Types for the CJS loader fixture module. |
| | Option | Type | Default | Description | | ||
| | --- | --- | --- | --- | | ||
| | `minify` | `boolean` | `true` | Minify output for smaller bundles | | ||
| | `target` | `string` | `"es2022"` | ECMAScript target version | | ||
| | `sourceMap` | `boolean` | `false` | Generate source maps | | ||
| | `externals` | `string[]` | `[]` | Packages to exclude from bundle | | ||
| | `externals` | `string[]` | `[]` | Packages to leave out of the bundle; must be available at runtime | | ||
| | `ignore` | `string[]` | `[]` | Packages to leave out of the bundle and replace with a stub that throws if loaded | | ||
| | `quiet` | `boolean` | `false` | Suppress build output | |
There was a problem hiding this comment.
Confirmed: target and quiet are not currently in BuildOptionsSchema (checked src/schemas/config.ts). This is a pre-existing documentation gap — not introduced by this PR. Leaving open as a follow-up candidate.
- Write the build.ignore stub into a private mkdtemp directory instead of a predictable shared temp path, removing a symlink and race risk, and remove the directory after the bundle is written - Document the exact-match limitation of build.ignore (subpath imports of an ignored module are not stubbed) in the configuration docs - Precompute Sets for the externals and ignore lookups in the externals callback - Correct the design spec wording for the empty-ignore case Signed-off-by: C. Spencer Beggs <spencer@beggs.codes>
Code ReviewCurrent Commit: f9f31bc SummaryThird-pass review. The new commit ( Validation Results
(Results from commit What Changed Since the Second ReviewCommit
All changes are presentation-only and look clean. Thread StatusAll four threads from the first-pass review are resolved:
Two Copilot threads remain open:
Overall AssessmentNothing to add on top of the second review. The implementation is solid, both threads I flagged are cleanly resolved, and the docs normalization commit is tidy. The Copilot This PR is ready to merge. |
- Rename docs/ pages to the NN-slug.md convention and add docs/README.md as a table-of-contents page - Rewrite title-case headings to sentence case across the docs - Join multi-line paragraphs onto single source lines - Add the standard badge block to the README - Add expected-output comments to programmatic examples and replace fabricated build-summary numbers with placeholders Signed-off-by: C. Spencer Beggs <spencer@beggs.codes>
Rewrite AI-tell prose (promotional phrasing, filler, forced rule-of-three, sycophantic pleasantries, copula avoidance) into more direct, specific wording across the README and all six docs pages. Code blocks, headings, badges, and examples are unchanged. Signed-off-by: C. Spencer Beggs <spencer@beggs.codes>
Signed-off-by: C. Spencer Beggs <spencer@savvyweb.systems>
Signed-off-by: C. Spencer Beggs <spencer@savvyweb.systems>
Summary
build.externalsstring entries were silently ignored in v0.6.4. Leading the rspack externals config with a function caused rspack to stop consulting trailing string entries; configured packages were bundled and hard-failed to resolve. Fixed by folding the entire externalization decision into a single function.build.ignore: Astring[]config option that replaces listed modules with a throwing ESM stub. Use it for optional transitive dependencies the action never exercises (native modules, optional plugins). Unlikebuild.externals(excluded because they are available at runtime), ignored modules are absent at runtime — code that wraps theirrequire()calls intry/catchcorrectly detects them as unavailable.build.ignoretakes precedence overbuild.externals.Test plan
pnpm test— 97 unit + 4 integration tests pass (includes two new regression tests:string-externals.int.test.tsguards theexternalsfix;ignore-modules.int.test.tsguards thebuild.ignorefeature end-to-end)pnpm run typecheck— cleanpnpm run lint— cleanCloses #81.
Signed-off-by: C. Spencer Beggs spencer@beggs.codes