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

Enable treeShaking import in webpack #1572

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@samMeow
Copy link

samMeow commented Mar 7, 2019

As webpack (create react app) have enable treeshaking feature to minimize the bundling size.
(https://webpack.js.org/guides/tree-shaking/)
(https://github.com/webpack/webpack/blob/master/lib/optimize/SideEffectsFlagPlugin.js)

It would be good if react-dates can support this feature

@coveralls

This comment has been minimized.

Copy link

coveralls commented Mar 7, 2019

Coverage Status

Coverage remained the same at 84.486% when pulling 0d48d1e on samMeow:master into 64c9857 on airbnb:master.

@ljharb
Copy link
Member

ljharb left a comment

I prefer not to include nonstandard package.json flags; webpack's support for "module" is why node itself won't be able to use that field ever.

@@ -3,6 +3,8 @@
"version": "20.0.0",
"description": "A responsive and accessible date range picker component built with React",
"main": "index.js",
"module": "esm/index.js",
"sideEffects": false,

This comment has been minimized.

@ljharb

ljharb Mar 7, 2019

Member

this is inaccurate; some of our files do have side effects.

This comment has been minimized.

@samMeow

samMeow Mar 7, 2019

Author

Thanks for pointing this out. I have just update to initialize.js and all css
using Regex ^\w+\( to find all self executed function. Hope I didn't miss any.

@samMeow samMeow force-pushed the samMeow:master branch from b37de79 to 0d48d1e Mar 7, 2019

@samMeow

This comment has been minimized.

Copy link
Author

samMeow commented Mar 7, 2019

@ljharb I just curious and want to know more about the usage of module field in nodeJS. I have try my best to do some research.
So the original proposal capture in Nov 2016 where one of the approach is package opt-in.
original

Then after a long discussion around how to determine if a module is es6 or commonjs.
Basically arguments around package opt-in / different file extension, etc approach is will it add extra complexity or extra cost for developer and module system.

Then later the proposal now/ Aug 2017 use mjs file extension.

Node v11.11.0 experimental es module

@ljharb

This comment has been minimized.

Copy link
Member

ljharb commented Mar 8, 2019

Yes, the default is going to be the mjs file extension, but there may be additional mechanisms, including a package.json field. What it will never be is a string “module” field, because of webpack and other tools poisoning it.

@samMeow

This comment has been minimized.

Copy link
Author

samMeow commented Mar 8, 2019

Got you. I will close this merge request as it (module field/ package opt-in) violate the standard node package json purpose in this moment.

@samMeow samMeow closed this Mar 8, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.