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

Transpile the ESM bundle to ES5 but still use ES module system #106

Merged
merged 2 commits into from
Oct 7, 2020

Conversation

stefcameron
Copy link
Member

@stefcameron stefcameron commented Oct 7, 2020

Fixes #99

See webpack/webpack#5756

The initial approach was to add a browser target to package.json and
point it at the UMD build so that bundlers would pick that up instead of
the ESM build, since they (except for Rollup) tend to prefer browser
first, then module (ESM), then main.

But that would potentially affect tree-shaking, and we don't want to
hamper that optimizing process.

So this PR transpiles the code in the ESM bundle down to ES5, so that
it runs in IE9+ like the CJS and UMD bundles do, while keeping the
import/export ESM statements.

Features and Bug Fixes

  • Issue being fixed is referenced.
  • Test coverage added/updated.
  • Typings added/updated.
  • README updated (API changes, instructions, etc.).
  • Changes to dependencies explained.
  • Changeset added (run yarn changeset locally to add one, follow prompts).

Fixes #99

AFAICT, this seems to be the best solution to what __a lot__ of folks
in the community have been experiencing with ESM bundles, referenced
from a package's `module` entry, that aren't also transpiled down
to ES5 (which is technically in conflict with ESM, which is an ES6
feature).

The problem arises because this package states both `module` and
`main`, but Webpack defaults `mainFields` to
`['browser', 'module', 'main']`, in order. So it will always pick
`module` over `main` by default, because it doesn't understand
that the build's target is NOT a browser that supports ESM...

See webpack/webpack#5756
@changeset-bot
Copy link

changeset-bot bot commented Oct 7, 2020

🦋 Changeset detected

Latest commit: cca4d41

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@stefcameron
Copy link
Member Author

@khamiltonuk @bparish628 Trying to fix tabbable... I think this should get things fixed for the bundlers.

@bparish628
Copy link

bparish628 commented Oct 7, 2020

This would work in the sense that the code used by webpack would use transpiled code, but the main reason people like esm is because it gets treeshaken out.

I was looking at how other libraries (chakra-ui, emotion) do it and it seems that the code is transpiled to the current browser supported by the library in all of the supported module formats. It would be nice if this library also did that, where all formats would be transpiled down to the browser that this library says it supports (ie9).

I think most projects use esm so things get treeshaken out (with webpack), not necessarily on what browser's can run that code (that is what babel is for).

I think browser support and bundler module definitions are two different things. Browser support is babel and import/exports definitions are the bundler's job.

This is your library, so don't want to tell you what to do! Just giving my 2 cents 🙂

@stefcameron
Copy link
Member Author

@bparish628 Thanks for your thoughts. I appreciate it! Sure seems like conflicting intents for ESM bundles, but you make a good point about tree shaking with ESM imports and exports, and the browser approach would remove that possibility. I'll digest this a bit...

@stefcameron
Copy link
Member Author

I thought about this for a bit. Tree shaking is definitely compelling, and we've done work in this module recently to make it tree-shakable (see #39), so I wouldn't want to thwart that effort. I think I went astray when I equated the module system with the browser target, as you (@bparish628) pointed out:

I think browser support and bundler module definitions are two different things. Browser support is babel and import/exports definitions are the bundler's job.

When I think of ESM as just the module system, then it makes sense to transpile the code inside to the same browser target as the CJS and UMD builds.

So that's what I will do! Hopefully that will unbreak things happily. 😄

@stefcameron stefcameron changed the title Add browser entry to package.json Transpile the ESM bundle to ES5 but still use ES module system Oct 7, 2020
@stefcameron stefcameron merged commit 95563c2 into master Oct 7, 2020
@stefcameron stefcameron deleted the issue-99 branch October 7, 2020 23:35
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.

version 5.1.1 contains an arrow function
2 participants