[code-infra] Convert leaf @mui/system .js+.d.ts pairs to TypeScript (part 1)#48578
Open
Janpot wants to merge 12 commits into
Open
[code-infra] Convert leaf @mui/system .js+.d.ts pairs to TypeScript (part 1)#48578Janpot wants to merge 12 commits into
Janpot wants to merge 12 commits into
Conversation
…part 1) First slice of the @mui/system conversion: 26 of 41 hand-written .js + .d.ts pairs collapsed into single .ts/.tsx source, the same setup as @mui/utils and @mui/styled-engine (mui#48544) / @mui/private-theming (mui#48565). Follow-up PR will cover the heavyweight files (createStyled, createTheme, styleFunctionSx core, cssVars/createCssVarsProvider, colorManipulator, top-level index.{js,d.ts}). Scope of this PR: - Leaf style helpers: borders, breakpoints, cssGrid, display, flexbox, palette, positions, shadows, sizing, spacing, typography - Theming wrappers: ThemeProvider/, RtlProvider/, Container/index, useTheme, useThemeProps/, useThemeWithoutDefault - Misc utilities: compose, memoize, propsToClassKey, getThemeValue - Box (Box.tsx + index.ts), createBox.tsx, styled.ts, createTheme/shape, createBreakpoints/, styleFunctionSx/{defaultSxConfig, extendSxProp} @mui/system still uses the same build pipeline as before (`code-infra build --flat`, no `--skipTsc` since the package was already partially TS). Quality gates: - `pnpm -F @mui/system build` succeeds (tsc emits declarations) - `pnpm -F @mui/system typescript` passes - `pnpm -F @mui/material build` (downstream consumer) succeeds Pragmatic conversion choices for this batch: - Style helpers use `as unknown as SimpleStyleFunction<…>` casts to preserve the hand .d.ts's narrower type for each export - `(X as any).propTypes /* remove-proptypes */ = { ... }` for the propTypes assignments that need a tsc expando suppression - Type-only namespace imports (`import type * as React`) to keep the emitted JS unchanged - A few exports that the hand .d.ts had broader/narrower than the .js runtime are normalized to match runtime (e.g. `createTheme`'s barrel still re-exports the same surface)
- Revert RtlProvider/ (used by @mui/material-pigment-css). Defer to follow-up. - Add `| undefined` on optional props (breakpoints, createBox, createBreakpoints). - Rename `_margin`/`_padding`/`_spacing` to `marginFn`/`paddingFn`/`spacingFn`. - Drop `Shape` annotation on the runtime `shape` const so consumer-side module augmentation (sm/md/lg) is tolerated, restoring the original .js + .d.ts semantics via `as unknown as Shape`. - Reformat with Prettier.
These were exported by the original .js but not declared in the original hand-written .d.ts. The TS conversion now declares them and ships them in the emitted .d.ts as side effect. Annotate them with /** @internal */ to document their internal-only intent (IDE tooling and API extractors will respect it): - borders/borders.ts: borderTransform - breakpoints/breakpoints.ts: values, createEmptyBreakpointObject, removeUnusedBreakpoints, computeBreakpointsBase - getThemeValue/getThemeValue.ts: styleFunctionMapping, propToStyleFunction - palette/palette.ts: paletteTransform - sizing/sizing.ts: sizingTransform - spacing/spacing.ts: marginKeys, paddingKeys stripInternal:true is NOT enabled in tsconfig.build.json: it cascades into Grid.unstable_level (pre-existing @internal) and breaks mui-material's GridOwnerState. Fixing Grid is out of scope here.
…rnal on Grid.unstable_level stripInternal:true strips the 11 helpers marked @internal in the previous commit from the published .d.ts surface, restoring the original @mui/system public type surface. The flag also strips Grid.unstable_level (pre-existing @internal), which mui-material's GridOwnerState references — but the @internal marker on unstable_level was incorrect: it IS visible to consumers via GridOwnerState already, so it's not actually internal. Removing the JSDoc tag is the right call.
…eakpointKeys cast
Three CI failures from the previous round:
- test_bundle_size_monitor: rolldown couldn't resolve @mui/system/RtlProvider.
The part-1 conversion removed the explicit exports map entry assuming the
./*: ./src/*/index.ts wildcard would cover it, but the revert restored
RtlProvider/index.js (not .ts). Re-add the explicit entry.
- test_static (Generate PropTypes): typescript-to-proptypes asserts an
Identifier object in the propTypes MemberExpression LHS — (X as any).propTypes
is a TSAsExpression, which trips it. Switch ThemeProvider and Box to the
mui-material/Portal.tsx pattern: X.propTypes = { ... } as any; for the main
assignment, and (X as any)['propTypes' + ''] = exactProp(...) for the
dev-only reassignment. Re-running pnpm proptypes is now clean.
- test_types (module augmentation gridCustomBreakpoints): a module aug that
overrides BreakpointOverrides narrows Breakpoint so the literal
'xs'|'sm'|'md'|'lg'|'xl' no longer fits. Cast the array via
`as unknown as Breakpoint[]` to bypass the element-wise check.
Deploy previewhttps://deploy-preview-48578--material-ui.netlify.app/ Bundle size
Check out the code infra dashboard for more information about this PR. |
Janpot
added a commit
to Janpot/material-ui
that referenced
this pull request
May 27, 2026
…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.
Janpot
added a commit
to Janpot/material-ui
that referenced
this pull request
May 27, 2026
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.
The api-docs-builder's react-docgen fallback resolver (created specifically for Box-like createX(...) factories) only unwraps ONE TSAsExpression layer; Box.tsx had `createBox(...) as unknown as OverridableComponent<BoxTypeMap>`, two layers, which made the resolver miss the CallExpression and throw "No suitable component definition found". Pattern B (`(Box as any).propTypes`) accidentally masked this because react-docgen didn't recognize the TSAsExpression LHS as a propTypes assignment, never reached the resolver. Pattern A made it visible. Fix is the right kind of fix: drop the redundant `as unknown` — createBox's inferred return type satisfies `OverridableComponent<BoxTypeMap>` directly, so a single `as` is enough. Also commit the api-docs-builder's regenerated output (box.json, grid.json, grid translations) and Grid.tsx's auto-synced @internal-comment removal from previous round.
ThemeProvider/DefaultPropsProvider/GlobalStyles were emitting a
`declare namespace X { var propTypes: any; }` block in their .d.ts /
.d.mts because the source had `X.propTypes /* remove-proptypes */ = {…}
as any;` — the `as any` was on the RHS value, so tsc still saw a
property assignment on the exported function and synthesized a
namespace. Move the cast to the LHS receiver (`(X as any).propTypes =
{…}`) to suppress namespace inference and keep the published surface
clean (matching the pattern already used in @mui/private-theming).
In getThemeValue.ts, drop the `Array.from((spacing as any).filterProps)`
materialization. spacing.filterProps is a Set; the local
filterPropsMapping is only consumed via iteration. Widen the mapping
type to `Record<string, Iterable<string>>` (Sets and arrays both
satisfy it) and switch the inner reducer loop from `.forEach()` to
`for…of`. Preserves the prior runtime identity (no new allocation) and
removes the unnecessary spacing-only special case.
…sProvider/GlobalStyles
The previous round suppressed tsc's `declare namespace X { var propTypes: any }`
synthesis by casting the LHS — `(X as any).propTypes = {…}` — but that LHS is
a `TSAsExpression`, which trips `typescript-to-proptypes`
(`Expected type "Identifier", got "TSAsExpression"`). The earlier round had
the inverse: `X.propTypes = {…} as any` kept the script happy but the `as any`
on the value doesn't stop tsc from capturing the property assignment as a
namespace expando on the function declaration.
Fix the binding shape instead. Convert each `function X(){}` to a typed
const-bound function expression with an interface that declares `propTypes`:
interface XType {
<T>(props: XProps<T>): React.ReactElement<XProps<T>>;
propTypes?: any;
}
const X: XType = function X<T>(props: XProps<T>) { … };
X.propTypes /* remove-proptypes */ = { … };
Once `propTypes` is part of the declared type, the assignment needs no cast
and tsc emits `declare const X: XType` (no namespace). The
proptypes-script's `assertIdentifier` on the LHS object passes (plain
Identifier). The dev-only `exactProp` reassignment can revert to plain
`X.propTypes = exactProp(X.propTypes)` — no computed-key
`['propTypes' + '']` workaround needed.
JS shape: `function X(){}` becomes `const X = function X(){}` — same
identity, name, length, semantics; the same trade-off the skill already
documents for the `export default function f(){} → function f(){}; export
default f;` case.
Verified:
- `pnpm proptypes --pattern "mui-system/src/(Box|ThemeProvider|DefaultPropsProvider|GlobalStyles)"` runs clean and is idempotent.
- `pnpm -F @mui/system build` succeeds.
- `grep -l "declare namespace" packages/mui-system/build/{ThemeProvider,DefaultPropsProvider,GlobalStyles}/*.d.ts` → empty.
- `pnpm -F @mui/system typescript` clean.
- ThemeProvider/GlobalStyles/useLayerOrder unit tests pass (18 tests).
The proptypes script auto-appends `as any` to the RHS when regenerating into
a TS source (`disablePropTypesTypeChecking` in `injectPropTypesInFile`); that
cast is harmless and doesn't reintroduce the namespace.
Janpot
added a commit
to Janpot/material-ui
that referenced
this pull request
May 29, 2026
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.
…aultPropsProvider/GlobalStyles" This reverts 611c650 and also rolls back the propTypes-related portion of 7083171 ("Clean up @mui/system TS-conversion artifacts") on these three files. Net: back to the Pattern A shape introduced by a8f4580 — plain `X.propTypes = {...} as any` LHS assignment with the dev-only `(X as any)['propTypes' + '']` exactProp reassignment. Trade-off: this re-introduces the `declare namespace X { var propTypes: any }` expando in the emitted .d.ts/.d.mts for ThemeProvider, DefaultPropsProvider, GlobalStyles, but keeps `pnpm proptypes` (test_static) green. The namespace artifact is benign at consume-time; the binding-shape fix (typed const + interface) is left for a follow-up if the artifact becomes a problem. The getThemeValue.ts change from 7083171 is unaffected.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR is the first slice of a mechanical conversion of @mui/system from hand-written .js + .d.ts pairs into single .ts/.tsx sources, following the same pattern already applied to @mui/utils, @mui/styled-engine (#48544), and @mui/private-theming (#48565). It collapses 26 of 41 leaf modules — style helpers, theming wrappers, and miscellaneous utilities — into TypeScript, while leaving 15 heavier modules (notably createStyled, createTheme/createTheme, styleFunctionSx, cssVars/createCssVarsProvider, colorManipulator, top‑level index, and CSSProperties) for a follow-up.
Changes:
- Convert 26 leaf modules to
.ts/.tsx, preserving the existing emitted JS shape viaas unknown as SimpleStyleFunction<...>casts and(X as any).propTypes /* remove-proptypes */ = ...expando assignments. - Enable
stripInternal: trueinpackages/mui-system/tsconfig.build.jsonand annotate runtime-only exports (borderTransform,sizingTransform,paletteTransform,breakpointKeys,values, spacingmarginKeys/paddingKeys,getThemeValueinternals) with/** @internal */. - Simplify
packages/mui-system/package.jsonexportsby removing per-subpath entries (Container,createBreakpoints,ThemeProvider,useThemeProps) now covered by the./*wildcard; remove@internalfrom Grid'sunstable_levelJSDoc (which surfaces it in generated API docs JSON as a side-effect).
Reviewed changes
Copilot reviewed 58 out of 85 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
packages/mui-system/tsconfig.build.json |
Enable stripInternal for declaration emit. |
packages/mui-system/package.json |
Drop subpath exports now matched by ./* wildcard. |
packages/mui-system/src/{borders,cssGrid,display,flexbox,palette,positions,shadows,sizing,spacing,typography}/*.{ts,d.ts,js} |
Collapse leaf style helpers into .ts with SimpleStyleFunction<...> casts. |
packages/mui-system/src/{breakpoints,createBreakpoints,createTheme/shape}/*.{ts,d.ts,js} |
Convert breakpoint helpers and shape default to TS; align breakpointKeys (was keys in old .d.ts). |
packages/mui-system/src/{compose,memoize,propsToClassKey,getThemeValue,styled,useTheme,useThemeWithoutDefault,useThemeProps,ThemeProvider,Container,Box,createBox}/*.{ts,tsx,d.ts,js} |
Convert utilities, hooks, providers, and Box/createBox to TS/TSX; runtime behavior unchanged. |
packages/mui-system/src/styleFunctionSx/{defaultSxConfig,extendSxProp}.{ts,d.ts,js} |
Convert defaultSxConfig and extendSxProp to TS while leaving styleFunctionSx itself for the follow-up. |
packages/mui-system/src/Grid/{Grid.tsx,GridProps.ts} |
Remove @internal from unstable_level JSDoc. |
docs/pages/system/api/{box,grid}.json, docs/translations/api-docs/grid/grid.json |
Regenerated API docs reflecting Box.tsx rename and Grid unstable_level visibility. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
zannager
added
the
scope: code-infra
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.
First slice of the
@mui/systemTypeScript conversion: 26 of 41 hand-written.js+.d.tspairs collapsed into a single.ts/.tsxsource, same setup as@mui/utils/@mui/styled-engine(#48544) /@mui/private-theming(#48565). A follow-up PR will cover the heavyweight files (createStyled,createTheme,styleFunctionSxcore,cssVars/createCssVarsProvider,colorManipulator, top-levelindex.{js,d.ts}, plus theCSSProperties.d.tsrename).Scope of this PR
borders,breakpoints,cssGrid,display,flexbox,palette,positions,shadows,sizing,spacing,typographyThemeProvider/,RtlProvider/,Container/index,useTheme,useThemeProps/,useThemeWithoutDefaultcompose,memoize,propsToClassKey,getThemeValueBox(Box.tsx+index.ts),createBox.tsx,styled.ts,createTheme/shape,createBreakpoints/,styleFunctionSx/{defaultSxConfig, extendSxProp}@mui/systemkeeps the same build pipeline (code-infra build --flat, no--skipTscsince the package was already partially TS). The remaining.js+.d.tspairs continue to coexist with the new.tsfiles exactly as the existing mixed-state files already do.Pragmatic conversion choices for this batch
as unknown as SimpleStyleFunction<…>casts to preserve the hand.d.ts's narrower types(X as any).propTypes /* remove-proptypes */ = { ... }for the propTypes assignments — suppresses the tsc expando and keeps the Babel production guard, as in the prior conversion PRsimport type * as React) where the JS doesn't need the runtime namespace.d.tswas broader/narrower than the.jsruntime are normalizedQuality gates
pnpm -F @mui/system buildsucceeds (tsc emits declarations)pnpm -F @mui/system typescriptpassespnpm -F @mui/material build(downstream consumer) succeedsOut of scope (follow-up PR)
The 15 remaining
.js+.d.tspairs in this package have richer types that benefit from a more careful per-file review than batch conversion allowed for:colorManipulator,createStyled,createTheme/{createTheme,index},styleFunctionSx/{styleFunctionSx,index},cssVars/createCssVarsProvider, top-levelindex.{js,d.ts},CSSProperties.d.ts. Those will land in a follow-up PR.Published artifact diff
https://code-infra-dashboard.onrender.com/diff-package?package1=https://pkg.pr.new/mui/material-ui/@mui/system@861f7cc&package2=https://pkg.pr.new/mui/material-ui/@mui/system@15dfe9b