Skip to content

Conversation

jsg2021
Copy link
Contributor

@jsg2021 jsg2021 commented Jan 31, 2020

This will allow users to consume the original esm modern code. This would make #275 a minor non-blocking issue. :)

@jsg2021
Copy link
Contributor Author

jsg2021 commented Feb 12, 2020

I rebased onto master

@josephfrazier josephfrazier changed the title expose esm entry for rollup/webpack Add module to package.json for rollup/webpack Feb 13, 2020
@josephfrazier josephfrazier merged commit 99ee554 into slevithan:master Feb 13, 2020
@josephfrazier
Copy link
Collaborator

Thanks! I still need a bit more time to evaluate #282 and #243, but this looks like a no-brainer.

@josephfrazier
Copy link
Collaborator

Hmm actually, after clicking through those PRs, I found this comment at #243 (comment):

I think we'd probably want the module file to point to something that has passed through babel, to make sure that the only non-ES5 feature used is the import/export syntax.

EDIT: See rollup/rollup/wiki/pkg.module#wait-it-just-means-import-and-export--not-other-future-javascript-features

I guess we can see if this is actually a problem, once this change is published.

@jsg2021
Copy link
Contributor Author

jsg2021 commented Feb 13, 2020

Ah well, if there are usages of non-standard syntax or features not in node 8, then we’d just remove this.

The only reason I proposed it was to prevent bundlers from importing (yet another version of/potentially conflicting) babel runtime. As well as potentially making the bundled size even smaller.

The real and final solution will be to reconfigure babel not to use the runtime. The Rollup change does this. (The runtime is meant to be used by applications to minimize babel helper code duplication at the app level... libraries are discouraged from using it)

@jsg2021 jsg2021 deleted the module-export branch February 13, 2020 08:27
@jsg2021
Copy link
Contributor Author

jsg2021 commented Feb 13, 2020

I've updated my other pr to produce a esm module so that the src goes through babel.

@jantimon
Copy link

@jsg2021 this change broke xregexp for all webpack users - please see #305

@jsg2021
Copy link
Contributor Author

jsg2021 commented Nov 18, 2020

All webpack users? or commonjs-webpack users?

@jantimon
Copy link

jantimon commented Nov 18, 2020

for all webpack users - we are not using commonjs but one of our dependencies does

@jsg2021
Copy link
Contributor Author

jsg2021 commented Nov 18, 2020

Ah

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.

3 participants