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

[docs] Document the /es folder #10888

Merged
merged 1 commit into from
Apr 3, 2018

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Apr 2, 2018

Document and make the /es folder official as requested by @dantman.

Closes #10857, I believe the root of the problem is to find in webpack side. Ideally, I would have tried with Rollup to see if that works.

@oliviertassinari oliviertassinari added the docs Improvements or additions to the documentation label Apr 2, 2018
@dantman
Copy link
Contributor

dantman commented Apr 3, 2018

Note that #10857 is being closed not because /es fixes the tree shaking problem. But because whether we use index.es.js or the raw /es code WebPack does not tree shake the modules and we cannot figure out why.

/es makes unrelated optimizations in babel available, not tree shaking optimizations.

@oliviertassinari oliviertassinari merged commit e23dd6f into mui:v1-beta Apr 3, 2018
@oliviertassinari oliviertassinari deleted the docs-es-folder branch April 3, 2018 08:28
@dantman
Copy link
Contributor

dantman commented Apr 3, 2018

I actually use the /es folder via a WebPack alias. Perhaps I should add some documentation for that later.

@oliviertassinari
Copy link
Member Author

@dantman webpack is popular, why not.

@dantman
Copy link
Contributor

dantman commented Apr 4, 2018

Actually I'm not great at writing documentation. Writing can be difficult for me.

I just wanted to note that I use the following in my WebPack config:

	resolve: {
		alias: {
			// Load ECMAScript 6+/module variants of packages
			lodash: 'lodash-es',
			'material-ui': 'material-ui/es',
			'material-ui/styles': 'material-ui/es/styles',
			'material-ui-icons': 'material-ui-icons/es',
		},
	},

You can see the aliases for material-ui and material-ui-icons. I also alias material-ui/styles because I import it for now until #10858 is handled.

Doing it this way makes it so you don't need to explicitly write /es over an over when this is really a technical difference (import {TextField} from 'material-ui'; and import {TextField} from 'material-ui/es'; have the same API, the only difference is the behind the scenes syntax). And it also avoids the issue of 3rd party dependencies importing material-ui directly resulting in duplicates, since WebPack will rewrite material-ui to material-ui/es in those as well.

Although I suppose there is a chance that a 3rd party library could still try and import TextField from 'material-ui/TextField/TextField'; which would create duplicates.

@oliviertassinari
Copy link
Member Author

Although I suppose there is a chance that a 3rd party library could still try and import TextField from 'material-ui/TextField/TextField'; which would create duplicates.

@dantman Most third party libraries do import TextField from 'material-ui/TextField'; over import { TextField } from 'material-ui';. It's what I think we should encourage for now. For instance:

It's also the approach we document in all the demos. The react-router approach is interesting, but I what does it provide that the current approach doesn't?

@dantman
Copy link
Contributor

dantman commented Apr 4, 2018

@dantman Most third party libraries do import TextField from 'material-ui/TextField'; over import { TextField } from 'material-ui';.

Ok, then if I start using them I'm going to have to come up with an alternative configuration that rewrites the whole path based on a pattern instead of just aliasing individual modules.

@dantman
Copy link
Contributor

dantman commented Apr 17, 2018

I'm trying out this plugin config in WebPack instead of using resolve.alias:

// Force imports of packages like material-ui and material-ui-icons to use the /es versions
new webpack.NormalModuleReplacementPlugin(
	/^(material-ui|material-ui-icons)(\/|$)/,
	resource => {
		resource.request = resource.request.replace(
			/^([^/+]+)(?:\/es)?(\/.*)?$/,
			'$1/es$2'
		);
	}
),

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants