Skip to content

Commit d2c6925

Browse files
committed
[skill] Fix propTypes pattern; add wildcard-exports finding
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.
1 parent 3881ca3 commit d2c6925

1 file changed

Lines changed: 57 additions & 17 deletions

File tree

  • .claude/skills/ts-package-migration

.claude/skills/ts-package-migration/SKILL.md

Lines changed: 57 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -166,24 +166,64 @@ Re-run the type-equivalence probe after any lint/format fix that touches source.
166166
- **propTypes on a component creates a tsc expando** → emits
167167
`declare namespace X { var propTypes: any }` and splits
168168
`export default function X` into `declare function X; export default X;`.
169-
Suppress it **and** keep the Babel production guard with:
170-
`(X as any).propTypes /* remove-proptypes */ = { ... };`
171-
The `/* remove-proptypes */` trailing comment on the assignment LHS triggers
169+
Two patterns exist; pick based on whether the file is processed by the
170+
repo's `pnpm proptypes` (`typescript-to-proptypes`) generator — which is
171+
**everything in `@mui/system`, `@mui/material`, `@mui/lab`, `@mui/joy` with
172+
a React component**.
173+
174+
**Pattern A — `mui-material` convention** (used by `Portal.tsx`,
175+
`FocusTrap.tsx`; mandatory for any file in `typescript-to-proptypes`'s
176+
input list, since the script asserts the propTypes-assignment LHS object
177+
is an `Identifier` and rejects `TSAsExpression`):
178+
```ts
179+
X.propTypes /* remove-proptypes */ = {
180+
/* ... */
181+
} as any;
182+
if (process.env.NODE_ENV !== 'production') {
183+
// eslint-disable-next-line
184+
(X as any)['propTypes' + ''] = exactProp((X as any).propTypes);
185+
}
186+
```
187+
The `/* remove-proptypes */` trailing comment on the static LHS triggers
172188
`babel-plugin-transform-react-remove-prop-types` `forceRemoval` (keeps the
173-
`process.env.NODE_ENV !== "production" ? … : void 0` wrap), while the
174-
`(X as any)` LHS stops `tsc` from creating the expando — so the `.d.ts`
175-
default export stays `export default function X`. (Plain
176-
`X.propTypes = {} as any` keeps the guard but adds the expando; `(X as
177-
any).propTypes = {}` without the comment drops the guard — JS regression.)
178-
**The companion dev-only `exactProp` reassignment inside
179-
`if (process.env.NODE_ENV !== 'production') { … }` must NOT carry the
180-
`/* remove-proptypes */` marker** — the env-guard already dead-code-
181-
eliminates it in production builds, so the marker is redundant. Use:
182-
`(X as any).propTypes = exactProp((X as any).propTypes);` — the `(X as any)`
183-
cast is just to satisfy TS for the read/write; no LHS comment, no expando
184-
(no static `.propTypes = {}` assignment on `X`'s declaration). Matches
185-
`mui-material`'s convention (e.g. `Portal.tsx`, `FocusTrap.tsx`). Surfaced
186-
on `@mui/private-theming` PR #48565 review.
189+
`process.env.NODE_ENV !== "production" ? … : void 0` wrap). The `as any`
190+
on the RHS satisfies tsc — at the cost of allowing the expando
191+
`declare namespace X { var propTypes: any }` in the emitted `.d.ts`, which
192+
is benign and matches what `mui-material` ships. The dev-only `exactProp`
193+
reassignment uses a computed-key `['propTypes' + '']` LHS so
194+
`typescript-to-proptypes` doesn't try to inject into it; the marker
195+
comment is omitted because the `NODE_ENV` env-guard already dead-code-
196+
eliminates it in production.
197+
198+
**Pattern B — `styled-engine` convention** (no expando in the emitted
199+
`.d.ts`, but **breaks `typescript-to-proptypes`** with `Expected type
200+
"Identifier", got "TSAsExpression"`):
201+
`(X as any).propTypes /* remove-proptypes */ = { ... };` with
202+
`(X as any).propTypes = exactProp((X as any).propTypes);` for the dev
203+
reassignment. Use **only** when the file is not in `pnpm proptypes`'s
204+
input list — e.g. a styling-only package with no React-component
205+
propTypes (where Pattern A would emit a needless expando but the script
206+
never runs anyway). The original skill commit (and `@mui/private-theming`
207+
PR #48565 review) recommended Pattern B globally; that bricked
208+
`test_static` (Generate PropTypes) on `@mui/system` PR #48578 for
209+
`Box.tsx` and `ThemeProvider.tsx`. Default to Pattern A.
210+
211+
(Plain `X.propTypes = {}` without `as any` — no cast at all — keeps the
212+
guard but emits a propTypes type-mismatch error at the tsc step. `(X as
213+
any).propTypes = {}` without the `/* remove-proptypes */` comment drops
214+
the Babel guard — JS regression.)
215+
- **Wildcard `package.json` `exports` resolve only the wildcard's literal
216+
extension.** A catch-all like `"./*": "./src/*/index.ts"` only resolves
217+
directories whose `index` file is `.ts`. If the conversion leaves any
218+
directory on `index.js` — e.g. a partial conversion or a revert of one
219+
dir mid-PR, as with `RtlProvider/` on `@mui/system` PR #48578 — that dir
220+
needs an explicit entry:
221+
`"./RtlProvider": "./src/RtlProvider/index.js"`. Otherwise rolldown (and
222+
any strict ESM resolver) fails at bundle time with
223+
`"./X" is not exported under the conditions [...]`. The package's own
224+
build won't catch this — only downstream bundling does (CI surfaces it
225+
as `test_bundle_size_monitor` failure). The set of explicit entries to
226+
keep around therefore mirrors the set of dirs still on `.js`.
187227
- **`stripInternal: true` + `/** @internal *​/`** on a declaration removes
188228
runtime-only exports (e.g. `TEST_INTERNALS_DO_NOT_USE`) from emitted `.d.ts`.
189229
**An export that exists at runtime but is missing from the hand-written

0 commit comments

Comments
 (0)