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

fix(build): Don't mix ES Module and CommonJS syntax #663

Merged
merged 1 commit into from Sep 17, 2019

Conversation

@pat-son
Copy link
Contributor

pat-son commented Sep 17, 2019

It is bad form to mix ES Module imports and CommonJS requires. It also causes problems when using Rollup, because it skips CommonJS module resolution when it encounters ES Module syntax. See here.

@jayphelps

This comment has been minimized.

Copy link
Member

jayphelps commented Sep 17, 2019

Awesome! Thank you! 🎉

For context, it was done this way during a time when ESM tree shaking was not yet as widely used and we wanted to exclude it from production builds. I think today tooling is in a good enough shape that if you care about bundle size enough to want the util helper excluded, using ESM tree shaking is the answer.

@jayphelps jayphelps merged commit 376dc5b into redux-observable:master Sep 17, 2019
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@jayphelps

This comment has been minimized.

Copy link
Member

jayphelps commented Sep 17, 2019

Released as 1.2.0 Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.