[core] Ship modern bundle by eps1lon · Pull Request #22814 · mui/material-ui (original) (raw)

@eps1lon eps1lon commented

Sep 29, 2020

•

edited by oliviertassinari

The targets of the default bundle are not final. They will be locked on release of @material-ui/core@5.0.0 from npx browserslist --mobile-to-desktop "> 0.5%, last 2 versions, Firefox ESR, not dead, not IE 11, maintained node versions"

new Supported Platforms documentation
new "minimizing bundle size" guide

We now ship 4 bundles:

stable (default, formerly esm) which targets a snapshot (on release) of > 0.5%, last 2 versions, Firefox ESR, not dead, not IE 11"
modern (formerly es) which targets the last 1 version of evergreen browsers and active node (currently that is 14. We can't use current node since that would include odd-numbered version which is not intended for production code)
node (formerly default) which targets a snapshot (on release) of maintained node versions
legacy which is stable + IE11

/icons only ships node and stable since the other two bundles don't do anything for size but would double the install size which can be problematic (though I haven't been able to reproduce these issues).

I'm not implementing exports and ES modules for node yet. That would require considerably more effort (since we need to import with file extensions) and ES modules are still considered experimental on node (though I think they're planned to be stable in a couple of weeks I seem to recall).

Closes #15496
Closes #18447

/cc @mui-org/core-team For input on the targets (see .browserslistrc).

Future work:

Remove internal polyfills required for IE 11 and document it

@material-ui/core: parsed: -4.81% 😍, gzip: -5.94% 😍
@material-ui/lab: parsed: -5.25% 😍, gzip: -3.91% 😍
@material-ui/styles: parsed: -1.89% 😍, gzip: -2.18% 😍
@material-ui/system: parsed: -16.16% 😍, gzip: -16.15% 😍
@material-ui/utils: parsed: +8.90% , gzip: +3.46%

Details of bundle changes

Generated by 🚫 dangerJS against d565045

last 1 safari version
node 14

# snapshot of "> 0.5%, last 2 versions, Firefox ESR, not dead, not IE 11" when the last major is released.

Choose a reason for hiding this comment

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

Widening to 0.2% (which is what create-react-app is using; 0.5 is browserslist default) would include:

chrome 49
edge 18
ios_safari 9.3

Chrome and edge are evergreen so I think we can ignore these versions. Older Chrome versions used to be relevant since it was what google used to crawl sites but they since upgraded and are doing so more often.

Note that safari prior to 12 would need prefixing of gap and column-gap in ImageList. Prior to 10.3 these properties aren't supported at all.

ios_safari cannot be considered evergreen because it's tied to the OS and therefore the device. The devices that support iOS 9 but not iOS 12 are:

iPod Touch (5th generation)
iPhone 4S
iPhone 5
iPhone 5C
iPad Mini (1st generation)
iPad 2
iPad (3rd generation)
iPad (4th generation)
iPad Air
iPad Pro (12.9-inch 1st generation)

Using https://en.wikipedia.org/wiki/IOS_9#Supported_devices and https://en.wikipedia.org/wiki/IOS_12#Supported_devices

@material-ui/utils: parsed: +11.22% , gzip: +7.54%

Only due to missing module concatenation (which depends on the app bundle not library code). Stat size is now at 14.58 kB (down from 15.94 kB)

I'm not so sure about the [...] and ios_safari change

Me neither. Breaking imagelist on iOS < 12 was unintentional...

@mbrookes What do you mean? Only supporting iOS >=13 was intentional.

I have seen the following error in development mode coming from the changes (none of the docs pages are working in dev)

I haven't figured out why yet.

@mbrookes What do you mean? Only supporting iOS >=13 was intentional.

Sure but it's not clear whether this is viable. The benefit of dropping iOS < 13 is that we can use PointerEvent as well.

This was referenced

Oct 12, 2020

oliviertassinari added a commit to oliviertassinari/material-ui that referenced this pull request

Oct 17, 2020

oliviertassinari added a commit to oliviertassinari/material-ui that referenced this pull request

Oct 17, 2020

Does it going to ship ES6+ syntax or later? Want to see that happen

Does it going to ship ES6+ syntax or later? Want to see that happen

The /modern bundle targets the latest 1 versions of major browsers so this should have the least amount of transpilation.

The default bundle should have a lot less transpilation as well.

What is so specific about ES6 syntax that you'd want to see?

This was referenced

Nov 24, 2020

A second version of the components is also published, which
you can find under the [`/es` folder](https://unpkg.com/@material-ui/core/es/).
All the non-official syntax is transpiled to the [ECMA-262 standard](https://www.ecma-international.org/publications/standards/Ecma-262.htm), nothing more.
⚠️ In order to minimize duplication of code in users' bundles, library authors are strongly discouraged to import from any of the other bundles.

Choose a reason for hiding this comment

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

Sorry for commenting on a closed PR, but is this recommendation saying for libraries to only import either /modern or /legacy? Or is it saying to keep imports as they were before, e.g. @material-ui/core/Button (which would later be used alongside a Babel transform that repoints at either /legacy or /modern, when you build an actual app)?

Choose a reason for hiding this comment

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

Libraries should not deal with from where they import. This should all be up to the bundler. So yes, import only from @materialui/*/* in your library.

@oliviertassinari

I repent for using this PR to ask a question, but it's a small one:

Why does MUI distribution includes CommonJS? Won't all React components be bundled anyhow by a bundler that support esm?

What's the use case for a cjs build where a React component will be imported using require?

What's the use case for a cjs build where a React component will be imported using require?

SSR maybe

The ecosystem isn't ready for ES modules. For example, next.js requires CommonJS.

For example, next.js requires CommonJS.

Is it? NextJS is using webpack, which will prioritise module over main.

I've got a library of React components and I'm transpiling it with esm only and NextJS is reading it no problem.

I'm asking this question because to the best of my knowledge pretty much all React components are bundled using webpack (for the consuming app at least, not libraries) and webpack has been supporting esm for quite some time.

Is it?

Yes: vercel/next.js#23725

NextJS is using webpack, which will prioritise module over main.

module and main are only part of native ES modules.

I've got a library of React components and I'm transpiling it with esm only and NextJS is reading it no problem.

Are you sure you're using native ES modules and not some loose interpretation that bundlers used to use? For example, relative file imports do need a file extension. Once you have e.g. import Component from './Component' you no longer have native ES modules.

Are you sure you're using native ES modules

OK, probably my wrong (am I confusing between esm and es?) I'm not talking about native ES modules (the mjs stuff).

But, if I'm not wrong, the babel config I'm using is borrowed from how MUI used to build the esm folder a year or so ago:

env: { esm: { presets: [ [ '@babel/preset-env', { // Do not transform modules - keep as esm modules: false, targets: { node: '6.10', }, }, ], ], plugins: [['@babel/plugin-transform-runtime', { useESModules: true }]], }, },

You're probably shipping something that is only understood by bundlers. With commonJS at least node understands it. With proper ES modules node, browsers and bundlers will understand the code.

This comment has been minimized.

You never execute these on node, or import them without some bundler being involved?

You do when doing server-side rendering.

You do when doing server-side rendering.

But this is done via webpack, which goes to the "module" field in package.json and reads files distributed per the 4.1.2 config above alright.

NextJS uses webpack.
createReactApp uses webpack.
Is anybody out there builds React apps not via webpack or some other bundler that understand that sort of esm export like in 4.1.2?

OK. I think I see what I'm missing. I've been with NextJS for too long probably...

Some people (as in some React examples) do SSR where the server simply requires the React Component (no webpack involved). These people are unlikely to write cjs style - most likely they write es6 and have some transpiler for the code, but not for node_modules.

So I guess the answer is - for people who do SSR without a bundler.

Yes, the most simple SSR is to use react-dom/server package to render the JSX directly. That requires the code to be able to run in NodeJS directly.

presets: defaultPresets.concat(['@babel/preset-react', '@babel/preset-typescript']),
plugins: [
module.exports = function getBabelConfig(api) {
const useESModules = api.env(['legacy', 'modern', 'stable']);

Choose a reason for hiding this comment

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

Am I reading this right that legacy SHOULD use ESModules? This is almost certainly a mistake, right?

Choose a reason for hiding this comment

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

I believe this stands for legacy ESM.

From microbundle readme:

"module": "./dist/foo.module.js", // legacy ES Modules output (for bundlers)
"exports": "./dist/foo.modern.js", // modern ES2017 output

Choose a reason for hiding this comment

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

Am I reading this right that legacy SHOULD use ESModules? This is almost certainly a mistake, right?

Could you open an issue and fill out the template to describe the mistake? Discussions on merged PRs are oftentimes forgotten.

Choose a reason for hiding this comment

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

I guess I thought legacy was for ancient browsers like ie11, it seems that I might be incorrect about that. Thanks for the explanation, sorry for the mis-informed question. Have a nice day.

Choose a reason for hiding this comment

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

legacy is for IE11. But IE11 does not understand ES modules nor CommonJS modules i.e. the code always needs a bundler anyway at which point we might as well ship ES modules to help tree-shaking.

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

[ Show hidden characters]({{ revealButtonHref }})