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

Improved shouldWrap logic (Closes #304) #355

Merged
merged 1 commit into from
Apr 30, 2020

Conversation

danielgindi
Copy link
Contributor

Rollup Plugin Name: commonjs

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: #304

Description

The older commonjs implementation avoided even looking inside modules that have import/export statements, and thus many require statements in mixed code were not looked into. Even more so when the main entry point was a pure es6 module.

The newer code with dynamic require support (#206) removed that disability, but introduced a new issue: ES6 modules with references to module were automatically wrapped with the commonjs helper, as the old shouldWrap logic did not account for es6 modules being digested.

This PR simply fixes that shouldWrap to avoid touching modules with import/export statements.

Copy link
Collaborator

@shellscape shellscape left a comment

Choose a reason for hiding this comment

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

LGTM

@shellscape shellscape merged commit 0ef0adb into rollup:master Apr 30, 2020
LarsDenBakker pushed a commit to LarsDenBakker/plugins that referenced this pull request Sep 12, 2020
michaelfig added a commit to Agoric/agoric-sdk that referenced this pull request Feb 22, 2021
michaelfig added a commit to Agoric/agoric-sdk that referenced this pull request Feb 22, 2021
michaelfig added a commit to Agoric/agoric-sdk that referenced this pull request Feb 22, 2021
kriskowal pushed a commit to endojs/endo that referenced this pull request Jan 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants