Unused modules in bundle

[erykpiast]

I've started analyzing material-ui part (quite big as it turned out) of my application bundle. My app is very simple for now, I use not many different widgets, so I was quite surprised seeing modules like Popover inside the bundle. I've traced dependency graph and I found quite a long path from Select (which I consciously use), through SelectInput and Menu modules, to that Popover thing. Why would I need it, I asked myself, I use only native Select! Quick look at source code of SelectInput gave me the answer: module Menu, which depends on Popover, is eagerly loaded even if it's not actually used. render method of SelectInput returns early in native branch of code and this is obviously not traceable by any three-shaker.

I don't have extensive experience with material-ui, so I don't know if native select is the only such case or that pattern (I mean conditional usage of component based on runtime properties) is common in the project. Are you interested in optimizaton of this? I suppose for big and large projects, using a lot of different UI components, it's not an issue at all, as that Popover component will be used in some other place eventually. For me, personally, savings are quite significant, though. Removing Menu and all its dependencies shrinks my bundle by 8% (11.5 kB gzipped).

• I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior

I'm able to use native select component without loading bunch of components used by not-native implementation.

Current Behavior

No matter if I use native or custom select, I have all code required by the latter one in my bundle.

Steps to Reproduce (for bugs)

https://codesandbox.io/s/p79koy741j

Quite a lot of dependencies for a single native select, don't you think?

Context

It affects performance somehow (I cannot provide you real numbers, but think of downloading 11.5 kB and parsing around 50 kB of never used JS). And performance matters, right?

Your Environment

Tech	Version
Material-UI	1.0.0-beta.33
React	16.2.0
browser	any
etc	-

[oliviertassinari]

@erykpiast Thanks for raising this issue! I agree. We need to do something. The best option I can think of is having two SelectInput components, then find some way to expose them with a clean API.

[dantman]

Maybe it's not perfect, but one alternative might be to add some sort of global constants. Like ALWAYS_USE_NATIVE_{SELECT,INPUT,...} and instead of if (native) { write if (ALWAYS_USE_NATIVE_SELECT || native) {.

Then if you are one of the subset of people that doesn't use non-native inputs you can declare those in your bundler's config. Then that would become if (true || native) {, which your bundler should hopefully recognize is always true, means the non-native code afterwards will never run, which means imports used only in that portion of code (Menu) are not used, which if you don't use them elsewhere means the exports aren't used, which the tree shaking algorithm should then decide to omit the exports of that module, which your minifier will then take as allowing it to strip the unused code from your bundle.

Basically, the same way if ( __DEV__ ) works!

Alternatively we could provide an official WebPack plugin like lodash-webpack-plugin (or perhaps a babel plugin). Which will change if (native) to if(true) in all the components you want that behaviour in.

[erykpiast]

@dantman It may work, but I think it’s more than native select. It’s about managing dependencies carefully.

[oliviertassinari]

What about moving forward on this issue by exposing a Select and a NativeSelect component? We can share the same stylesheet and Select.render() implementation between the two versions.

[dantman]

@oliviertassinari
Will <Select native /> still be available? We probably want to let <Select native={isMobile} /> be a pattern.
Unless we decide we instead want to add in a way to easily write micro components that switch between native/non-native.

[oliviertassinari]

Will <Select native /> still be available?

@dantman Yes, we can. The <Select native /> implementation overhead is minimal. But what about?

const Select = isMobile ? MuiNativeSelect : MuiSelect;

<Select />

Maybe it's not needed after all.

[erykpiast]

I can just say that the solution with separate native select component could solve my issue. In my opinion it’s good enough for now, especially if it doesn’t introduce breaking change to existing API :)

[olee]

I also think a separate NativeSelect component sharing most of it's code with Select would be preferrable

[oliviertassinari]

@olee Do you think we should keep supporting <Select native />? I'm working on this issue.
Not supporting it enables better type checking.

[olee]

Yes, because in a way, the non-native select is just an extension to the native select in a way and there's nearly no component overhead in importing NativeSelect when Select is imported, but the opposite is quite different.
Also it would keep the nice functionality to be able to show Select on desktop and NativeSelect on mobile with ease.

[sakulstra]

sorry for the ot question here, but where does the "isMobile" come from? I cannot find it anywhere in the docs, so i guess it's a 3th party package? Could be useful to add this to the docs as well.

[oliviertassinari]

where does the "isMobile" come from?

@sakulstra From some of your internal logic. You can use user-agent, screen-width, touch support or any other heuristics as you see fit.