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

Handle generating outputs with non-inlined dynamic imports after inlined ones #4589

Merged
merged 1 commit into from Jul 27, 2022

Conversation

lukastaegert
Copy link
Member

@lukastaegert lukastaegert commented Jul 27, 2022

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:

Description

The problem was that when Rollup generated an output with inlineDynamicImports: true, it would leave a flag on the (shared) ImportExpression AST node that would mark it is inlined which would not be removed for the next output. The result was that for the next output, the import would be rendered both as "inlined" and as "non-inlined", resulting in an error. The fix was indeed simple.
Admittedly, the state handling behind this is not ideal, but resolving this is a much bigger topic for another time.

@github-actions
Copy link

github-actions bot commented Jul 27, 2022

Thank you for your contribution! ❤️

You can try out this pull request locally by installing Rollup via

npm install rollup/rollup#non-inlined-import-after-inlined

or load it into the REPL:
https://rollupjs.org/repl/?pr=4589

@codecov
Copy link

codecov bot commented Jul 27, 2022

Codecov Report

Merging #4589 (c4355bf) into master (3be1609) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #4589   +/-   ##
=======================================
  Coverage   98.86%   98.86%           
=======================================
  Files         209      209           
  Lines        7344     7345    +1     
  Branches     2098     2098           
=======================================
+ Hits         7261     7262    +1     
  Misses         27       27           
  Partials       56       56           
Impacted Files Coverage Δ
src/ast/nodes/ImportExpression.ts 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us.

@lukastaegert lukastaegert force-pushed the non-inlined-import-after-inlined branch from 2d76951 to c4355bf Compare July 27, 2022 04:55
@lukastaegert lukastaegert merged commit 3abd545 into master Jul 27, 2022
@lukastaegert lukastaegert deleted the non-inlined-import-after-inlined branch July 27, 2022 05:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant