Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[core] Add exports field to packages #41596

Closed
wants to merge 23 commits into from
Closed
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
1646c6f
Initial commit from POC changes
DiegoAndai Mar 21, 2024
365aaea
Fix @app/next-app build
DiegoAndai Mar 21, 2024
a8a9c75
Refactor exports field flag
DiegoAndai Mar 21, 2024
330c04b
Update mui-material/src/styles/index.d.ts
DiegoAndai Mar 21, 2024
f3f6a46
Refactor build structure check and exports field computation
DiegoAndai Mar 22, 2024
bf84a60
Add `@mui/types` exports field
DiegoAndai Mar 22, 2024
91383ac
Breaking changes guide
DiegoAndai Mar 27, 2024
222eef3
Merge branch 'next' into add-exports-field
DiegoAndai Apr 10, 2024
bee0004
Refactor to add compatibility mode with previous configuration
DiegoAndai Apr 10, 2024
9b37c2a
Add modern export to exports field
DiegoAndai Apr 11, 2024
d7d133d
pnpm dedupe
DiegoAndai Apr 12, 2024
a28200d
pnpm prettier
DiegoAndai Apr 12, 2024
9fe59d8
Update broken link
DiegoAndai Apr 12, 2024
74cb37c
Exclude codemod and docs packages from exports format
DiegoAndai Apr 12, 2024
6bda919
pnpm dedupe
DiegoAndai Apr 12, 2024
6bad76d
Merge branch 'next' into add-exports-field
DiegoAndai Apr 15, 2024
5d0050b
pnpm install / pnpm dedupe
DiegoAndai Apr 15, 2024
35a501e
Merge branch 'next' into add-exports-field
DiegoAndai Apr 15, 2024
90a3325
Change "modern" custom export to "mui-moder"
DiegoAndai Apr 16, 2024
b94b4a6
Make exports field opt-in instead of opt-out
DiegoAndai Apr 16, 2024
6964f8b
Merge branch 'next' into add-exports-field
DiegoAndai Apr 16, 2024
7442410
Remove 'use client' directive from index files
DiegoAndai Apr 18, 2024
fb7a4ff
Exclude material-nextjs from exports format build
DiegoAndai Apr 18, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/pigment-css-vite-app/src/Slider/ZeroSlider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
import { isHostComponent, useSlotProps } from '@mui/base/utils';
import { styled } from '@pigment-css/react';
import { capitalize } from '@mui/material/utils';
import SliderValueLabel from '@mui/material/Slider/SliderValueLabel';
import { SliderValueLabel } from '@mui/material/Slider';
import { useSlider, valueToPercent } from '@mui/base/useSlider';
import { alpha, lighten, darken } from '@mui/system/colorManipulator';
import type { Theme } from '@mui/material/styles';
Expand Down
4 changes: 4 additions & 0 deletions babel.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,10 @@ module.exports = function getBabelConfig(api) {
]);
}

if (process.env.MUI_ADD_IMPORT_EXTENSIONS === 'true') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason to not always add the extensions?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The regressions and rollup (umd) builds use this config file as well, and those break if we add the extensions.

When we remove the umd build, I could test if the issue with the regressions build is fixable, and we can remove this check. Does that make sense?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally al tests use builds that are as close to the real build as possible

Copy link
Member Author

@DiegoAndai DiegoAndai Mar 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense, I'll look into adapting the regressions builds as well 👍🏼

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @Janpot, I investigated the issue a bit. The regressions build uses babel.config.js to compile the packages as well as other files, like the docs data files and the regressions tests files. It also points to the src of the packages instead of the build (see the regressions webpack config and the base webpack config).

I was able to use the packages builds instead of the src code by doing

diff --git a/test/regressions/webpack.config.js b/test/regressions/webpack.config.js
index b4472eedd1..c9d02bb705 100644
--- a/test/regressions/webpack.config.js
+++ b/test/regressions/webpack.config.js
@@ -64,6 +64,20 @@ module.exports = {
       // Exclude polyfill and treat 'zlib' as an empty module since it is not required. next -> gzip-size relies on it.
       zlib: false,
     },
+    alias: {
+      ...webpackBaseConfig.resolve.alias,
+      '@mui/material': path.resolve(__dirname, '../../packages/mui-material/build'),
+      '@mui/icons-material': path.resolve(__dirname, '../../packages/mui-icons-material/build/esm'),
+      '@mui/lab': path.resolve(__dirname, '../../packages/mui-lab/build'),
+      '@mui/styled-engine': path.resolve(__dirname, '../../packages/mui-styled-engine/build'),
+      '@mui/styled-engine-sc': path.resolve(__dirname, '../../packages/mui-styled-engine-sc/build'),
+      '@mui/styles': path.resolve(__dirname, '../../packages/mui-styles/build'),
+      '@mui/system': path.resolve(__dirname, '../../packages/mui-system/build'),
+      '@mui/private-theming': path.resolve(__dirname, '../../packages/mui-private-theming/build'),
+      '@mui/base': path.resolve(__dirname, '../../packages/mui-base/build'),
+      '@mui/utils': path.resolve(__dirname, '../../packages/mui-utils/build'),
+      '@mui/joy': path.resolve(__dirname, '../../packages/mui-joy/build'),
+    },
   },

But even after doing that I still had to filter out some files and not add the file extensions to them:

diff --git a/babel.config.js b/babel.config.js
index 12d09a03c2..2d3ba4a891 100644
--- a/babel.config.js
+++ b/babel.config.js
@@ -103,10 +103,6 @@ module.exports = function getBabelConfig(api) {
     ]);
   }
 
-  if (process.env.MUI_ADD_IMPORT_EXTENSIONS === 'true') {
-    plugins.push(['babel-plugin-add-import-extension', { extension: useESModules ? 'mjs' : 'js' }]);
-  }
-
   return {
     assumptions: {
       noDocumentAll: true,
@@ -119,6 +115,12 @@ module.exports = function getBabelConfig(api) {
         exclude: /\.test\.(js|ts|tsx)$/,
         plugins: ['@babel/plugin-transform-react-constant-elements'],
       },
+      {
+        exclude: /(\/test\/regressions|packages\/mui-docs|docs)/,
+        plugins: [
+          ['babel-plugin-add-import-extension', { extension: useESModules ? 'mjs' : 'js' }],
+        ],
+      },
     ],
     env: {
       coverage: {

This is because the files in those folders do not have extensions, as we're not adding them. We could add them, but I think this is outside of this PR's scope.

In conclusion, I would do

  1. Maintain the MUI_ADD_IMPORT_EXTENSIONS flag for this PR, adding the extensions only when building using build.mjs
  2. Remove the UMD build in a separate PR
  3. Create an issue to use the package builds instead of src for the regression tests. In that issue's PR, we could revisit removing MUI_ADD_IMPORT_EXTENSIONS and modifying the regression infrastructure accordingly.

What do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok for me

plugins.push(['babel-plugin-add-import-extension', { extension: useESModules ? 'mjs' : 'js' }]);
}

return {
assumptions: {
noDocumentAll: true,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import * as React from 'react';
// import of a small, pure module in a private demo
// bundle size and module duplication is negligible
/* eslint-disable-next-line no-restricted-imports */
import { convertLength } from '@mui/material/styles/cssUtils';
import { createTheme, responsiveFontSizes } from '@mui/material/styles';
import {
createTheme,
responsiveFontSizes,
unstable_convertLength as convertLength,
} from '@mui/material/styles';
import Box from '@mui/material/Box';
import { LineChart } from '@mui/x-charts';

Expand Down
39 changes: 39 additions & 0 deletions docs/data/material/migration/migration-v5/migration-v5.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,42 @@
This list is a work in progress.
Expect updates as new breaking changes are introduced.
:::

### Added exports field to package.json

The `exports` field has been added to the `@mui/material/package.json` file to improve the ESM and CJS builds split:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The `exports` field has been added to the `@mui/material/package.json` file to improve the ESM and CJS builds split:
The `exports` field has been added to the `@mui/material/package.json` file to improve the split between ESM and CJS builds:


```json title="@mui/material/package.json"
// ...
"exports": {
".": {
"types": "./index.d.ts",
"import": "./index.mjs",
"default": "./node/index.js"
},
"./*": {
"types": "./*/index.d.ts",
"import": "./*/index.mjs",
"default": "./node/*/index.js"
}
}
// ...
```

This limits the exported modules to the root import and one level deep imports.
If you were importing from deeper levels, you will need to update your imports:

Check warning on line 49 in docs/data/material/migration/migration-v5/migration-v5.md

View workflow job for this annotation

GitHub Actions / runner / vale

[vale] reported by reviewdog 🐶 [Google.Will] Avoid using 'will'. Raw Output: {"message": "[Google.Will] Avoid using 'will'.", "location": {"path": "docs/data/material/migration/migration-v5/migration-v5.md", "range": {"start": {"line": 49, "column": 47}}}, "severity": "WARNING"}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
If you were importing from deeper levels, you will need to update your imports:
If you were previously importing from deeper levels, you must update your imports as shown below:


```diff title="index.mjs"
- import buttonClasses from '@mui/material/Button/buttonClasses';
+ import { buttonClasses } from '@mui/material/Button';
```

```diff title="index.cjs"
- const { default: Button } = require('@mui/material/node/Button');
+ const { default: Button } = require('@mui/material/Button');
```

If you were importing from `/node` as a workaround, this is no longer necessary as the `exports` field maps CJS to the correct files.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
If you were importing from `/node` as a workaround, this is no longer necessary as the `exports` field maps CJS to the correct files.
If you were previously importing from `/node` as a workaround, this is no longer necessary because the `exports` field maps CJS to the correct files.

You might have to update your bundler configuration to support the new structure.

Read more about the `exports` field in the [Node.js documentation](https://nodejs.org/api/packages.html#exports).
39 changes: 31 additions & 8 deletions docs/data/system/migration/migration-v5/migration-v5.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,39 @@
Expect updates as new breaking changes are introduced.
:::

### Root code is now ESM
### Added exports field to package.json

The ESM code, previously under the `esm/` build, has been moved to the root of the package.
The CommonJS code, previously on the root, has been moved to the `node/` build.
The `exports` field has been added to the `@mui/system/package.json` file to improve the ESM and CJS builds split:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The `exports` field has been added to the `@mui/system/package.json` file to improve the ESM and CJS builds split:
The `exports` field has been added to the `@mui/system/package.json` file to improve the split between ESM and CJS builds:


:::info
This is an intermediate step to prepare for adding the `exports` field to the `package.json` file.
If you have trouble using this new structure, please wait for the future update which adds the `exports` field.
You can follow progress on https://github.com/mui/material-ui/issues/30671.
:::
```json title="@mui/system/package.json"
// ...
"exports": {
".": {
"types": "./index.d.ts",
"import": "./index.mjs",
"default": "./node/index.js"
},
"./*": {
"types": "./*/index.d.ts",
"import": "./*/index.mjs",
"default": "./node/*/index.js"
}
}
// ...
```

This limits the exported modules to the root import and one level deep imports.
If you were importing from deeper levels, you will need to update your imports:

Check warning on line 49 in docs/data/system/migration/migration-v5/migration-v5.md

View workflow job for this annotation

GitHub Actions / runner / vale

[vale] reported by reviewdog 🐶 [Google.Will] Avoid using 'will'. Raw Output: {"message": "[Google.Will] Avoid using 'will'.", "location": {"path": "docs/data/system/migration/migration-v5/migration-v5.md", "range": {"start": {"line": 49, "column": 47}}}, "severity": "WARNING"}

```diff
- import styled from '@mui/system/esm/styled';
+ import styled from '@mui/system/styled';
```

If you were importing from `/esm` as a workaround, this is no longer necessary as the `exports` field maps ESM to the correct files.
You might have to update your bundler configuration to support the new structure.

Read more about the `exports` field in the [Node.js documentation](https://nodejs.org/api/packages.html#exports).

### GridProps type

Expand Down
9 changes: 5 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,9 @@
},
"dependencies": {
"@googleapis/sheets": "^5.0.5",
"@slack/bolt": "^3.17.1",
"@netlify/functions": "^2.6.0",
"@slack/bolt": "^3.17.1",
"babel-plugin-add-import-extension": "^1.6.0",
"execa": "^8.0.1",
"google-auth-library": "^9.7.0"
},
Expand All @@ -106,17 +107,17 @@
"@babel/preset-typescript": "^7.23.3",
"@babel/register": "^7.23.7",
"@mnajdova/enzyme-adapter-react-18": "^0.2.0",
"@mui/internal-docs-utils": "workspace:^",
"@mui/internal-scripts": "workspace:^",
"@mui-internal/api-docs-builder": "workspace:^",
"@mui-internal/api-docs-builder-core": "workspace:^",
"@mui-internal/test-utils": "workspace:^",
"@mui/internal-docs-utils": "workspace:^",
"@mui/internal-scripts": "workspace:^",
"@mui/joy": "workspace:*",
"@mui/material": "workspace:^",
"@mui/utils": "workspace:^",
"@pigment-css/react": "workspace:^",
"@next/eslint-plugin-next": "^14.1.3",
"@octokit/rest": "^20.0.2",
"@pigment-css/react": "workspace:^",
"@playwright/test": "1.42.1",
"@types/enzyme": "^3.10.18",
"@types/fs-extra": "^11.0.4",
Expand Down
2 changes: 1 addition & 1 deletion packages/mui-core-downloads-tracker/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
},
"scripts": {
"build": "mkdir build && pnpm build:copy-files",
"build:copy-files": "node ../../scripts/copyFiles.mjs",
"build:copy-files": "node ../../scripts/copyFiles.mjs --skipExportsField",
DiegoAndai marked this conversation as resolved.
Show resolved Hide resolved
"prebuild": "rimraf build",
"release": "pnpm build && pnpm publish"
},
Expand Down
2 changes: 1 addition & 1 deletion packages/mui-icons-material/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
"build:modern": "echo 'Skip modern build'",
"build:node": "node ../../scripts/build.mjs node --largeFiles --outDir lib",
"build:stable": "node ../../scripts/build.mjs stable --largeFiles --outDir lib",
"build:copy-files": "node ../../scripts/copyFiles.mjs",
"build:copy-files": "node ../../scripts/copyFiles.mjs --skipExportsField",
"build:typings": "node ./scripts/create-typings.mjs",
"prebuild": "rimraf build",
"release": "pnpm build && pnpm publish",
Expand Down
12 changes: 10 additions & 2 deletions packages/mui-material/src/styles/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,11 @@ export { ComponentsProps, ComponentsPropsList } from './props';
export { ComponentsVariants } from './variants';
export { ComponentsOverrides, ComponentNameToClassKey } from './overrides';
export { Components } from './components';
export { getUnit as unstable_getUnit, toUnitless as unstable_toUnitless } from './cssUtils';
export {
getUnit as unstable_getUnit,
toUnitless as unstable_toUnitless,
convertLength as unstable_convertLength,
} from './cssUtils';

export type ClassNameMap<ClassKey extends string = string> = Record<ClassKey, string>;

Expand All @@ -97,7 +101,11 @@ export { default as makeStyles } from './makeStyles';
export { default as withStyles } from './withStyles';
export { default as withTheme } from './withTheme';

export * from './CssVarsProvider';
export {
useColorScheme,
getInitColorSchemeScript,
Experimental_CssVarsProvider,
} from './CssVarsProvider';

export { default as experimental_extendTheme } from './experimental_extendTheme';
export type {
Expand Down
12 changes: 10 additions & 2 deletions packages/mui-material/src/styles/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,11 @@ export function experimental_sx() {
export { default as createTheme, createMuiTheme } from './createTheme';
export { default as unstable_createMuiStrictModeTheme } from './createMuiStrictModeTheme';
export { default as createStyles } from './createStyles';
export { getUnit as unstable_getUnit, toUnitless as unstable_toUnitless } from './cssUtils';
export {
getUnit as unstable_getUnit,
toUnitless as unstable_toUnitless,
convertLength as unstable_convertLength,
} from './cssUtils';
export { default as responsiveFontSizes } from './responsiveFontSizes';
export { duration, easing } from './createTransitions';
export { default as useTheme } from './useTheme';
Expand All @@ -44,7 +48,11 @@ export { default as makeStyles } from './makeStyles';
export { default as withStyles } from './withStyles';
export { default as withTheme } from './withTheme';

export * from './CssVarsProvider';
export {
useColorScheme,
getInitColorSchemeScript,
Experimental_CssVarsProvider,
} from './CssVarsProvider';
export { default as experimental_extendTheme } from './experimental_extendTheme';
export { default as getOverlayAlpha } from './getOverlayAlpha';
export { default as shouldSkipGeneratingVar } from './shouldSkipGeneratingVar';
Expand Down
5 changes: 5 additions & 0 deletions packages/mui-types/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,5 +49,10 @@
"@types/react": {
"optional": true
}
},
"exports": {
".": {
"types": "./index.d.ts"
}
}
}
12 changes: 12 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions scripts/build.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ async function run(argv) {
NODE_ENV: 'production',
BABEL_ENV: bundle,
MUI_BUILD_VERBOSE: verbose,
MUI_ADD_IMPORT_EXTENSIONS: true,
};
const babelConfigPath = path.resolve(getWorkspaceRoot(), 'babel.config.js');
const srcDir = path.resolve('./src');
Expand Down Expand Up @@ -69,6 +70,8 @@ async function run(argv) {
}[bundle],
);

const outExtension = bundle === 'node' ? '.js' : '.mjs';

const babelArgs = [
'--config-file',
babelConfigPath,
Expand All @@ -77,6 +80,8 @@ async function run(argv) {
srcDir,
'--out-dir',
outDir,
'--out-file-extension',
outExtension,
'--ignore',
// Need to put these patterns in quotes otherwise they might be evaluated by the used terminal.
`"${ignore.join('","')}"`,
Expand Down
38 changes: 31 additions & 7 deletions scripts/copyFiles.mjs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/* eslint-disable no-console */
import path from 'path';
import yargs from 'yargs';
import {
createModulePackages,
createPackageFile,
Expand All @@ -23,9 +24,9 @@ async function addLicense(packageData) {
`;
await Promise.all(
[
'./index.js',
'./legacy/index.js',
'./modern/index.js',
'./index.mjs',
'./legacy/index.mjs',
'./modern/index.mjs',
'./node/index.js',
'./umd/material-ui.development.js',
'./umd/material-ui.production.min.js',
Expand All @@ -43,13 +44,14 @@ async function addLicense(packageData) {
);
}

async function run() {
const extraFiles = process.argv.slice(2);
async function run(argv) {
const { extraFiles, skipExportsField } = argv;

try {
// TypeScript
await typescriptCopy({ from: srcPath, to: buildPath });

const packageData = await createPackageFile();
const packageData = await createPackageFile(skipExportsField);

await Promise.all(
['./README.md', '../../CHANGELOG.md', '../../LICENSE', ...extraFiles].map(async (file) => {
Expand All @@ -67,4 +69,26 @@ async function run() {
}
}

run();
yargs(process.argv.slice(2))
.command({
command: '$0 [extraFiles..]',
description: 'copy files',
builder: (command) => {
return command
.positional('extraFiles', {
type: 'array',
default: [],
})
.option('skipExportsField', {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be a temporal flag until we close #35233, which we're doing next. Depending on how the icons package is handled it might or might not be necessary.

type: 'boolean',
default: false,
describe:
'Set to `true` if you wish to skip adding the exports field. Adding the exports field is only supported for top level ESM packages.',
});
},
handler: run,
})
.help()
.strict(true)
.version(false)
.parse();
Loading
Loading