[core] Update scripts to support base-ui #45784

brijeshb42 · 2025-04-01T11:30:02Z

Some of the notable changes -

Updated getWorkspaceRoot function to get the root directory relative to the current working directory. This would allow us to use this function across other repos as well. Without this change, it would return a directory inside node_modules when used in other repos.
Updated the build script to customize the cjs output. Default continues to be the root of the build directory. This allows us to use this in base-ui repo to override this to cjs directory inside build.
Updated buildTypes.mts script to optionally accept a list of directories to copy the emitted types to (from the default esm directory). The default has been set to ['build', 'build/modern'] to cater to core repo.

The scripts have been tested in both base-ui and mui-next repos using git commit hash.

I have followed (at least) the PR section of the contributing guide.

mui-bot · 2025-04-01T12:18:40Z

Netlify deploy preview

https://deploy-preview-45784--material-ui.netlify.app/

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against 11b9247

brijeshb42 · 2025-04-01T16:11:28Z

scripts/utils.mjs

@@ -1,11 +1,33 @@
 import path from 'path';
 import url from 'url';
+import fs from 'fs';
+
+function findUpFile(fileName, cwd = process.cwd(), maxIterations = 5) {


Previously I was using find-up package but later realised that this won't work with the useReactVersion script if we were to import getWorkspaceRoot in that file since it runs before pnpm install.

But useReactVersion from core repo doesn't use getWorkspaceRoot, so maybe these changes aren't necessary anymore?

I'd like to eventually get rid of getWorkspaceRoot, there doesn't seem to be any case where this is strictly needed. It makes those scripts harder to port. We should just require users to pass a path if they want to operate on a different folder than CWD.

But this can be done in follow-up.

True. But its required in other scripts. We can't use it in useReactVersion anyway because there won't be any packages installed when the script runs.
Base UI was using their version of getWorkspaceRoot in the useReactVersion script. So I though I'd update it in core as well.
But reverted later since it won't work across repos.

scripts/useReactVersion.mjs

Janpot · 2025-04-01T18:13:33Z

scripts/build.mjs

@@ -143,6 +152,11 @@ yargs(process.argv.slice(2))
          describe:
            "Set to `true` if you don't want to generate a package.json file in the /esm folder.",
        })
+        .option('cjsDir', {


Does it make sense to do something different in base than in core?

Currently, base has two outputs cjs and esm dirs unlike core, were cjs is at the top level of build.
If we want to change the structure in either of the repos, it makes sense to do it in base since it is in alpha anyways. Though, I am sure this won't be a breaking change.
Personally, explicit cjs and esm (and modern) seems cleaner to me than what we are doing in core currently.

The idea behind the structure in core was to keep support for node10 resolution. Not sure how you can achieve that with the base structure.
The current structure makes it too easy to forget properties like sideEffects in the subfolder package.json (as is currently hapening in base)
IMO the ideal structure is a flat one with cjs and mjs file extensions. This avoids package.json lookups to determine module type. But that's a bit more difficult to achieve with our current setup.

I was surprised to find that this structure supports node10 resolution.

Something different in base-ui's package.json is that it lists each export individually. There are no * exports.
So if supporting node10 resolution was the only issue for us, I am sure we can restructure core's build output.
This could be done separately and when done, we can update the script to not have the new argument.

The current structure makes it too easy to forget properties like sideEffects in the subfolder package.json (as is currently hapening in base)

But with the updated code to use latest infra changes, this has been corrected.

I was surprised to find that this structure supports node10 resolution.

The type resolution yes, through typesVersions, I'm not sure it would actually resolve the javascript correctly under node10, I don't see how without support for the exports field. But I may be missing something

Something different in base-ui's package.json is that it lists each export individually

Yes, we're likely also going to move to something like that eventually. There's a mui-src condition that is supported by our package.json exports generator that helps with this. e.g.

material-ui/packages/mui-material/package.json

Line 109 in 7a7c530

"mui-src": "./src/ButtonBase/TouchRipple.ts"

@Janpot Yeah. The node10 resolution only works for types. Not for the implementation file.

scripts/buildTypes.mts

Janpot · 2025-04-01T18:17:02Z

scripts/buildTypes.mts

+          default: false,
+          describe: 'Set to `true` if you want the legacy behavior of just copying .d.ts files.',
+        })
+        .option('copy', {


Is it possible to just do the same thing in core and base?

while not requiring any changes in core repo.

brijeshb42 · 2025-04-02T07:03:31Z

.eslintrc.js

@@ -64,7 +64,7 @@ module.exports = /** @type {Config} */ ({
    'plugin:eslint-plugin-import/recommended',
    'plugin:eslint-plugin-import/typescript',
    'eslint-config-airbnb',
-    './eslint/config-airbnb-typescript.js',
+    require.resolve('./eslint/config-airbnb-typescript.js'),


This makes sure that the .eslintrc.js file can be imported from other repos without it trying to search for config-airbnb-typescript.js file in the local folders.

in eslint 9 we will be able to just list the preset as an object

Janpot

Should be good to move forward with 👍

brijeshb42 · 2025-04-02T07:13:41Z

I'll merge this once I am able to pass the builds on base-ui. This change breaks the attw script for module resolution which I am looking into.

brijeshb42 · 2025-04-02T11:06:37Z

Merging as I found the issue was about an old version of @mui/internal-babel-plugin-resolve-imports. Apparently a newer version hasn't been published since past two months. I wonder why.

brijeshb42 added core Infrastructure work going on behind the scenes scope: code-infra Specific to the core-infra product labels Apr 1, 2025

brijeshb42 force-pushed the code-infra-base-ui branch 4 times, most recently from 91ead23 to c81770b Compare April 1, 2025 12:16

brijeshb42 force-pushed the code-infra-base-ui branch 5 times, most recently from 9f4239d to b8e45c0 Compare April 1, 2025 14:43

brijeshb42 requested a review from Janpot April 1, 2025 14:53

brijeshb42 commented Apr 1, 2025

View reviewed changes

brijeshb42 force-pushed the code-infra-base-ui branch 2 times, most recently from 8470145 to d21f7a0 Compare April 1, 2025 16:14

Janpot reviewed Apr 1, 2025

View reviewed changes

scripts/useReactVersion.mjs Outdated Show resolved Hide resolved

brijeshb42 force-pushed the code-infra-base-ui branch from d21f7a0 to 737a9b0 Compare April 1, 2025 18:12

Janpot reviewed Apr 1, 2025

View reviewed changes

Brijesh Bittu added 3 commits April 2, 2025 00:05

[code-infra] Update scripts to support base-ui

5800760

Write package.json if output is in a subdirectory or non commonjs build

385b4d3

Update buildTypes to support base-ui requirements

8447112

while not requiring any changes in core repo.

brijeshb42 force-pushed the code-infra-base-ui branch from 737a9b0 to 8447112 Compare April 1, 2025 18:37

brijeshb42 changed the title ~~[code-infra] Update scripts to support base-ui~~ [core] Update scripts to support base-ui Apr 1, 2025

Add require.resolve to local eslint plugin import

11b9247

brijeshb42 commented Apr 2, 2025

View reviewed changes

Janpot approved these changes Apr 2, 2025

View reviewed changes

brijeshb42 merged commit a80e7b7 into mui:master Apr 2, 2025
19 checks passed

Janpot mentioned this pull request Apr 14, 2025

[code-infra] Remove type generation of modern build #45912

Merged

Uh oh!

[core] Update scripts to support base-ui #45784

[core] Update scripts to support base-ui #45784

Uh oh!

Conversation

brijeshb42 commented Apr 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mui-bot commented Apr 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Netlify deploy preview

Bundle size report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brijeshb42 Apr 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brijeshb42 Apr 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Janpot Apr 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brijeshb42 Apr 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Janpot left a comment

Choose a reason for hiding this comment

Uh oh!

brijeshb42 commented Apr 2, 2025

Uh oh!

brijeshb42 commented Apr 2, 2025

Uh oh!

Uh oh!

Uh oh!

brijeshb42 commented Apr 1, 2025 •

edited

Loading

mui-bot commented Apr 1, 2025 •

edited

Loading

brijeshb42 Apr 1, 2025 •

edited

Loading

brijeshb42 Apr 2, 2025 •

edited

Loading

Janpot Apr 2, 2025 •

edited

Loading

brijeshb42 Apr 2, 2025 •

edited

Loading