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

Improve how acorn is imported, update dependencies #2636

Merged
merged 2 commits into from Jan 9, 2019

Conversation

lukastaegert
Copy link
Member

This PR contains:

  • bugfix
  • feature
  • refactor
  • documentation
  • other

Are tests included?

  • yes (bugfixes and features will not be merged without tests)
  • no

Breaking Changes?

  • yes (breaking changes will not be merged unless absolutely necessary)
  • no

List any relevant issue numbers:
Closes #2628

Description

Since acorn unfortunately needs to be external now, there is an issue that only shows when rollup itself is bundled by another ESM aware bundler in that

  • rollup-plugin-commonjs will translate require('acorn') to a default import
  • the other bundler will attempt to use the ESM version of acorn that has no default export

This is only an issue with the two bundled acorn plugins for dynamic import and import.meta. The solution is to manually import the (more efficient!) ESM version of the dynamic import plugin and translate the remaining import manually in the renderChunk hook. This is not elegant but an error will be thrown if the expected import changes.

Also, some dependencies are updated.

@tchetwin
Copy link
Contributor

tchetwin commented Jan 7, 2019

Amazing @lukastaegert, thanks for the deep dive on this. ESM/require interop is fiendish!

I pulled this branch and verified locally that a Webpack workaround is not needed.

We have this part of our code base earmarked for some TLC so it will hopefully receive the ESM/TS treatment and we will once again feel enlightened 😃

@lukastaegert lukastaegert merged commit d2e2c9f into master Jan 9, 2019
@lukastaegert lukastaegert deleted the better-acorn-import branch January 9, 2019 06:29
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.

dist/rollup.es.js imports default export from acorn that's not there
3 participants