fix(components): scope the browser process shim to the package source - #6834
Merged
Conversation
`src/global.d.ts` declared `const process: { env: { NODE_ENV: string } }`, and
`tsconfig.test.json` globs `src/**/*.d.ts` in for the ambient declarations its
tests rely on. That project sets `"types": ["node"]`, so `@types/node` is in the
program — but the ambient declaration REPLACES the node global rather than
augmenting it, and because `@types/node` spells its module as `export = process`
it also became what `import process from 'node:process'` resolved to. Every
spelling of `process.cwd()` in this package's tests failed with
`TS2339: Property 'cwd' does not exist on type '{ env: { NODE_ENV: string; }; }'`
while `types: ["node"]` sat in the config, correct and configured.
Measured before choosing a repair, with `tsc --listFiles` on both projects: the
SOURCE project contains zero `@types/node` files and the test project contains
82, and an ablation of the declaration turned the source project red with five
`TS2591: Cannot find name 'process'` (renderers/basic/div.tsx,
renderers/basic/span.tsx, renderers/form/form.tsx x3). The shim is load-bearing
for the source, so it is narrowed rather than deleted:
- the declaration moves to `src/browser-process-shim.d.ts`;
- `tsconfig.test.json` names that one file in `exclude`;
- `src/__tests__/browser-process-shim-scope.test.ts` pins both halves. Its own
compilation under `tsc -p tsconfig.test.json` is the compile-time pin — a
runtime assertion cannot see this defect, because all three spellings always
worked at runtime.
No release: declaration files and `tsconfig.test.json` are checking-only inputs
and the built `dist/**/*.d.ts` carries no `process` declaration at all.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
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
2 participants
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.
Fixes #6809
packages/components/src/global.d.tsdeclared an ambient browser shim,const process: { env: { NODE_ENV: string } }, andpackages/components/tsconfig.test.jsonglobssrc/**/*.d.tsin for the ambient declarations its tests rely on. That project sets"types": ["node"], so@types/nodeis in the program — but the ambient declaration replaces the node global rather than augmenting it, and because@types/nodespells its module asexport = processit also became whatimport process from 'node:process'resolved to. The result was a self-contradicting diagnostic, identical for all three obvious spellings:The plain
join(process.cwd(), …)idiompackages/i18n's ratchet tests use happily simply did not compile one directory over.The measurement that chose the repair
The card refused to recommend one of its three directions and asked for a measurement first: is the shim still load-bearing at all? It is — so deleting it was never available, and this PR narrows it instead.
1. What in the package source reads
process— grep overpackages/components/src, tests excluded:src/renderers/basic/div.tsxsrc/renderers/basic/span.tsxsrc/renderers/form/form.tsxsrc/renderers/form/form.tsxFive reads of the bundler-replaced
process.env.NODE_ENVidiom, three files. Control for the zero-hit discipline: the same grep over the same tree hitsReactin 240 files andNODE_ENVin 10, so a zero here would have been a reading rather than a broken query. It was not zero.2. Whether those reads have any other source of typings —
tsc --listFileson both projects, counting@types/nodefiles in each program:@types/nodefilessrc/global.d.ts(control)tsconfig.json(source)tsconfig.test.json(tests)The source project names no
typesand@types/nodeis not reachable frompackages/components/node_modules, so its program contains zero node typings. The control —src/global.d.tsitself — is present in both, so the 0 is a measurement of the program, not of a query that failed to run.3. Ablation — the declaration removed, the source project recompiled:
Mutation confirmed on disk before reading the result (
declare const processcount 1 → 0, blob1129a9ae→d06edf62); restored afterwards to a byte-identical tree (git diff HEADempty, blob back to1129a9ae).⇒ The shim is load-bearing for the source and must stay. Adding
"types": ["node"]totsconfig.jsoninstead would be the wrong repair — it hands a browser library the wholefs/path/child_processsurface. So: direction 1, narrow the shim to the source project.The change
src/browser-process-shim.d.ts— new. Carries the declaration, and a header stating why it is source-only and why it must not move back.src/global.d.ts— the declaration removed; a pointer left in its place. What stays in this file reaches both projects, which is exactly the property that made the shim wrong here.tsconfig.test.json—"exclude": ["src/browser-process-shim.d.ts"]. The separation is mechanical, not a convention: theincludestill globssrc/**/*.d.tsfor the ambient declarations tests genuinely rely on (the*.cssmodule declaration, theNodeJS.ProcessEnvnarrowing), and only this one file is held out.src/__tests__/browser-process-shim-scope.test.ts— new pin.The check that goes red without the fix
packages/components/tsconfig.jsonexcludes tests, sotsc --noEmitsays nothing about a new test file. Verified explicitly against the test project with--listFiles:src/__tests__/browser-process-shim-scope.test.tssrc/browser-process-shim.d.tssrc/global.d.ts(control)The pin's compile-time half is the file compiling at all — a runtime assertion cannot see this defect, because all three spellings always worked at runtime. Reverse-verified against the committed fix, same test file both legs:
GREEN —
tsc -p tsconfig.test.json→ exit 0, no output.RED — declaration put back in
global.d.ts,excludedropped, shim file removed → exit 2:The exact diagnostic the card measured. During that same red leg the source project stayed green (exit 0) — the red is specific to the shim leaking into the test project, not collateral damage. Mutation confirmed on disk each leg; tree restored byte-identically afterwards (
git diff HEADempty).The four runtime assertions guard the two ways the arrangement can be dismantled while still compiling: deleting the shim outright (which breaks the source build — a different tsconfig project than the one that reads the test) and re-adding a
processdeclaration to the sharedglobal.d.ts.Verification
All at
88439fc5, the final commit.pnpm --filter @object-ui/components^... build→ dependency closure built first,VERDICT command-exit 0.pnpm run type-checkinpackages/components(tsc --noEmit && tsc -p tsconfig.test.json, the hyphenated script CI's Type Check job runs) →VERDICT command-exit 0.pnpm exec vitest run packages/components/src/__tests__/browser-process-shim-scope.test.ts --reporter=verbose, root-form — the file that actually executed, printed per test:|unit| packages/components/src/__tests__/browser-process-shim-scope.test.ts.Test Files 1 passed (1),Tests 5 passed (5).pnpm exec vitest run packages/components/, root-form →Test Files 210 passed (210),Tests 1965 passed (1965).pnpm --filter @object-ui/components build→ green, and neitherglobal.d.tsnor the new shim is emitted:grep -rn "declare const process\|declare var process" dist --include=*.d.tsfinds nothing, so the published surface is unchanged.check-type-check-coverage(41/41 packages compile their tests, 0 declared debt),check-lint-coverage(46/46),check-changeset-presence,check-changeset-no-major,check-changeset-overwrite,check-changeset-fixed,check-control-bytes(scanned 5650 tracked text file(s)),check-vi-mock-specifiers,check-package-self-import,check-published-dist-tooling(39 published package(s),0 tooling artifact(s) in build output),check-doc-fence-languages— all ✅.eslint packages/components --format json→ 427 files, 0 errors, 933 warnings, all pre-existing; the three files this PR adds or edits contribute 0 of each. Three pieces of evidence that this narrowing is a measurement and not a skipped run: (a) the population comes from eslint's own config resolution, not from a guess about which files count — every path it was handed came back with a result rather than an ignore notice; (b) the count of 427 is read from the--format jsonoutput, not estimated; (c) type-aware linting is not enabled —eslint.config.jsusestseslint.configs.recommended(notrecommendedTypeChecked) and itslanguageOptionscarries onlyecmaVersionandglobals, with noparserOptions.projectand noprojectService, so no rule reads type information and a.d.tsor tsconfig edit cannot move a verdict in an untouched file. The repo-wideeslint .run belongs to CI.No existing test moved. The diff is three added files and two modified ones — a
.d.tsand a tsconfig. No existing test file is touched.Changeset
.changeset/6809-browser-process-shim-source-only.md, empty frontmatter — the explicit "declares no release" form. Declaration files andtsconfig.test.jsonare checking-only inputs, and the builtdist/**/*.d.tscarries noprocessdeclaration at all, so nothing published changes. ⛔ Noskip-changesetlabel applied: in this repo that label object exists only as a historical mis-tag, no workflow or script reads it, and a pin test holds that line — the empty-frontmatter changeset is the declaration that actually works here.Out of fence
The
globalThisworkaround PR #6808 left at its call site is untouched here, as the dispatch required. That PR is still open and unmerged, so the call site does not exist in this branch's base — there was literally nothing to edit. This fix does make the cast unnecessary once #6808 lands, and that cleanup is filed separately as #6833 with aBlocked-by:line rather than done here. Deduped first against one page of the 100 newest open issues plus a local grep, with #6809 itself as the control hit; the global/search/issuesREST path is not reachable from this seat.One thing noticed and deliberately not changed: the
declare namespace NodeJS { interface ProcessEnv { NODE_ENV: … } }block left inglobal.d.tsis inert in the source project — the shim typesenvas a plain{ NODE_ENV: string }and never referencesNodeJS.ProcessEnv— and only does real work in the test project, where it augments@types/node. That is a separate question from this card's, and no behaviour of this PR depends on the answer.Generated by Claude Code