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

Use rollup to generate bundles for UMD, ESM and CJS #63

Merged
merged 4 commits into from
Feb 28, 2019
Merged

Use rollup to generate bundles for UMD, ESM and CJS #63

merged 4 commits into from
Feb 28, 2019

Conversation

Joezo
Copy link
Contributor

@Joezo Joezo commented Feb 27, 2019

Co-authored-by: Jack Clark

What does this PR do?

Generates bundles for UMD, ESM and CommonJS. This brings with it support for browsers >= IE11.

Whilst working on this we published a few version to npm with the test tag so that it could be tested. This codepen has been created which uses the UMD build. We spun up a quick Create react app to test out ESM which worked great. The nextjs example app has also been updated locally to use the commonjs version.

Our rollup config was heavily influenced by the one in redux.

Bundle size has slightly increased but we now have increased browser support and our module is now tree shakeable, meaning potentially reduced size for end users.

Important
We now use Promises to write this library as transpiling from async/await is usually avoided in libraries as it's expensive. We opted instead to use untranspiled Promises and then inform the user they should provide an environment that includes Promises.

Related issues

Resolves #11

Checklist

  • I have checked the contributing document
  • I have added or updated any relevant documentation
  • I have added or updated any relevant tests

bmullan91
bmullan91 previously approved these changes Feb 27, 2019
Copy link
Contributor

@bmullan91 bmullan91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Joezo / @jackdclark LGTM - One question about async/await.

@Joezo Joezo force-pushed the rollup branch 2 times, most recently from 62cb4d3 to c7f7cfa Compare February 27, 2019 15:58
@Joezo Joezo requested a review from bmullan91 February 27, 2019 15:59
Copy link
Contributor

@bmullan91 bmullan91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only a few small things - everything else is perfect 👌

"contributors:generate": "all-contributors generate"
"contributors:generate": "all-contributors generate",
"build": "rollup -c",
"prepublishOnly": "npm run build"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to run npm run build in prepack also.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we add it in prepack I believe it will run twice when we run npm publish. Are we using npm pack?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think npm pack might be an edge case and if people want to use it they could do npm run build first?

Copy link
Contributor

@bmullan91 bmullan91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM :shipit:

@Joezo Joezo merged commit ae154d3 into master Feb 28, 2019
@Joezo Joezo deleted the rollup branch February 28, 2019 10:45
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.

4 participants