Repository navigation
fix(unplugin): exclude the bundler's output directory from discovery - #411
Merged
Merged
Conversation
The lane's exclude list replaces the default `dist` exclusion; with a `.js` entry name the bundle is a discovered source. Two new builds: the second must fail under strict, naming the file and the exclude fix. One watch: its rebuild must emit the first build's stylesheet and read no output. Red on main: the second build passes and the sheet grows from 7,281 to 38,810 bytes. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
unplugin never set the Session's structural exclusion, so an `exclude` list (which replaces the default `dist`) or a non-`dist` output directory let a build analyse the previous build's output. webpack, rspack, esbuild and Vite name the output directory before analysis; it is excluded exactly. Rollup and Rolldown name it only at renderStart: later builds in the same process exclude it, and a build whose analysis already read a file there warns, naming the files and the directory to add to `exclude`, and fails under strict outside a watch. The README stated that `exclude` merges with the defaults; a list replaces `dist`, `.test.` and `.spec.`. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
…clude # Conflicts: # packages/unplugin/src/core.ts
…tself dx-codex's #411 review round. vite-plugin's exclusions row builds twice: an `ssr` environment with its own `outDir`, and `rolldownOptions` outputs `esm` and `cjs`; none of their files is analysed. rollup-app's own-output leg writes `output.file` inside `src/`: the first build succeeds, and a new build that read the bundle fails under strict naming that file alone. Red on c8cafcaf: the per-environment row analyses `server-out/entry.js`, and the first build beside sources fails naming every source in `src/`. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
dx-codex's #411 review. Output targets are the directories a bundler fills and the files it writes by exact name (extract/pipeline output-targets.ts); the Session's structural exclusion and the Rollup family's render check both read them, and vite-plugin's exclusion now does too instead of its own narrower reader. - Vite: every environment's outDir, and each rolldownOptions/rollupOptions output's dir, as Vite's getResolvedOutDirs reads them (vite-plugin and unplugin's Vite host). - webpack: output.path up to its first template placeholder segment. - An output file (Rollup output.file, esbuild outfile) is excluded by its own name, at the root too, and never stands for its directory. - The render check reports a file target the analysis read as earlier output, and files inside an output directory as earlier output or sources beside the output, never a source as the build's own output. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
webpack writes under its path's fixed prefix; an output file at the root is excluded by its own name while a root directory excludes nothing; an output file beside sources names itself alone, and a directory over sources is reported as such. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
…clude # Conflicts: # packages/extract/pipeline/index.ts
…clude # Conflicts: # packages/unplugin/src/core.ts # packages/vite-plugin/src/config.ts
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.
Gut-check item B1, ticket 12. Over-emission, not flagged first (board-ui's ruling, recorded on ticket 12); a full review all the same.
Problem
unplugin never set the Session's
structuralExclude. Its output directory was excluded only through the defaultdistpattern, which a userexcludelist replaces. So a project with its own exclude list, or with an output directory not nameddist, had its previous build's output analysed on the next build.On main, e2e/rollup-app (its
exclude: ['fixtures/**']), built twice with a.jsentry name: the second build analyses the bundle. ItscreateElement(target)turns whole-component removal off, andanimus.cssgrows from 7,281 to 38,810 bytes with no source change. The build warns twice, naming the bundle. The lane's ownbundle.mjsname hid it, because.mjsisn't a default extension.Subset check (board-ui's condition). The first build's CSS is a subset of the second's with identical values: all 56 of its leaf rules are in the second build's 520, with identical declarations and the same relative order. A one-value mutation of the first sheet is caught by the same check. Run-and-discard; the script is not retained.
Fix
output.path, read per run after webpack applies its defaults;outdirordirname(outfile);build.outDir). It goes intostructuralExcludethrough the sharedoutputExclusion, as the Vite and Next plugins already do.renderStart, after analysis. Later builds in the same process exclude it. A build whose analysis already read a file there warns, naming the files and the directory to add toexclude; understrictoutside a watch, it fails. There is no cross-process record, by root's decision: a new process over stale output reports instead of excluding.excludewas documented as merging with the defaults; a list replacesdist,.test.and.spec..Proof
e2e/rollup-app/scripts/assert-own-output.mjsto the lane'sverify:assert. It pins the contract that a build never silently analyses its own output. On main it fails 4 of 5 checks: the second new build passes silently, and the watch rebuild's sheet grows from 7,281 to 38,810 bytes. On fbbe5fb it passes 5 of 5.distdirectory, with the lane's exclude list.[animus] this build analysed its own earlier output: own-rolldown/bundle.js. Add 'own-rolldown' to the plugin's \exclude` option.`Limits
Review round (dx-codex at fbbe5fb), folded into this PR
One model of where a bundler writes,
extract/pipeline/output-targets.ts: the directories it fills and the files it writes by exact name. The Session's structural exclusion and the Rollup family's render check both read it, and so does vite-plugin's own exclusion (config.ts), which had the same narrowbuild.outDirreader.outDir, and eachrolldownOptions/rollupOptionsoutput'sdir(which environments inherit), as Vite'sgetResolvedOutDirsreads them. Applies to vite-plugin and unplugin's Vite host.output.pathis cut at its first template placeholder segment (out/[fullhash]→out).output.fileandoutfileare excluded by their own name, at the root too. A directory that is the root excludes nothing.Red first (b63ff26, against #411's head):
exclusions.test.ts: anssrenvironment with its ownoutDir, andesm/cjsoutputs. On fix(unplugin): exclude the bundler's output directory from discovery #411's head it analysedserver-out/entry.js.assert-own-output.mjs:output.fileinsidesrc/. The first build must succeed; a new build that read the bundle fails under strict naming that file alone. On fix(unplugin): exclude the bundler's output directory from discovery #411's head the first build failed, naming every source.Green:
Merge note: main was merged in (no force-push). The only conflict was the host state's
servedfield from #408, kept alongsideanalyzedFiles.🤖 Generated with Claude Code