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

Import compoments directly to avoid components/picker/index.js and have smaller bundle #245

Merged
merged 3 commits into from
Nov 13, 2018

Conversation

rosskhanas
Copy link

Those simple changes would allow importing Nimbleicker directly and avoid having all.json bundled when using apple.json or other emoji collections.

@rosskhanas
Copy link
Author

Probably related issue: #229

@EtienneLem
Copy link
Member

Interesting! I was under the impression that it was tree shaking’s job to be smart about these imports.

@EtienneLem EtienneLem merged commit d54b7f2 into missive:master Nov 13, 2018
@EtienneLem
Copy link
Member

Thanks 🤘

@rosskhanas
Copy link
Author

I think tree shaking is not possible with old require and only works with import. I might be wrong.

Anyway, this is a super simple way to reduce bundle size with no tree shaking and other advanced things. No need to import a lot and strip useless code if we can just import what is needed. 😃

Thank you for merging, would be nice to see a new emoji-mart version in NPM! 😊

@EtienneLem
Copy link
Member

I think tree shaking is not possible with old require and only works with import. I might be wrong.

That’s my understanding as well, which is why we have 2 dist folders and specify the one with import via package.json module.

"main": "dist/index.js",
"module": "dist-es/index.js",

But as far as I’m concerned we’ve been lied to… 😅

Thank you for merging, would be nice to see a new emoji-mart version in NPM! 😊

ETA few minutes 😄

EtienneLem added a commit that referenced this pull request Nov 13, 2018
Import all components directly, similar to #245
@rosskhanas
Copy link
Author

Before:

screen shot 2018-11-13 at 11 15 36 pm

After:
screen shot 2018-11-13 at 11 50 54 pm

Thank you 😃

@EtienneLem
Copy link
Member

Amazing! Thank YOU 🤘

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants