fix(scripts): teach check-side-effects-array to see a call in a top-level initializer - #8851
Merged
baozhoutao merged 2 commits intoSep 9, 2026
Merged
Conversation
…evel initializer `classifyEffect` sorted a module's top-level effects by looking for a call STATEMENT, so `export const X = f(...)` — a binding declared AND an effect performed in one statement a bundler drops whole — was filed as "declares a constant, does nothing". `@object-ui/types`' `AnyComponentSchema = defineNodeComponentUnion(...)` is the live shape: the initializer writes the node recursion point's option slot into `base.zod.ts`, and the gate that exists to enumerate exactly those effects could not see it. The widening is bounded by measurement, not by taste. Reading ANY call in an initializer as a registration takes `@object-ui/app-shell` from 14 registering modules to 122 — `new Set([...])`, `React.createContext(...)`, `new RegExp(...)` produce the binding's value and nothing else, and naming their modules spends exactly the consumer bytes the array exists to save. So an initializer registers only when the call is evaluated NOW (the walk stops at function boundaries, so a returned closure is not load-time), its callee resolves inside this package through relative imports, and that callee writes to a binding it did not itself declare — the `local-binding-write` argument applied to the callee's scope. Measured over 1,623 source files in `packages/` and `apps/`: one declaration is flagged, the true positive above. No package's verdict changes. The header now states the recognition rule in both directions, including the two things a zero from this gate still does not mean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FhBNJcLRZLe8M87VcUgpKr
The header's list of what a zero from this gate still does not mean said the secondary-entry blindness was "tracked separately" without saying where. It is objectui#8850. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FhBNJcLRZLe8M87VcUgpKr
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
|
baozhoutao
marked this pull request as ready for review
September 9, 2026 14:28
baozhoutao
deleted the
claude/issue-8578-side-effects-const-initializer
branch
September 9, 2026 14:56
Contributor
⭐ LANDED — merged
|
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 #8578
classifyEffectsorted a module's top-level effects by looking for a callSTATEMENT, so
export const X = f(...)— one statement that declares a bindingAND performs an effect, and that a bundler drops whole — was filed as "declares
a constant, does nothing".
@object-ui/types'AnyComponentSchema = defineNodeComponentUnion(...)is the live shape: theinitializer writes the node recursion point's option slot into
base.zod.ts,and the gate that exists to enumerate exactly those effects could not see it.
The recognition rule, and why it is not "any call in an initializer"
The opposite failure is the expensive one, and it was measured rather than
argued. Reading ANY call in a top-level initializer as a registration takes
@object-ui/app-shellfrom 14 registering modules to 122 —new Set([...]),React.createContext(...),new RegExp(...)andReact.lazy(...)are callsthat produce the binding's VALUE and nothing else, and naming their modules
spends exactly the consumer bytes the array exists to save.
So an initializer registers only when all three hold:
a closure that is RETURNED is not a load-time effect.
withSettleSignalinapp-shellincrements a module counter inside the wrapper it returns, and itis the ONE site a coarser rule flagged workspace-wide. A function handed as
an ARGUMENT to a call being made now IS walked, because that call may invoke
it now.
re-exports only.
local-binding-writeargument applied one level in, to the callee's ownscope: anything it did not declare outlives the call and is observable by
someone other than the dropped module.
The header now states this in both directions, including the two things a zero
from this gate still does not mean (a call into another package is read as
value-producing; only the source barrel's graph is walked).
False-positive tail — measured before shipping
Scanned every
.ts/.tsx/.mts/.js/.jsx/.mjssource underpackages/andapps/(tests,
dist/andnode_modules/excluded) with the widened classifier:One flagged declaration, and it is the true positive the card names — out of 88
const-initializer call sites in that one file. No package changes verdict.
node scripts/check-side-effects-array.mjs --listis byte-identical before andafter, and the gate stays green:
@object-ui/typesscores zero not only because of the classifier, butbecause
evaluatePackagewalks only thesrc/index.*graph, andsrc/zod/index.zod.tsis not reachable frompackages/types/src/index.ts.Even with this fix the package-level count is unchanged; pointing
scanModulestraight at the file does report the registration. Filed ascheck-side-effects-array walks only the SOURCE BARREL's graph, so a registrar reachable only from a secondary entry form is outside the enumeration #8850, ⛔ not fixed here.
@object-ui/typesdeclaressideEffects: falsetoday, so it is not inthis gate's population at all (
readArrayPackagestakes ARRAY declarationsonly). Whether it should be is The node-recursion fill is tree-shaken away for bundled consumers — closing it needs a maintainer-floor census edit #8577 / spec(types): redirect the node recursion point from BaseSchemaCore to AnyComponentSchema — measured at 9 newly-refused documents, and it drops 118 phantom strict refusals #8344, ⛔ untouched here — no
package's
sideEffectsarray is edited by this PR.⭐ On #8344. Its decision batch #93 refused the "narrow
sideEffectsto thezod barrel" route citing this card verbatim — "its detector cannot see a
const-initializer call — so A is not implementable as spelled withoutweakening a gate." That premise has moved: the detector can now see that
shape, and seeing it costs no gate. ⛔ This PR does not re-open #8344 and takes
no position on it; correction 1 above is the thing whoever re-prices that route
needs to read first, because the detector is no longer the only obstacle.
Tests
Both directions, per this suite's own rule that every assertion needs a partner
that makes the same fixture fail:
registration; callee onlycomputes the value → nothing.
withSettleSignalshape) → nothing;callback handed to a call being made now →
registration.z.object({}),new Set([1, 2]),React.createContext(null)→ nothing.register()→ nothing.import away must be NAMED in the array; identical fixture with a non-writing
callee must not.
(
scripts/__tests__/check-side-effects-array.test.ts,check-sdui-registration-pins.test.ts,side-effects-declaration-consistency.test.ts— the three test files in the treethat name the changed module.)
Ablation, from the committed state, with the mutation proven on disk
(HEAD blob
fa73bcb1→ mutated725e3857) and a byte-identical restore(
git diff HEADempty,git hash-objectback tofa73bcb1): commenting out theone new
classifyEffectbranch turns 3 tests red (the two positive-directionunit tests and the workspace-level one); the negative-direction partners stay
green, which is what they should do.
Other gates at
54491c3b0:type-check:scriptsexit 0 ·check-control-bytesOK (7,041 files) ·
check-changeset-presenceexit 0, "No source or publishedcontract of a released package changed in this range, so no changeset is owed"
· eslint over the two changed files 0 errors / 0 warnings (population 4,656
files; type-aware linting is not configured, so this diff cannot move the
verdict on any untouched file).
维护者速读(草稿)
改了什么 — 只改了一个内部门禁脚本
scripts/check-side-effects-array.mjs和它的测试。让它能看见"写在顶层
const初始化器里的加载期副作用",这类副作用以前被静默判成"什么也没做"。
为什么改 —
sideEffects数组是发布给所有下游打包器的承诺;数组漏掉一个模块,下游会静默丢掉一次注册,没有报错、没有警告、退出码 0。这个门禁就是为了
挡住这种静默失败,而它自己有一个盲区。
风险与代价(含回滚) — 放宽判定的反向风险是把纯声明误判成注册,那会让数组
被迫写满整个包、把这个数组省下来的字节全花掉。所以先做了全量测量:全仓 1,623
个源文件只命中 1 条,就是要找的那一条,两个数组包的结论一字未变。不改任何包的
sideEffects,不动发布产物,无需 changeset。回滚 = revert 这两个 commit,门禁回到旧行为。
席位意见 — (留空)
你要做的 — 确认两点是否同意:① 对"调用进另一个包"(
z.object、React.createContext)一律读作产值不读作注册,这条边界写进了文件头;② #8344 那条被本卡挡住的路线,前提已经变了,但本 PR 不去动它,重新定价留给
持有那张卡的人 —— 另外还有 #8850 是更关键的一半。
🤖 Generated with Claude Code
https://claude.ai/code/session_01FhBNJcLRZLe8M87VcUgpKr
Generated by Claude Code