[code-infra] Accomodate build requirements from mui-x #2
Conversation
brijeshb42
commented
Jul 17, 2025
brijeshb42
commented
- I have followed (at least) the PR section of the contributing guide.
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
| '--ignore', | ||
| // Need to put these patterns in quotes otherwise they might be evaluated by the used terminal. | ||
| `"${ignore.join('","')}"`, | ||
| ...babelFlags, |
There was a problem hiding this comment.
Not the best approach but works in the short term (allows babel to copy css files in the same directory structure to the output path).
scripts/build.mjs
Outdated
| ); | ||
| } else if (babelRuntimeVersion === 'catalog:') { | ||
| // resolve the version from the given package | ||
| const { stdout: listedBabelRuntime } = await exec('pnpm list "@babel/runtime" --json'); |
There was a problem hiding this comment.
Maybe we should just do this altogether instead of reading the version from package.json? Then we can forget about checking for catalog:.
There was a problem hiding this comment.
What do you mean by altogether ?
There was a problem hiding this comment.
I meant we don't need to actually check for catalog: but alwaus resolve the version with pnpm list.
But now that I'm thinking about it, the X method of resolving the catalog is wrong. We don't want the resolved version that's installed at that moment in X, we want the version range as defined in pnpm-workspace.yaml. This is important, because our users may potentially have an older @babel/runtime installed than us. The correct way would be to read it from pnpm-workspace.yaml instead.
There was a problem hiding this comment.
I checked the output and it actually outputs what specified in pnpm-workspace.yaml without the ^.
So I don't see the issue here because our packages already specify the minimum required version (which is also what's used in babel transform).
In this case, the output of pnpm list "@babel/runtime" --json is -
[
{
"name": "@mui/x-charts",
"version": "8.8.0",
"path": "/Users/brijesh/projects/mui/mui-x/packages/x-charts",
"private": false,
"dependencies": {
"@babel/runtime": {
"from": "@babel/runtime",
"version": "7.27.6",
"resolved": "https://registry.npmjs.org/@babel/runtime/-/runtime-7.27.6.tgz",
"path": "/Users/brijesh/projects/mui/mui-x/node_modules/.pnpm/@[email protected]/node_modules/@babel/runtime"
}
}
}
]And in catalog -
catalog:
'@babel/runtime': ^7.27.6Are you referring to the case when we specify ^7.27.6 in catalog but the actual version installed is ^7.27.8 due to renovate ?
There was a problem hiding this comment.
Are you referring to the case when we specify
^7.27.6in catalog but the actual version installed is^7.27.8due to renovate ?
Yes, exactly that! e.g. someone dedupes or recreates our lockfile, and the package updates without updating the range specifier. We would set 7.27.8 for the babel plugin which potentially makes it use helpers that are not available in 7.27.6, which one of our users may have installed due to the semver range ^7.27.6. Resulting in errors in their bundles that we wouldn't see in our repo. Instead if we supply ^7.27.6 to the plugin, it would only use helpers that are compatible with that whole version range. (cc @LukasTy)
* Allow passing of extra flags to the babel-cli * Check existence of package files (like license, changelogs etc) before copying them.
brijeshb42
force-pushed
the
infra-sync-x-build
branch
from
8fffd51 to
c3b4b50
Compare
| } else if (babelRuntimeVersion === 'catalog:') { | ||
| // resolve the version from the given package | ||
| // outputs the pnpm-workspace.yaml config as json | ||
| const { stdout: configStdout } = await exec('pnpm config list --json'); |
There was a problem hiding this comment.
Love this. Just one more thing, perhaps we should move the following check
if (!babelRuntimeVersion) {
throw new Error(
'package.json needs to have a dependency on `@babel/runtime` when building with `@babel/plugin-transform-runtime`.',
);
}underneath? Just in case for some reason whatsoever pnpmWorkspaceConfig.catalog['@babel/runtime']; is undefined
There was a problem hiding this comment.
Done. I've moved the PR to material-ui since I figured there are no conflicts with your mjs file PR - mui#46551
brijeshb42
force-pushed
the
infra-sync-x-build
branch
from
c3b4b50 to
ad869b4
Compare
