[code-infra] Add ts-package-migration skill#48545
Draft
Janpot wants to merge 6 commits into
Draft
Conversation
Deploy previewhttps://deploy-preview-48545--material-ui.netlify.app/ Bundle size
Check out the code infra dashboard for more information about this PR. |
Janpot
force-pushed
the
code-infra/ts-migration-skill
branch
5 times, most recently
from
32dcd0a to
ee59374
Compare
Janpot
force-pushed
the
code-infra/ts-migration-skill
branch
from
ee59374 to
054b411
Compare
The reassignment inside if (NODE_ENV !== 'production') is already dead-code-eliminated in production, so the babel forceRemoval marker is redundant — matches mui-material's convention. Lesson from PR mui#48565.
…warning Two findings surfaced on @mui/system PR mui#48578 worth encoding: 1. Per-item triage on packages with many undeclared runtime exports — shape-of-the-export tells you which way the judgement goes (style- function siblings → promote; cross-submodule helpers → @internal), saving per-item deliberation on packages with 20+ such leaks. 2. stripInternal:true strips every @internal in the package, including pre-existing ones. If a pre-existing @internal is reachable through the public type surface (as Grid.unstable_level was via GridOwnerState), enabling the flag breaks downstream consumers' declaration builds — not the converted package's own build, so it slips past local steps that skip Verification 6. Audit + drop the lying tag before enabling.
Two corrections from @mui/system PR mui#48578: - The (X as any).propTypes = {} pattern this skill recommended bricks typescript-to-proptypes (Expected type "Identifier", got "TSAsExpression"). It worked for styled-engine because that package has no propTypes-bearing components in the generator's input list — but failed immediately on @mui/system's Box.tsx and ThemeProvider.tsx. Default to the mui-material convention used by Portal.tsx / FocusTrap.tsx: `X.propTypes = { ... } as any;` for the main assignment, `(X as any)['propTypes' + ''] = exactProp((X as any).propTypes);` for the dev reassignment. This admits the tsc expando but keeps the Babel guard AND passes the generator. - Wildcard package.json `exports` (`./*: ./src/*/index.ts`) only resolve `.ts` files; any dir still on `.js` (partial conversion or mid-PR revert) needs an explicit entry, otherwise rolldown bundle-size fails to resolve the package subpath.
Both cast-based patterns documented before this commit had failure modes:
- `X.propTypes = {…} as any` keeps the proptypes script happy but does not
prevent tsc from synthesizing `declare namespace X { var propTypes: any }`
in the emitted .d.ts (the `as any` is on the value, not on the property —
tsc still augments). The skill previously claimed this expando was
"benign and matches mui-material"; on @mui/system part-1 it was the trigger
for the cleanup commit that broke `pnpm proptypes`.
- `(X as any).propTypes = {…}` does prevent the namespace but trips
typescript-to-proptypes with `Expected type "Identifier", got
"TSAsExpression"` because the script asserts the LHS object is an
Identifier. PR mui#48578 hit this after the first fix.
The fix is the binding shape, not a cast. Either the receiver of `.propTypes`
already has propTypes in its declared type (forwardRef/memo wrapping, via
@types/react — Portal.tsx, mui-x/GridRow.tsx), or it's a const-bound function
expression typed by an interface that lists `propTypes?: any` (the only
correct shape for non-wrapped components like ThemeProvider/GlobalStyles/
DefaultPropsProvider whose original .js was a `function X(){}` declaration).
With either binding shape, the assignment site needs no casts at all — and
the namespace synthesis disappears because tsc no longer has an
unaccounted-for property assignment to capture.
Three concrete shapes documented:
- wrapped (Shape 1) — `const X = forwardRef(function X(...){...})`,
`X.propTypes = {…}` — clean .d.ts emits as
`declare const X: ForwardRefExoticComponent<…>`.
- generic / bare-function (Shape 2) — `const X: XType = function X<T>(...){...}`
with `interface XType { …; propTypes?: any }` — clean .d.ts emits as
`declare const X: XType`.
- generic + wrapped (Shape 3, mui-x DataGrid) — non-exported inner
`const XRaw = function X<T>(){...}`, exported wrapper cast to a
propTypes-bearing interface — clean .d.ts emits the interface and the const.
Critically: a same-name `interface X { propTypes?: any }` declaration-merged
with `function X(){}` is NOT sufficient. Verified empirically — tsc still
synthesizes the namespace even with the interface present. The
const-binding (Shape 2) is load-bearing; the interface alone is not.
Reference: mui-x/packages/x-data-grid/build/DataGrid/DataGrid.d.ts and
.../GridRow.d.ts ship both Shape 1 and Shape 3 today, both with clean
declarations.
This reverts commit 07cf748.
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
1 participant
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.
Adds a reusable Claude Code skill (
.claude/skills/ts-package-migration/) that encodes the playbook for migrating a MUI package from hand-written.js+.d.tsto true TypeScript (single.ts/.tsxsource,tsc-emitted declarations), like@mui/utils.Captures the procedure and the non-obvious findings discovered during the
@mui/styled-engineconversion (#48544): thecode-infra builddeclaration pipeline andtsconfig.build.jsonrequirement, Babel's import-elision behavior (import type * as/export type *), the(X as any).propTypes /* remove-proptypes */trick to suppress the tsc expando while keeping the production guard,stripInternalfor runtime-only exports, themui-env.d.tsconvention forprocess.env(nonodein build types), the'use client'+export *ESLint constraint, the quality-gate commands, and a bidirectional type-equivalence probe to prove the exported type surface is preserved.Tooling/docs only — no runtime or package changes.