feat(plugins): import tracking, options bag, and preload coordination#20
Open
exo-nikita wants to merge 6 commits into
Open
feat(plugins): import tracking, options bag, and preload coordination#20exo-nikita wants to merge 6 commits into
exo-nikita wants to merge 6 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a comprehensive node:test suite for the Stasis webpack plugin (lock add/frozen/replace/ignore, bundle add/replace, scope variants, config conflicts, tamper detection) along with two fixture projects, and modernizes src/webpack.js to use the hooks.tap() API with node:assert/strict, recording imports via state.addImport and tagging entries via addFile(..., { isEntry }).
Changes:
- Rewrite
src/webpack.jsto usecompiler.hooks.normalModuleFactory/nmf.hooks.afterResolvetaps, trackisEntry, and callstate.addImport(parentURL, rawRequest, url). - Add
tests/webpack.test.jsandtests/webpack-run.helper.jsrunning webpack in a child process with a freshStateper test. - Add
webpack-fullandwebpack-nmfixtures (with committedstasis.config.jsonandstasis.lock.json).
Reviewed changes
Copilot reviewed 13 out of 17 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/webpack.js | Switch from legacy plugin() callbacks to hooks.tap(), record imports and entry flag |
| tests/webpack.test.js | New 300+ line test suite covering lock modes, bundle, tampering, scopes, conflicts |
| tests/webpack-run.helper.js | Child-process helper that constructs State, runs webpack with the plugin, then writes state |
| tests/fixtures/webpack-full/* | Full-scope fixture: entry, hello, package.json, committed config & lockfile, pnpm-workspace marker |
| tests/fixtures/webpack-nm/* | node_modules-scope fixture: entry/helper, fake-cjs-pkg under node_modules, committed config & lockfile |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
23c5b5f to
a55e84a
Compare
Migrate StasisWebpack to the .hooks API and fill in the previously
TODO import-recording path: each after-resolve event now calls
state.addImport(issuer, rawRequest, resource) for non-entry resolves,
and state.addFile(url, { isEntry: !issuer }) marks the entry on its
first sighting.
Mirror the esbuild test suite: tests/webpack-run.helper.js drives a
webpack build with the StasisWebpack plugin against a fixture cwd,
then writes state. New fixtures webpack-full and webpack-nm cover
lock=add/frozen/replace/ignore in full and node_modules scope,
package.json metadata mismatch, brand-new entries under lock=frozen,
hash mismatch (src and node_modules), bundle=add/replace including
tamper detection, and env-vs-config scope conflict.
Config now reads env vars at construction time and accepts a plain-object options bag with the same shape as the CLI flags. When both env and an option set the same key they must match, otherwise 'Plugin options can not override stasis env' is raised. State forwards options through to Config. Both StasisWebpack and StasisEsbuild now take a constructor options argument and support every CLI mode (lock=add|replace|frozen|ignore|none, bundle=add|replace|load|ignore|none, plus bundleFile/scope/debug). If State.instance already exists (preload path) the plugin asserts its config matches the options; otherwise the plugin constructs State with them. Tests: parallel option-driven coverage for both plugins via a STASIS_TEST_PLUGIN_OPTIONS env var the helpers forward to the plugin constructor. Covers every lock and bundle mode, bundleFile, env-vs-option conflict, unknown option, and invalid value.
a55e84a to
344e72d
Compare
Implements the plugin↔preload rules described in PR #20: 1. Plugin lockfile mode (other than 'none' / 'ignore') without an active preload is a hard throw; the lockfile would otherwise silently miss every dependency the bundler itself pulled in. 2. With a preload, the plugin's lock must agree with the preload's. 3. Lockfile is unified: when plugin and preload agree, the plugin reuses the preload State so one lockfile is written. 4. With a preload that has bundle on, the plugin can't disable bundle and the bundle mode must match; only bundleFile may differ. 5. Same bundleFile (or unspecified) reuses the preload. 6. Different bundleFile constructs a sidecar State -- shares the preload's hashes/entries/modules by reference so lockfile coverage stays unified, but has its own sources/formats/imports/resources and writes only its own bundle file. 7. lock='none' AND bundle='none' with no preload is a no-op plugin (no hooks, no State) -- useful for env/flag-controlled builds. Surface: - src/state.js: State constructor accepts { parent: preload } for sidecar mode (mutually exclusive with preload: true). Sidecar shares hashes/entries/modules by reference; only the bundleFile differs. write() on a sidecar skips the lockfile. New exported helper resolvePluginState(label, options, cwd) encodes all seven rules and is the single entry point for plugin constructors. - src/{webpack,esbuild}.js: constructors delegate to resolvePluginState; no-op State means no hooks registered. - tests/{webpack,esbuild}-run.helper.js: by default construct a preload State (simulating loader-active) with options mirrored from the plugin's. STASIS_TEST_PRELOAD=0 opts a test out of preload to exercise standalone / noop / hard-throw paths. - tests/state-sidecar.test.js: 9 tests for sidecar mechanics + the rule 2/4/5/6 paths of resolvePluginState. - tests/state-resolve-no-preload.test.js: rule 1 (hard throw on every non-ignore lockfile mode) + rule 7 (noop). - tests/{webpack,esbuild}.test.js: rule 1 + rule 7 coverage through the full bundler harness. 275/275 tests pass, lint clean.
…ug conflicts Addresses three items from the b08876a review: 1. (bug) Sidecar and standalone State writes never reached disk in production. The loader registers beforeExit/exit hooks only for the preload (src/loader.js); the plugins didn't register anything for non-preload States, so rule 6 (sidecar with a separate bundleFile) silently produced no bundle output despite being its entire point. StasisWebpack now hooks compiler.hooks.done and StasisEsbuild hooks build.onEnd to call state.write() whenever state !== State.preload. 2. (bug) resolvePluginState silently dropped plugin scope/debug on sidecar paths. A user passing { scope: 'node_modules' } while the preload was 'full' got no error and a sidecar with the preload's scope. The function now rejects scope/debug disagreements at the top of the preload-active branch alongside the existing lock check. 3. (clarity) The helper auto-preload mirrors plugin options onto the preload, so most env-driven and option-driven test cases exercise the rule-3/5 reuse path even when test names suggest otherwise. Added a leading comment in tests/{webpack,esbuild}.test.js and an expanded usage block in the helper files. The helpers now also honor STASIS_TEST_PRELOAD_OPTIONS so a test can construct a preload with options distinct from the plugin's -- exercised by the new sidecar end-to-end tests. New end-to-end coverage: - 'plugin standalone with bundle writes the bundle via done/onEnd' proves the plugin's exit hook writes the bundle in the standalone case (no preload). - 'sidecar bundle (rule 6) is emitted by the plugin alongside preload bundle' proves both bundles land on disk and the lockfile is unified when preload and plugin pick different bundleFile paths. 279/279 tests pass, lint clean.
Addresses six items from the fresh plugin review:
1. (bug) Failed builds no longer overwrite the user's lockfile/bundle.
Webpack's done hook and esbuild's onEnd both fire on errors as well
as success; the plugins now check stats.hasErrors() and
result.errors.length and skip state.write() when the build failed.
Otherwise lock=replace on a broken compilation would clobber a good
lockfile with a partial one.
2. (bug) StasisEsbuild crashed on any extension outside
js/ts/jsx/tsx/json. The onLoad handler now maps known extensions
(incl. css and text) to their esbuild loaders and falls back to the
'default' loader for everything else, so CSS/asset/extensionless
files work instead of crashing the build.
3. (bug) StasisEsbuild rejected legal import
attributes. The onLoad handler now permits the attribute
(still tripwiring anything else, since saving import-attribute
state to the lockfile isn't designed yet).
4. (bug) StasisEsbuild rejected any resolution that produced a
warning -- including benign deprecation notices from sibling
plugins. The assert.equal(res.warnings.length, 0) check is dropped;
warnings propagate upward as warnings.
5. (bug) StasisWebpack asserted data.resource === path.resolve(data
.resourceResolveData.path), which trips for any resource carrying
a query string (./foo.js?raw) or inline-loader prefix. It also
forwarded non-filesystem resources (data: URIs, null-loader output,
virtual modules from other plugins) into state.addFile where they
crashed existsSync. The assertion is gone; the plugin now uses
resourceResolveData.path, skips non-absolute / non-existent
resources, and reads + sniffs binary content so non-UTF8 assets
(.png, .wasm, ...) route through state.resources via isBinary
instead of failing addFile's isUtf8 assertion.
6. (bug) resolvePluginState rejected plugin bundle='ignore' under a
no-bundle preload: assertOptionsMatchConfig compared preload's
bundleMode='none' against the plugin's 'ignore' and threw. Both
mean 'plugin writes no bundle', so the bundle field is now
excluded from the cross-check on that reuse path.
(polish) Rule-1 error message reads 'the default lockfile mode (add)'
when the user supplied no lock option, instead of attributing 'add'
to them.
Tests added:
- failed-build-no-clobber for both bundlers (webpack and esbuild)
- esbuild .css import via 'default' loader
- esbuild import attributes with { type: 'json' }
- preload-without-bundle + plugin bundle='ignore' (regression test)
- the no-options branch of the rule-1 error message
289/289 tests pass, lint clean.
The previous fix made the esbuild plugin tolerate non-JS/JSON files by falling back to esbuilds default loader, and the webpack plugin auto-sniffed binary content -- but both still called state.addFile for every file, so .css/.png/.wasm/.txt all ended up in the lockfile. That widened coverage past what the lockfile semantics actually support: the Node loader asserts the format is module/commonjs/json when serving from a bundle, so any non-JS-shaped file we attest to has no path to be loaded back. Now both plugins consult a shared isTrackedPath(filePath) helper from state.js and skip addFile + addImport for untracked extensions. The bundler still compiles them through its own loader machinery (esbuilds default loader, webpacks asset modules / loaders); the lockfile and bundle just stop attesting to bytes they can never round-trip. Tracked extensions: js, mjs, cjs, ts, jsx, tsx, json. Tests: - esbuild: untracked .css compiles, but is absent from the lockfile, while the .js entry stays present. - webpack: same shape with a .css file present in the workspace -- asserts only tracked files appear in lockfile.sources. 290/290 tests pass, lint clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three stacked changes, on top of the now-merged Config refactor (#30) and the multi-State groundwork (#31).
1.
StasisWebpack: record imports and entry statusMigrated to webpack's modern
compiler.hooks.normalModuleFactory.tap/nmf.hooks.afterResolve.tapAPI.afterResolvederivesisEntry = !issuerand records bothstate.addImport(parentURL, rawRequest, url)for every non-entry edge andstate.addFile(url, { isEntry })once per file. New fixtures (webpack-full,webpack-nm) and a spawning helper.2. Both plugins accept a constructor options bag
new StasisWebpack({ lock, bundle, bundleFile, scope, debug })andnew StasisEsbuild({ ... })accept the same keys as the CLI flags. Validation goes throughvalidatePluginOptionsfrom #30.3. Plugin↔preload coordination rules
The plugin used to assert "stasis preload is not active" which blocked the standalone use case. We now codify a richer contract via a new
resolvePluginState(label, options, cwd)helper:lockother thannone/ignore, no preloadbundle: 'none'/'ignore'bundleFileunset or matches preload'sbundleFilediffers from preload'sState— shareshashes/entries/moduleswith preload by reference (unified lockfile), has its ownsources/formats/imports/resourcesand writes only its own bundle filelock: 'none'ANDbundle: 'none'with no preloadSidecar
Stateis enabled by the newparent: preloadoption on theStateconstructor (mutually exclusive withpreload: true);write()on a sidecar emits the bundle but skips the lockfile (the preload owns it).The plugin's
setup/applyshort-circuits whenstate === null(rule 7), registering no hooks.Depends on
validatePluginOptions/assertOptionsMatchConfigState.preloadand multi-State supportTest plan
node --run lint— cleannode --run test— 275/275 passtests/state-sidecar.test.js— sidecar mechanics: shared hashes/entries/modules, independent sources, sidecarwrite()emits bundle but not lockfile, rule-2/4/5/6 paths ofresolvePluginStatetests/state-resolve-no-preload.test.js— rule 1 (hard throw acrossadd/frozen/replace) + rule 7 (noop)tests/{webpack,esbuild}.test.js— rule 1 + rule 7 end-to-end through the actual bundler harness; existing plugin tests adapted to run under an auto-preload (helpers now construct a preload by default;STASIS_TEST_PRELOAD=0opts out)lock=frozen, bundle=loadEXODUS_STASIS_LOCK=frozenandnew StasisWebpack({ lock: 'add' })together raiseConfig options can not override stasis env