You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Found while implementing #6666 (property-chain leg for check-i18n-dead-keys). Filed unassigned; out of scope for that PR, which touches a different mechanism.
The claim, and why it is false today
Two test files carry a constant interpolated into their fixture sources, each justified by a comment about a sibling gate's matching strategy:
Both say, in substance, that scripts-type-check.test.tsgrepsscripts/ for import statements and that its regex cannot tell a string literal from a real import.
That was true when written. It is not true now. #4902 replaced the regex with an AST walk — workspaceImportSpecifiers() in scripts/__tests__/scripts-type-check.test.ts:98 builds a ts.createSourceFile and visits ImportDeclaration / ExportDeclaration / ImportEqualsDeclaration nodes. Its own docstring states the correction outright:
read from the AST rather than grepped from the file's text (objectui#4902). A textual from\s+'...' match cannot tell a real import edge apart from the same shape sitting inside a line comment, a block comment, or a JSDoc @example — this repo has hit that exact false positive
And scripts-type-check.test.ts:354-392 pins the new behaviour directly, including the case the stale comments describe as impossible to distinguish:
workspaceImportSpecifiers('probe.ts',"const doc = \"see '@object-ui/core' for details\";\n")
Why it is worth a card rather than a silent cleanup
The comments are not decoration — each one is the stated reason a fixture is built the awkward way it is. A reader who trusts them concludes that any specifier written plainly in a scripts/ test will be misread as an import edge, and keeps paying an interpolation tax that nothing charges any more. A reader who checks them discovers the sibling gate's docstring says the opposite, and now has to work out which of the two is current. That is the maintenance cost documentation drift actually imposes, and it is why #4902's own dispatch treated the false-positive class as worth a gate rather than a convention.
Two things to decide, not to assume
Whether the interpolation constants (I18N_PKG and its twin) should be removed along with the comments, or kept with a corrected justification. There may be a second, still-live text-level reader of scripts/ that the comments conflate with scripts-type-check.test.ts — check-phantom-dependencies.mjs excludes scripts/ (its header, near line 185), but that is one gate checked, not the set.
Found while implementing #6666 (property-chain leg for
check-i18n-dead-keys). Filed unassigned; out of scope for that PR, which touches a different mechanism.The claim, and why it is false today
Two test files carry a constant interpolated into their fixture sources, each justified by a comment about a sibling gate's matching strategy:
scripts/__tests__/check-i18n-dead-keys.test.ts:31-39scripts/__tests__/check-i18n-call-site-keys.test.ts:94-99Both say, in substance, that
scripts-type-check.test.tsgrepsscripts/for import statements and that its regex cannot tell a string literal from a real import.That was true when written. It is not true now. #4902 replaced the regex with an AST walk —
workspaceImportSpecifiers()inscripts/__tests__/scripts-type-check.test.ts:98builds ats.createSourceFileand visitsImportDeclaration/ExportDeclaration/ImportEqualsDeclarationnodes. Its own docstring states the correction outright:And
scripts-type-check.test.ts:354-392pins the new behaviour directly, including the case the stale comments describe as impossible to distinguish:Why it is worth a card rather than a silent cleanup
The comments are not decoration — each one is the stated reason a fixture is built the awkward way it is. A reader who trusts them concludes that any specifier written plainly in a
scripts/test will be misread as an import edge, and keeps paying an interpolation tax that nothing charges any more. A reader who checks them discovers the sibling gate's docstring says the opposite, and now has to work out which of the two is current. That is the maintenance cost documentation drift actually imposes, and it is why #4902's own dispatch treated the false-positive class as worth a gate rather than a convention.Two things to decide, not to assume
I18N_PKGand its twin) should be removed along with the comments, or kept with a corrected justification. There may be a second, still-live text-level reader ofscripts/that the comments conflate withscripts-type-check.test.ts—check-phantom-dependencies.mjsexcludesscripts/(its header, near line 185), but that is one gate checked, not the set.git grepfinds for this wording; a broader sweep for prose describing that gate has not been done.Reproduce
Measured on
origin/mainat4357ec754.Generated by Claude Code