fix(build): keep test files out of the page globs and the bundle - #328
Merged
Merged
Conversation
`pages/**/*.tsx` matched `*.test.tsx` at both glob sites — the host's
pages.ts and the module globs the manifest generator emits. Two
consequences: the resolver registered a phantom page ("Error.test"), and
Vite followed the import into the production build, shipping the test
file and dragging @testing-library into a vendor chunk.
Measured on one branch with only the glob differing:
vendor chunk 1,073 kB -> 555 kB (gzip 310 -> 175 kB)
total build 1,848 kB -> 1,328 kB raw
test files in the bundle 3 -> 0
files with console.log 2 -> 0
Every visitor was downloading ~135 kB gzipped of test tooling, and those
were the only console.log calls in the whole production build.
Excluding the file from the *page* glob does not stop it being a test:
vitest has its own include pattern and still runs Error.test.tsx (5
tests; 463 total unchanged).
The glob rule moves to page_globs.py — adding the negations pushed
manifest.py to 302 lines, over the cap, and glob semantics is a separate
responsibility from writing the four generated files.
Claude-Session: https://claude.ai/code/session_01CwgTb8hULSfHoW2DrFrQAW
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Deploying simple-module-python with
|
| Latest commit: |
bf84179
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://84884d80.simple-module-python.pages.dev |
| Branch Preview URL: | https://fix-exclude-test-files-from.simple-module-python.pages.dev |
Extend the test-file exclusion to the two places the review found it missing: - make doctor's SM003 orphan-page check (collect_tsx_pages) globbed pages/**/*.tsx with no exclusion, so a colocated Browse.test.tsx — the exact pattern the Vite glob now supports — was reported as an orphan page. The suffix list is duplicated rather than imported: hosting depends on core, so importing hosting.page_globs here would invert the layering. Regression test covers .test.tsx and .spec.tsx. - A scaffold test now ties the CLI's pages.ts template to the host's checked-in pages.ts, so the two cannot drift apart silently. Deferred, deliberately: vite.config.ts optimizeDeps.entries still globs pages/**/*.tsx for the dev-server dependency crawler. It is dev-only, never reaches the production bundle, and the proper fix touches four files (host + scaffold vite.config.ts and module-assets.ts) — a follow-up, not part of this fix. Claude-Session: https://claude.ai/code/session_01CwgTb8hULSfHoW2DrFrQAW
Owner
Author
|
Ran the full review → QA → verify pipeline on this branch. One commit added: Review found two places the original fix had missed the same pattern:
QA proved no real page was lost, which is the one way a glob negation can go wrong: asked Vite directly for its resolved sets — host glob 7 → 6 (only Verify: rebased on |
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
1 participant
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
pages/**/*.tsxmatched*.test.tsxat both glob sites — the host'shost/client_app/pages.tsand the module globs the manifest generator emits. Two consequences:Error.test), which is also why it showed up in the resolvable-pages list.@testing-libraryinto a vendor chunk.Found while auditing browser console output on an unrelated branch: the only
console.logcalls in the entire production bundle turned out to be testing-library's (screen.debug,logTestingPlaygroundURL).Impact
Measured on one branch with only the glob differing, so the numbers are attributable:
.js,.br,.gz)console.logEvery visitor was downloading roughly 135 kB gzipped of test tooling.
The fix
Both glob sites now exclude
*.test.tsxand*.spec.tsx, anchored on the same base as the include — Vite resolves a negation relative to the importing file just like the include, so an exclusion written against a different base silently matches nothing. The scaffold template gets the same treatment, so newly-scaffolded apps don't inherit the bug.Excluding the file from the page glob does not stop it being a test. Vitest has its own include pattern and still runs
Error.test.tsx— 5 tests, and the suite total is unchanged at 463.The glob rule moves into a new
page_globs.py: adding the negations pushedmanifest.pyto 302 lines, over the 300-line cap, and per CLAUDE.md the rule is to split by responsibility rather than squeeze. Glob semantics is genuinely separate from writing the four generated files.manifest.pyre-exports_glob_pattern_forso its caller and tests are unaffected.Tests
Three new tests in
test_manifest.py, all of which fail without the fix:!…test.tsx/!…spec.tsxnegationspages.tscarries the same negations — the two must agree onTEST_FILE_SUFFIXES, and nothing else would catch them drifting apartVerification
make lintgreen · 2979 pytest · 463 vitest · clean rebuild confirms 0 test files and 0console.loginhost/static/dist.Test plan
npm run build --workspace host/client_appand confirms no*.test.*inhost/static/dist/assets/npx vitest run host/client_app/pages/Error.test.tsxand sees it still passhttps://claude.ai/code/session_01CwgTb8hULSfHoW2DrFrQAW