-
Notifications
You must be signed in to change notification settings - Fork 208
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
Added module entry point in package.json #234
Conversation
Thanks @Andarist! We used to have I'm not going to close this PR just yet. If you or someone else can come up with a few workarounds for people who will run into trouble from this, I'm okay merging this and releasing it as v8 (so that people will have to make a manual upgrade + migration). |
The problem described in the other issue was caused by using Using |
I am a huge 👍 to this change as long as our build configuration outputs the right files. This is what I do on personal libraries as well as libraries at work, and it works out really well. Furthermore, numerous other widely-used JS libraries do this, too, such as Redux. This configuration is the de facto way to support tree shaking with webpack without any side effects caused by unsupported extensions such as I discouraged us from using @Andarist makes a great point here, as well:
tl;dr, I am strongly in favor of this change, and holding off on |
I agree with James. Let's ship module field and go to .js instead of .mjs
Although by now I think create react app supports mjs
…On Fri, Dec 22, 2017, 10:49 PM James, please ***@***.***> wrote:
I am a huge 👍 to this change as long as our build configuration outputs
the right files.
This is what I do on personal libraries as well as libraries at work, and
it works out amazingly. Furthermore, numerous other widely-used JS
libraries do this, too, such as Redux. This configuration is the *de
facto* way to support tree shaking with webpack without any side effects
caused by unsupported extensions such as .mjs.
I discouraged us from using .mjs in the original PR where we introduced
module, and, perhaps unsurprisingly, that decision caused problems for
our users. The problem has never been the module field (which is widely
supported); it’s the extension (which has very little/nonexistent ecosystem
support).
tl;dr, I am strongly in favor of this change, and holding off on .mjs
until it is more widely supported 👍
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#234 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAL7zvLUJnMBqqBaBk8exzXVfO_lUiOmks5tDKKRgaJpZM4RLAAr>
.
|
Thanks for all the input folks! |
Webpack2+ and rollup will understand this out of the box when importing from the lib, no need to import from
'react-waypoint/es'