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

Way icons are imported #7940

Closed
nenadalm opened this issue Aug 28, 2017 · 6 comments
Closed

Way icons are imported #7940

nenadalm opened this issue Aug 28, 2017 · 6 comments
Labels
support: question Community support but can be turned into an improvement

Comments

@nenadalm
Copy link

Each icon needs to be imported separetely. E.g.:

import CallIcon from 'material-ui-icons/Call';
import CallEndIcon from 'material-ui-icons/CallEnd';
import ExitToAppIcon from 'material-ui-icons/ExitToApp';
import LanguageIcon from 'material-ui-icons/Language';
import RefreshIcon from 'material-ui-icons/Refresh';

<CallIcon />
<CallEndIcon />
<ExitToAppIcon />
<LanguageIcon />
<RefreshIcon />

Wouldn't it be better if all icons could be in signle alias and then used:

import * as i from 'material-ui-icons';

<i.Call />
<i.CallEnd />
<i.ExitToApp />
<i.Language />
<i.Refresh />

Or if somebody doesn't like aliases, they could still list all imports like:

import {
    Call as CallIcon,
    CallEnd as CallEndIcon,
    ExitToApp as ExitToAppIcon,
    Language as LanguageIcon,
    Refresh as RefreshIcon,
} from 'material-ui-icons';

<CallIcon />
<CallEndIcon />
<ExitToAppIcon />
<LanguageIcon />
<RefreshIcon />

?

In order to do that, typings would have to be written like

declare module 'material-ui-icons' {
    import SvgIcon from 'material-ui/SvgIcon';
    export class Call extends SvgIcon {}
}

instead of:

declare module 'material-ui-icons/Call' {
    import SvgIcon from 'material-ui/SvgIcon';
    export default class Call extends SvgIcon {}
}
@the-simian
Copy link

the-simian commented Aug 28, 2017

Wouldn't this greatly increase the bundle size and not take advantage of tree shaking?

specifically this:

import * as i from 'material-ui-icons';

<i.Call />
<i.CallEnd />
<i.ExitToApp />
<i.Language />
<i.Refresh />

Or are the bundlers now smart enough to only import what was used?

@oliviertassinari
Copy link
Member

oliviertassinari commented Aug 28, 2017

Wouldn't it be better if all icons could be in signle alias and then used:

@nenadalm As far as I know, you can already do the described pattern. It's supported.

@oliviertassinari oliviertassinari added the support: question Community support but can be turned into an improvement label Aug 28, 2017
@nenadalm
Copy link
Author

@oliviertassinari then you know it wrong :)

import * as i from 'material-ui-icons';
$ ./node_modules/.bin/webpack
...
ERROR in [at-loader] ./src/browser/components/component/view.tsx:14:20 
    TS2306: File '/var/www/html/project/node_modules/material-ui-icons/index.d.ts' is not a module.
...

some versions:

material-ui@1.0.0-beta.6
material-ui-icons@1.0.0-beta.5
typescript@2.5.1

@oliviertassinari
Copy link
Member

@sebald Is this a typescript issue?

@sebald
Copy link
Member

sebald commented Aug 29, 2017

Current release has no "index export" (#7820). It was added in #7874, if I didn't mess up.
So importing all the icons should be possible with the next release, I guess.

@nenadalm
Copy link
Author

ok, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
support: question Community support but can be turned into an improvement
Projects
None yet
Development

No branches or pull requests

4 participants