Skip to content

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

Merged
lukastaegert merged 1 commit intomasterfrom
non-inlined-import-after-inlined
Jul 27, 2022
Merged

Handle generating outputs with non-inlined dynamic imports after inlined ones#4589
lukastaegert merged 1 commit intomasterfrom
non-inlined-import-after-inlined

Conversation

@lukastaegert
Copy link
Copy Markdown
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
Copy Markdown

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
Copy Markdown

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