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

[Portal] Migrate to unstyled #24890

Merged
merged 8 commits into from
Feb 15, 2021
Merged

[Portal] Migrate to unstyled #24890

merged 8 commits into from
Feb 15, 2021

Conversation

povilass
Copy link
Contributor

@povilass povilass commented Feb 12, 2021

Task by #24857 request.

@mui-pr-bot
Copy link

mui-pr-bot commented Feb 12, 2021

@material-ui/unstyled: parsed: +2.70% , gzip: +2.51%

Details of bundle changes

Generated by 🚫 dangerJS against e892d03

Copy link
Member

@oliviertassinari oliviertassinari left a comment

Choose a reason for hiding this comment

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

I don't think that Portal should be prefixed with unstyled, as by definition, there are no styled version of it.

I think that the solution should be about moving the source from @material-ui/core to @material-ui/unstyled while reexporting the component in the core. The diff should be in the order of magnitude of +10/-10 LOCs.

@oliviertassinari oliviertassinari added the component: Portal The React component. label Feb 13, 2021
@povilass
Copy link
Contributor Author

Got it

@povilass
Copy link
Contributor Author

@oliviertassinari I left export from @material-ui/core/Portal if I remove it it will be breaking change.

@oliviertassinari oliviertassinari changed the title [Portal] Migration to unstyled [Portal] Migrate to unstyled Feb 14, 2021
Comment on lines +10 to +11
// Resolve imports like:
// import Portal from '@material-ui/unstyled/Portal';
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be done by babel-plugin-module-resolver?

Copy link
Member

@oliviertassinari oliviertassinari Feb 15, 2021

Choose a reason for hiding this comment

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

It also seems to work with:

diff --git a/babel.config.js b/babel.config.js
index 658f4724f9..f2a14b0472 100644
--- a/babel.config.js
+++ b/babel.config.js
@@ -132,6 +132,19 @@ module.exports = function getBabelConfig(api) {
           ],
         ],
       },
+      rollup: {
+        plugins: [
+          [
+            'babel-plugin-module-resolver',
+            {
+              root: ['./'],
+              alias: {
+                '@material-ui/unstyled': path.resolve(__dirname, './packages/material-ui-unstyled/src'),
+              },
+            },
+          ],
+        ],
+      },
       benchmark: {
         plugins: [
           ...productionPlugins,
diff --git a/packages/material-ui/package.json b/packages/material-ui/package.json
index fad598de19..2b67f5fdcf 100644
--- a/packages/material-ui/package.json
+++ b/packages/material-ui/package.json
@@ -31,7 +31,7 @@
     "build:modern": "node ../../scripts/build modern",
     "build:node": "node ../../scripts/build node",
     "build:stable": "node ../../scripts/build stable",
-    "build:umd": "cross-env BABEL_ENV=stable rollup -c scripts/rollup.config.js",
+    "build:umd": "cross-env BABEL_ENV=rollup rollup -c scripts/rollup.config.js",
     "build:copy-files": "node ../../scripts/copy-files.js",
     "build:types": "node ../../scripts/buildTypes",
     "extract-error-codes": "cross-env MUI_EXTRACT_ERROR_CODES=true yarn build:modern",
diff --git a/packages/material-ui/scripts/rollup.config.js b/packages/material-ui/scripts/rollup.config.js
index 8112288071..19460964f1 100644
--- a/packages/material-ui/scripts/rollup.config.js
+++ b/packages/material-ui/scripts/rollup.config.js
@@ -94,7 +94,7 @@ export default [
     external: Object.keys(globals),
     plugins: [
       nodeResolve(nodeOptions),
-      nestedFolder,
+      // nestedFolder,
       babel(babelOptions),
       commonjs(commonjsOptions),
       nodeGlobals(), // Wait for https://github.com/cssinjs/jss/pull/893

A preference?

Copy link
Member

Choose a reason for hiding this comment

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

Fine for now. I'll improve it in a follow-up. Our babel config already has all the relevant information. We just need to reconcile it with the rollup externals.

@oliviertassinari oliviertassinari merged commit 2d80e18 into mui:next Feb 15, 2021
@oliviertassinari
Copy link
Member

@povilass Alright, one item done :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: Portal The React component.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants