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

[code-infra] Update package layout for better ESM support #43264

Draft
wants to merge 49 commits into
base: master
Choose a base branch
from

Conversation

Janpot
Copy link
Member

@Janpot Janpot commented Aug 11, 2024

Add a new mode to the build:

  • Removes nested package.json files
  • Stores ESM files with .mjs extension
  • Add exports field to package.json
  • Leave icon package layout as is, but add exports field. Next time we generate it we'll have to update the exports field
  • Use rollup instead of babel to compile the modules. this makes it fully specify imports (made this optional for now)

Notes:

To Do:

Bundle size increase

The bundle size increase can be fully attributed to the inclusion of the following ESM/CJS interop helper in the webpack output:

Code
/******/ 	/* webpack/runtime/create fake namespace object */
/******/ 	(() => {
/******/ 		var getProto = Object.getPrototypeOf ? (obj) => (Object.getPrototypeOf(obj)) : (obj) => (obj.__proto__);
/******/ 		var leafPrototypes;
/******/ 		// create a fake namespace object
/******/ 		// mode & 1: value is a module id, require it
/******/ 		// mode & 2: merge all properties of value into the ns
/******/ 		// mode & 4: return value when already ns object
/******/ 		// mode & 16: return value when it's Promise-like
/******/ 		// mode & 8|1: behave like require
/******/ 		__webpack_require__.t = function(value, mode) {
/******/ 			if(mode & 1) value = this(value);
/******/ 			if(mode & 8) return value;
/******/ 			if(typeof value === 'object' && value) {
/******/ 				if((mode & 4) && value.__esModule) return value;
/******/ 				if((mode & 16) && typeof value.then === 'function') return value;
/******/ 			}
/******/ 			var ns = Object.create(null);
/******/ 			__webpack_require__.r(ns);
/******/ 			var def = {};
/******/ 			leafPrototypes = leafPrototypes || [null, getProto({}), getProto([]), getProto(getProto)];
/******/ 			for(var current = mode & 2 && value; typeof current == 'object' && !~leafPrototypes.indexOf(current); current = getProto(current)) {
/******/ 				Object.getOwnPropertyNames(current).forEach((key) => (def[key] = () => (value[key])));
/******/ 			}
/******/ 			def['default'] = () => (value);
/******/ 			__webpack_require__.d(ns, def);
/******/ 			return ns;
/******/ 		};
/******/ 	})();

This is caused by the following import

const maybeReactUseSyncExternalStore: undefined | any = (React as any)['useSyncExternalStore' + ''];

A potential switch to using use-sync-external-store has been explored in #43476 but ultimately it makes sense to keep it, it manifests in the webpack runtime.

@Janpot Janpot added the scope: code-infra Specific to the core-infra product label Aug 11, 2024
@Janpot Janpot changed the title [core] Try new build mode that creates package exports [core] Update package layout for better ESM support Aug 11, 2024
@mui-bot
Copy link

mui-bot commented Aug 11, 2024

Netlify deploy preview

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

Menu: parsed: +1.79% , gzip: +1.95%
Unstable_NumberInput: parsed: +5.11% , gzip: +6.01%
@material-ui/core/useMediaQuery: parsed: +5.27% , gzip: +4.86%
Select: parsed: +1.49% , gzip: +1.66%
Option: parsed: +6.94% , gzip: +7.46%
MenuItem: parsed: +6.65% , gzip: +6.88%
PigmentHidden: parsed: +1.13% , gzip: +1.07%
TabPanel: parsed: +12.63% , gzip: +12.16%
Tab: parsed: +7.42% , gzip: +7.56%
useAutocomplete: parsed: +5.19% , gzip: +5.72%
TablePagination: parsed: +7.75% , gzip: +8.63%
@material-ui/system: parsed: +0.79% , gzip: +0.76%
@mui/joy/Tooltip: parsed: +0.63% , gzip: +0.57%
@mui/joy/MenuList: parsed: +0.76% , gzip: +0.62%
@mui/joy/Menu: parsed: +0.49% , gzip: +0.54%
@mui/joy/Radio: parsed: +0.83% , gzip: +0.66%
Tooltip: parsed: +0.54% , gzip: +0.46%
@mui/joy/Drawer: parsed: +0.67% , gzip: +0.60%
@mui/joy/Checkbox: parsed: +0.80% , gzip: +0.62%
@mui/joy/Select: parsed: +0.47% , gzip: +0.47%
and 25 more changes

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against 029389f

@Janpot Janpot changed the title [core] Update package layout for better ESM support [code-infra] Update package layout for better ESM support Aug 12, 2024
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Aug 12, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Aug 23, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Aug 28, 2024
@@ -104,6 +104,14 @@
"engines": {
"node": ">=14.0.0"
},
"exports": {
"/ButtonBase/TouchRipple": {
Copy link
Member Author

Choose a reason for hiding this comment

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

"import": "./esm/*.js",
"require": "./*.js"
},
"./esm/*": {
Copy link
Member Author

Choose a reason for hiding this comment

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

Maintain backwards compatibility for users that have done

resolve: {
alias: [
{
find: /^@mui\/icons-material\/(.*)/,
replacement: '@mui/icons-material/esm/$1',
},
],
},


const exportedNames = new Set(Object.keys(packageExports));
for (const exportedName of exportedNames) {
const modernName = exportedName.replace(/^\./, legacyModernPrefix);
Copy link
Member Author

Choose a reason for hiding this comment

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

Maintain backwards compatibility for the legacy ./modern bundles. Just a re-export of the main exports, but with a ./modern prefix

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Sep 3, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: out-of-date The pull request has merge conflicts and can't be merged scope: code-infra Specific to the core-infra product
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants