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] Upgrade Babel 6 to Babel 7 #10964

Merged
merged 1 commit into from
May 8, 2018

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Apr 8, 2018

Following vercel/next.js#4050, it's still early, but let's see what's missing to complete the upgrade.

Blockers

Module build failed: Error: Requires Babel "^7.0.0-0", but was loaded with "6.26.0". If you are sure you have a compatible version of @babel/core, it is likely that something in your build process is loading the wrong version. Inspect the stack trace of this error to look for the first entry that doesn't mention "@babel/core" or "babel-core" to see what is calling Babel.

Following the stack trace, the error is coming from:
https://github.com/mui-org/material-ui/blob/1b90855ab0553b5cdb4c5bb9c1a66d8cb5cd48c6/docs/src/modules/components/withRoot.js#L7

  • A weird non transpilated file issue.

@oliviertassinari oliviertassinari force-pushed the babel-7 branch 3 times, most recently from 646aefc to d52d3d3 Compare April 8, 2018 17:16
@oliviertassinari oliviertassinari force-pushed the babel-7 branch 2 times, most recently from a8ed928 to dbfbfe6 Compare April 8, 2018 18:42
@oliviertassinari oliviertassinari added the on hold There is a blocker, we need to wait label Apr 8, 2018
@oliviertassinari oliviertassinari self-assigned this May 8, 2018
@oliviertassinari oliviertassinari force-pushed the babel-7 branch 2 times, most recently from 59c3b8f to 9748f65 Compare May 8, 2018 12:52
@oliviertassinari oliviertassinari force-pushed the babel-7 branch 3 times, most recently from 57ebdf6 to 588464d Compare May 8, 2018 14:56
@oliviertassinari oliviertassinari removed the on hold There is a blocker, we need to wait label May 8, 2018
@oliviertassinari oliviertassinari force-pushed the babel-7 branch 3 times, most recently from 903b0fe to 78641e1 Compare May 8, 2018 16:07
@oliviertassinari oliviertassinari merged commit 68d7985 into mui:v1-beta May 8, 2018
@oliviertassinari oliviertassinari deleted the babel-7 branch May 8, 2018 16:45
@oliviertassinari
Copy link
Member Author

oliviertassinari commented May 8, 2018

The performance metrics I have to compare Babel 7 vs 6 cc @loganfsmyth

Execution time 👍 :

I have no clue what the incertainty is, so it might not be siginiticant:

-test_build: 02:39
+test_build 02:25
-test_browser 01:47
+test_browser 01:37
-test_unit 02:46
+test_unit 02:46

Generated code size output 👎 :

{
     "name": "The initial cost people pay for using one component",
     "webpack": true,
     "path": "packages/material-ui/build/Paper/index.js",
-    "limit": "25.4 KB"
+    "limit": "26.0 KB"
   },
   {
     "name": "The size of all the modules of material-ui.",
     "webpack": true,
     "path": "packages/material-ui/build/index.js",
-    "limit": "101.4 KB"
+    "limit": "102.3 KB"
   },
   {

@NMinhNguyen
Copy link
Contributor

NMinhNguyen commented May 9, 2018

@oliviertassinari just curious, which bundles do you measure? I decided to look inside the development UMD bundle and noticed lots of _extends and _createClass helpers duplicated everywhere. I haven't had time to look into it, but I think it could be related to BABEL_ENV or NODE_ENV not being set properly for UMD causing transform-runtime not to be applied? I might get some time later, so could try to investigate further. But feel free to pick this up yourself if you wish 🙂

@oliviertassinari
Copy link
Member Author

@NMinhNguyen Interesting. I can see that too. To investigate :)

@oliviertassinari
Copy link
Member Author

@NMinhNguyen You were right, the umd/dev bundle was missing the execution of the transform-runtime Babel plugin. It should be fixed with #11298.

which bundles do you measure?

I don't use a bundle. I use an entry point:
https://github.com/mui-org/material-ui/blob/835286cb57f765fc9ed74f3dd1880a28fea00d06/.size-limit#L8-L13

@lookfirst
Copy link
Contributor

Here is another idea... stop using babel and switch to the typescript compiler?

@oliviertassinari
Copy link
Member Author

@lookfirst 1. We don't use TypeScript. 2. Babel is working on adding TypeScript support to it.

@lookfirst
Copy link
Contributor

lookfirst commented May 15, 2018

@oliviertassinari You don't have to write typescript (language) to use the typescript (compiler). I'd be curious if it produces smaller code.

@lookfirst
Copy link
Contributor

lookfirst commented May 15, 2018

@oliviertassinari As an experiment, I've been trying to compile using tsc and have made good progress, but I'm finding a bunch of issues, for example:

https://github.com/mui-org/material-ui/blob/v1-beta/packages/material-ui/src/Collapse/Collapse.d.ts#L3

...

src/Collapse/Collapse.d.ts(3,10): error TS2305: Module '"/material-ui/packages/material-ui/src/styles/createMuiTheme"' has no exported member 'Theme'.
src/Drawer/Drawer.d.ts(6,10): error TS2305: Module '"/material-ui/packages/material-ui/src/styles/createMuiTheme"' has no exported member 'Theme'.
src/Fade/Fade.d.ts(2,10): error TS2305: Module '"/material-ui/packages/material-ui/src/styles/createMuiTheme"' has no exported member 'Theme'.
src/Grow/Grow.d.ts(3,10): error TS2305: Module '"/material-ui/packages/material-ui/src/styles/createMuiTheme"' has no exported member 'Theme'.
src/Slide/Slide.d.ts(2,10): error TS2305: Module '"/material-ui/packages/material-ui/src/styles/createMuiTheme"' has no exported member 'Theme'.
src/Typography/Typography.d.ts(3,10): error TS2305: Module '"/material-ui/packages/material-ui/src/styles/createTypography"' has no exported member 'Style'.
src/Typography/Typography.d.ts(3,17): error TS2305: Module '"/material-ui/packages/material-ui/src/styles/createTypography"' has no exported member 'TextStyle'.
src/Zoom/Zoom.d.ts(2,10): error TS2305: Module '"/material-ui/packages/material-ui/src/styles/createMuiTheme"' has no exported member 'Theme'.
src/styles/MuiThemeProvider.d.ts(3,10): error TS2305: Module '"/material-ui/packages/material-ui/src/styles/createMuiTheme"' has no exported member 'Theme'.

@zannager zannager added the core Infrastructure work going on behind the scenes label Mar 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Infrastructure work going on behind the scenes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants