strip-internal: widen the fast gate, and call the sibling script through pnpm - #1640
Merged
Conversation
…ugh pnpm The pre-filter that decides whether a `.d.ts` is worth parsing only matched an underscore member written bare or behind `readonly`. A file whose underscore members all carry a modifier — `private _secret`, `protected _hidden()`, `static _shared` — failed the gate and was skipped outright, so those members survived into the published types. Silent, because the file was never parsed rather than parsed and passed over. Nothing leaks today: no emitted declaration currently uses those modifiers on an underscore member, so the hole is latent rather than active. It would open the moment someone wrote one in TypeScript source. Verified both ways against a probe declaration carrying only modified members and no `@internal` — the old pattern skipped the file, the new one parses it. `doc` also shelled out to `npm run doc:css` while every other script here uses pnpm, which can resolve differently in a workspace install. Raised by Copilot on #1639. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Aa37KGXZcnVrbn1yG4j1N
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The changes are small, targeted, and align with the stated goal of preventing silent .d.ts skipping while improving workspace script consistency.
Pull request overview
This PR tightens the type-declaration post-processing workflow in packages/melonjs by widening the “fast gate” heuristic used by scripts/strip-internal.ts so modified underscore-prefixed class members don’t cause entire .d.ts files to be skipped, and by making the docs pipeline consistently invoke sibling scripts via pnpm within the workspace.
Changes:
- Widen
UNDERSCORE_HINTto match underscore-prefixed members even when preceded by common class-member modifiers (e.g.private,protected,static,abstract,override,get,set). - Switch the
docscript to calldoc:cssviapnpminstead ofnpmfor workspace-consistent resolution.
File summaries
| File | Description |
|---|---|
| packages/melonjs/scripts/strip-internal.ts | Expands the prefilter regex so files containing only modified underscore members are still parsed and stripped correctly. |
| packages/melonjs/package.json | Makes doc invoke doc:css via pnpm to match the rest of the workspace scripts. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
Two follow-ups from Copilot's review of #1639.
The fast gate could skip a whole file
UNDERSCORE_HINTdecides whether a.d.tsis worth parsing at all. It only matched an underscore member written bare or behindreadonly:/^\s*(readonly\s+)?_[A-Za-z0-9_]+\s*[?:(<]/mA file whose underscore members all carry a modifier —
private _secret,protected _hidden(),static _shared— fails that test and is skipped outright, so those members survive into the published types. The failure mode is silent: the file is never parsed, rather than parsed and passed over.Nothing leaks today. No emitted declaration currently puts those modifiers on an underscore member, so the hole is latent. It opens the moment someone writes one in TypeScript source — which is exactly the kind of thing that gets noticed months later.
Verified in both directions against a probe declaration carrying only modified members and no
@internal:Erring wide costs one AST walk on a file with nothing to strip.
docshelled out to npmEvery other script in the package uses pnpm (
pnpm lint,pnpm types,pnpm clean);doccallednpm run doc:css. Inconsistent, and npm can resolve differently in a workspace install.Verified