fix(scripts): read the ESM gate's specifiers from the parser, not a comment mask - #5481
Merged
Merged
Conversation
…mask `withoutCommentedCode()` blanked block comments before line comments, with a pattern that had no notion of already being inside a `//` line. Any slash-star sequence in ordinary line-comment prose opened a comment that ran to the next star-slash anywhere in the file, blanking the live code between them. Re-measured on main at 478ec54 over the 805 files of the 13 specifier-preserving packages: the mask saw 2132 relative specifiers, the TypeScript parser 2133. The missing one is a live import in packages/app-shell/src/preview/DraftChangesPanel.tsx, hidden by a line comment naming a package glob eight lines above it. Leg 1 now reads each file as written and takes its module edges from check-phantom-dependencies.mjs's shared TypeScript scanner, so comments, strings, template literals and regex literals stop being questions this gate has an opinion about. Line numbers stay the compiler's: the specifier literal is located inside the statement the AST already identified. Refs #5382 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RV6yuVCxymHYE16PL9vQkE
Pins both halves of objectui#5382. The measured shape — a line comment naming a package glob, then a real import, then a doc comment whose closing delimiter is where the fake block comment ended — is asserted all the way to the verdict, not just to the specifier list: under the retired mask that source yields zero specifiers, so the import was invisible to a leg that has been a hard requirement since SPECIFIER_DEBT emptied. The counter-probe is pinned alongside it, because a zero is only worth what the same method still finds: the prose-only fixture asserts both that the real specifiers are still reported and that the commented-out ones still are not. Also pinned: the string-literal half of the same class, JSX parsing, the specifier-vs-statement line number, and that ESM_MODULE_EDGE names exactly the four forms Node's ESM resolver has to resolve. Refs #5382 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RV6yuVCxymHYE16PL9vQkE
Empty frontmatter — CI tooling only, no package src/ is touched, so this publishes nothing and says so explicitly rather than being left undeclared. Refs #5382 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RV6yuVCxymHYE16PL9vQkE
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.
Fixes #5382
Leg 1 of the ESM-load gate blanked comments out of every source with two ordered regexes and then matched specifiers in the result. The block-comment pass ran first and had no notion of already being inside a
//line, so a slash-star sequence in ordinary line-comment prose opened a comment that ran to the next closing delimiter anywhere in the file, blanking every line between — live code included.Re-measured on
mainat 478ec54Not the card's numbers; the repository moved since it was written (805 files now, 424 in app-shell then vs 426 now).
RELATIVE_SPECIFIERSet difference, one entry, one direction — the parser sees it and the gate did not:
Hidden by the line comment eight lines above it, which names the
@objectstackchunk group by glob. Restricted to@object-ui/app-shellalone the same measurement is 1275 vs 1276, which is the card's 1270 vs 1271 plus the five specifiers the package has grown since.The direction is the bad one. Since
SPECIFIER_DEBTemptied, leg 1 is a hard requirement rather than a ratchet, so a blind spot in it is somewhere a regression can sit permanently behind a green run.The fix is not a cleverer pattern
A pattern cannot answer "am I inside a comment", and the two
replacecalls whose ORDER decided the answer were the defect. Leg 1 now reads each file exactly as written and takes its module edges fromcheck-phantom-dependencies.mjs's TypeScript-backedmoduleSpecifiers()— the same scannercheck-package-self-import.mjsalready shares, so three gates cannot drift apart on what a module edge is. Comments, strings, template literals and regex literals stop being questions this gate has an opinion about.RELATIVE_SPECIFIERandwithoutCommentedCode()are deleted rather than left unused.Two details that are load-bearing rather than tidy:
moduleSpecifiers()reports where the STATEMENT starts;tscreports this class at the SPECIFIER, and the two differ for 255 of 2066 relative specifiers here (12.3%), by up to 7 lines. Measured directly:src/index.ts(8,8): error TS2835for an import whose statement opens on line 6. So the specifier literal is LOCATED inside the statement the parser already identified — nothing here decides what is code. Verified by construction: over all 2132 specifiers the old method could see, the new one reports the identical specifier-and-line pair for every single entry.moduleSpecifiers()also readsrequire()andimport x = require(), which Node's ESM resolver never sees.ESM_MODULE_EDGEnames the four kept forms — exactly what the retired regex matched, so the leg's scope is unchanged by the swap. Measured before excluding them: zero relative edges of either CommonJS kind exist in any specifier-preserving package.Before / after, and the blast radius
origin/main)The gate does not go red. The newly visible import already carries its
.jsextension — it was cleared by #5357 while still invisible to the gate that was supposed to be watching it. Nothing was exempted, nothing was ledgered,SPECIFIER_DEBTis still empty. The whole blast radius is +1 visible specifier and +1.7s.Counter-probe — the zero is only worth what the same method still finds
"No hidden specifiers" is meaningless from a scanner that finds nothing, so both buckets are reported. Over the same 805 files:
All six were read and confirmed prose — JSDoc usage examples, a retired commented-out
export, and a doc comment quoting a public export. Zero false positives in either direction, and pinned as a test pair rather than only measured.Reverse-verification
relativeSpecifiers's body was replaced with the retired mask-and-regex mechanism, keeping the export and signature so the signal is about the mechanism rather than a missing import.Predicted before running: 3 red / 35 green — the two glob fixtures and the finding-level assertion; the counter-probe, line-number, JSX and form-scope pins unaffected because the retired regex handles those cases identically.
Observed:
Tests 3 failed | 35 passed (38), on exactly those three.No build sits between the edit and the run: the test imports
'../check-node-esm-load.mjs'by relative path, and no vitest alias redirects it — there is nodist/for a mutation to fail to reach. Both legs were proved to reach the code under test (the ablated build returned[]for the fixture, the restored one returns the import again). Restored withgit checkout; tree clean, ablation marker count 0, suite back to 38 passed.On #5367 — NOT subsumed, deliberately untouched
readTsconfig()in this same script still strips comments with.replace(/\/\*[\s\S]*?\*\//g, '')plus a line-comment pass, and still destroys any tsconfig whose JSON strings contain a slash-star. That is the same defect CLASS and a separate live instance, in a different function, on a different input. Nothing in this PR touches it — it was out of the dispatched file surface, and it is left open.What this PR does change for whoever picks it up:
typescriptis now reachable from this script through the shared scanner, so #5367's own suggested shape (ts.parseConfigFileTextToJson) no longer costs a new dependency edge, and after this landsreadTsconfig()is the LAST context-unaware comment stripper in the file.Verification at 906bccb
Changeset: empty frontmatter — CI tooling only, no package
src/touched, so this publishes nothing and says so explicitly.Generated by Claude Code