[core] Use Babel assumptions instead of loose mode

[nicolo-ribaudo]

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

assumptions:

• allow more fine-grained control of "loose mode"
• are better self-documenting, since the name of the option tells what the option does
• don't require you to explicitly list the plugins you want to modify in your config, so when the plugin will be unnecessary because your targets evolve you will not have to remember to manually disable it

Came from #37422

[mui-bot]

Netlify deploy preview

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

@material-ui/core: parsed: +0.90% , gzip: +0.74%
@material-ui/lab: parsed: +0.64% , gzip: +0.53%
@material-ui/unstyled: parsed: +1.03% , gzip: +0.70%
@mui/material-next: parsed: +0.76% , gzip: +0.51%
@mui/joy: parsed: +1.04% , gzip: +0.87%

Bundle size report

Details of bundle changes

Generated by 🚫 dangerJS against fcf68f3

[nicolo-ribaudo]

Feel free to go through https://babeljs.io/docs/assumptions and check if there is anything else you want to enable :)

[nicolo-ribaudo]

One possible effect of this PR is that setSpreadProperties is applied more, because object spread is also compiled in some cases by the destructuring transform and you didn't enable loose mode in that plugin. Now all the object spreads are transformed with this assumption, regardless of which plugin actually does it.

[romgrk]

Looks good, those options make sense.

I think we can also enable for our docs?

material-ui/docs/babel.config.js

Lines 32 to 35 in 28c1c0a

	// TODO: Enable once nextjs uses babel 7.13
	// assumptions: {
	// noDocumentAll: true,
	// },

Do you wanna open a mirror PR on https://github.com/mui/mui-x? I'll get to it otherwise.

[eps1lon]

Awesome. I think I didn't add these originally because of conflicts with dependency conflicts with Next.js. Not sure if we still share a babel config with docs/ so this should be checked.

[romgrk]

I can confirm that adding the assumptions options to the docs (at least on the X side) works & improves performance. The docs for both core & X have been running with a different & outdated babel config than the one used for production release builds, so the docs have been running slower than they should.

There is also more cruft to remove from the docs babel config I think, can you confirm that removing the transformRuntimeVersion thing is fine at this point? (edit: this is for @eps1lon)

material-ui/docs/babel.config.js

Lines 27 to 45 in 8b407a9

	const { version: transformRuntimeVersion } = fse.readJSONSync(
	require.resolve('@babel/runtime-corejs2/package.json'),
	);
	module.exports = {
	// TODO: Enable once nextjs uses babel 7.13
	// assumptions: {
	// noDocumentAll: true,
	// },
	presets: [
	// backport of https://github.com/vercel/next.js/pull/9511
	[
	'next/babel',
	{
	'preset-react': { runtime: 'automatic' },
	'transform-runtime': { corejs: 2, version: transformRuntimeVersion },
	},
	],
	],

Ideally we'd share the babel configs but next.js has some requirements, so I'm not sure how feasable that is.

[eps1lon]

I was just saying that if they were shared, we should check if docs are still transpiled properly. But since they're separated, you don't need to.

[nicolo-ribaudo]

Hey sorry for opening the pull request and then disappearing. I have very limited knowledge about mui and your development setup, so I would appreciate if someone else could help getting this PR and the related changes to the finishing line 😅

I'm happy to give write access to my fork so that folks that don't already have write access to this repo can still push to this PR.

[ZeeshanTamboli]

I conducted browser tests on Browserstack for all browsers in this PR, including Safari, and encountered the same error mentioned in #36795 (comment). The error can be seen in the following CircleCI pipeline: https://app.circleci.com/pipelines/github/mui/material-ui/99494/workflows/4a552b39-6ae7-4e35-9797-035913c541e2/jobs/529880. We had attempted to remove these proposals in #36795.

[romgrk]

Not strictly related to this PR but I'll comment here: because the production docs site isn't using the loose options or the corresponding assumption, it's running about 10-15% slower than it should so our library appears slower than it is in reality. You should look into sharing the babel config between root & docs.

[michaldudak]

@romgrk have you actually measured that including the assumptions results in a substantial speedup?
Do you have an idea which one is responsible for this improvement?

[romgrk]

Yes, I didn't benchmark the core as extensively as X but I tested this use case:

• open https://mui.com/material-ui/react-autocomplete/
• click on the sidebar link to https://mui.com/material-ui/react-button/

The scripting time for that action takes ~540ms on my machine, ~60ms of which is spent in _objectSpread2 currently. See point 2 of mui/mui-x#9001 for more details on that topic.

You need to enable loose: true for '@babel/plugin-proposal-object-rest-spread' or set the assumption setSpreadProperties: true, both do the same thing. Ideally we should share the same babel config for docs & release, it helps us spot & avoid this kind of issue.

[nicolo-ribaudo]

@ZeeshanTamboli Regarding that error, if you want to support Safari 13 you shouldn't specify Safari 14 as your compilation target:
https://github.com/mui/material-ui/blob/master/.browserslistrc#L29

As an alternative, you can use @babel/preset-env's include option (https://babeljs.io/docs/babel-preset-env#include) to say "I know that my targets don't need plugin X, but please assume that it's still necessary for other reasons".

[ZeeshanTamboli]

@ZeeshanTamboli Regarding that error, if you want to support Safari 13 you shouldn't specify Safari 14 as your compilation target: https://github.com/mui/material-ui/blob/master/.browserslistrc#L29

I mentioned this in comment #36795 (comment), but according to @oliviertassinari, we support Safari (iOS) from v12.5 in comment #36795 (comment).

As an alternative, you can use @babel/preset-env's include option (https://babeljs.io/docs/babel-preset-env#include) to say "I know that my targets don't need plugin X, but please assume that it's still necessary for other reasons".

Before attempting that, I must determine the appropriate plugin to address the error. Specifically, I need to identify the unsupported syntax on Safari 13.1.2.

[nicolo-ribaudo]

Before attempting that, I must determine the appropriate plugin to address the error. Specifically, I need to identify the unsupported syntax on Safari 13.1.2.

https://github.com/babel/babel/blob/main/packages/babel-compat-data/data/plugins.json contains a list of all the preset-env plugins, with the minimum version of each browser that does not need them. You can probably find them looking for "safari": "14 and "safari": "13 (or better, just specify the correct targets so that Babel can automatically do this for you :P)

[ZeeshanTamboli]

@nicolo-ribaudo After adding the include option of preset-env, the previous error was resolved, but a new error is now being thrown:

Safari 13.1.2 (Mac OS 10.15.6) combineHooksSlotProps should work FAILED
	expected [ 'onFocus', 'onMouseDown', …(3) ] to deeply equal [ Array(5) ]
	AssertionError@/tmp/_karma_webpack_609540/commons.js:26804:22
	/tmp/_karma_webpack_609540/commons.js:882160:31
	assertEql@/tmp/_karma_webpack_609540/commons.js:883393:16
	methodWrapper@/tmp/_karma_webpack_609540/commons.js:889858:30
	[native code]
	assertEqual@/tmp/_karma_webpack_609540/commons.js:883332:15
	methodWrapper@/tmp/_karma_webpack_609540/commons.js:889858:30
	[native code]
	/tmp/_karma_webpack_609540/commons.js:149691:60

Any idea what could be the cause?

[nicolo-ribaudo]

It's possible that Safari 13.1.2 had a bug relative to order of keys with object spread, but I do not have access to it to test. I suggest editing that test and add a Object.keys(slotProps), to see what are the actual keys.

If they are correct but in the wrong order, then you could either:

1. decide that it's ok, and .sort() the two arrays before compiling them
2. add @babel/plugin-transform-object-rest-spread to includes, so that ... is transformed and it doesn't trigger that safari bug.

(2) would maintain parity with the previous babel config, since it forcefully compiled object rest/spread.

[ZeeshanTamboli]

The Object.keys(slotProps) has the correct order of keys but still I went with option 2 to try it. After doing that, the error is not triggered. But, there is no advantage in terms of bundle size - #37461 (comment).