-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
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] Migrate to import * as React from 'react' #19802
Conversation
@material-ui/core: parsed: -0.79% 😍, gzip: -0.17% 😍 Details of bundle changes.Comparing: 4c396bd...4854cde
|
What about jaredpalmer/tsdx#152 (comment)? |
React team did decision here. It's not an open question anymore. This may kinda break rollup though I think it's on user to provide all possible exports. Can be solved by this import * as react from 'react';
import * as reactDom from 'react-dom';
import * as reactIs from 'react-is';
import * as propTypes from 'prop-types';
...
commonjs({
namedExports: {
react: Object.keys(react),
'react-dom': Object.keys(reactDom),
'react-is': Object.keys(reactIs),
'prop-types': Object.keys(propTypes),
},
}), Also even this is gonna be fixed by rollup soon |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice. We can merge once CI is green.
I don't have much context on the difference, I would be curious to hear more about the following:
-import React, {Fragment, useCallback, useState} from 'react';
+import * as React from 'react';
+import {Fragment, useCallback, useState} from 'react'; I think that it would be interesting to delay things. We could see how the other React packages approach the problem or keep the changes internal (leaving the documentation outside of it). What do you think? |
Some packages denied |
Also material-ui-pickers imports namespace |
As far as I know So yeah we should stick with |
@eps1lon @TrySound Maybe we could employ two different strategies, |
@TrySound Thanks for the update. Should we apply the same change to the other packages (icons, lab, etc.)? |
That would be good. I opened this PR mostly as demo. |
Fine by me. Though it would help with the demos since we don't need |
As long as the demos match what most the developers are using in their applications, this sounds great. My concern is about not using Material-UI to influence the community both rather to follow it. In the case of JavaScript, I have very rarely seen |
This sounds out of character. I was under the impression you wanted to do the opposite. Maybe I misinterpreted past statements of yours. |
@eps1lon I think that Material-UI's objective should be providing what developers truly want (e.g. not what they say us they need). At the end of the day, it's all about how they feel about it. |
b3e895d
to
b6bebc5
Compare
So we do want to influence them. If we want to follow then we do what we're told. If we want to lead we're doing what we think they want. What you're describing is incoherent. |
Ok, per these terms definitions, it's incoherent. I think that we should be on the "leading" side of it. This means using empathy with the users, identifying their biggest problems, finding the root issues, finding solutions, implementing them, making sure the solution is technically solid, finally and more importantly, selling that our solution is great, create excitement. I would like to add the following note. There is a wide gap between what people say they will do and what they actually do. Actions are significantly more meaningful. In the previous message, when I said "what people truly want", I was referring to their future actions. |
Ref