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 synthetic entry handling #3738

Merged
merged 5 commits into from Aug 19, 2020
Merged

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:
Resolves rollup/plugins#532

Description

This resolves several issues around namespace imports, dynamic imports and synthetic exports

  • Do not include the synthetic namespace by default in static entry points unless it is actually used to reduce file size
  • Use a helper to create a proper namespace object when in a non-es format, a namespace is imported from a chunk with default export mode. Previously, the default export would have become the namespace in such a situation; now it is properly an object with a default property.
  • Use the same variable when in a chunk, a namespace is both imported and reexported as a binding. Previously, this would have caused two namespace imports from the same dependency with different variables to be rendered.
  • Make sure the chunking of one output does not interfere with the namespace objects of another output. Different chunking/use of preserveModules can cause different namespaces to be rendered as objects. Previously, the information which namespaces become reified objects was shared between outputs. This is fixed now.
  • Do no try to handle the synthetic namespace when dynamically importing a module with synthetic named exports when preserving modules. This essentially fixes [commonjs] Undeclared value gets exported plugins#532. A side-effect is that when preserving modules, namespace imports do not contain synthetic values because here, we do not create reified namespace objects. Regular synthetic imports work, though. Note also that for technical reasons, the synthetic namespace object is still included.

@rollup-bot
Copy link
Collaborator

rollup-bot commented Aug 19, 2020

Thank you for your contribution! ❤️

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

npm install rollup/rollup#improve-synthetic-entry-handling

or load it into the REPL:
https://rollupjs.org/repl/?circleci=12680

@codecov
Copy link

codecov bot commented Aug 19, 2020

Codecov Report

Merging #3738 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3738   +/-   ##
=======================================
  Coverage   96.98%   96.99%           
=======================================
  Files         184      184           
  Lines        6432     6452   +20     
  Branches     1862     1872   +10     
=======================================
+ Hits         6238     6258   +20     
  Misses        103      103           
  Partials       91       91           
Impacted Files Coverage Δ
src/Bundle.ts 100.00% <100.00%> (ø)
src/Chunk.ts 100.00% <100.00%> (ø)
src/Graph.ts 100.00% <100.00%> (ø)
src/Module.ts 100.00% <100.00%> (ø)
src/ast/variables/NamespaceVariable.ts 100.00% <100.00%> (ø)
src/finalisers/shared/getExportBlock.ts 100.00% <100.00%> (ø)
src/finalisers/shared/getInteropBlock.ts 100.00% <100.00%> (ø)
src/utils/deconflictChunk.ts 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1347fd6...8f576d7. Read the comment docs.

@lukastaegert lukastaegert force-pushed the improve-synthetic-entry-handling branch 2 times, most recently from 939ed74 to 2a1f487 Compare August 19, 2020 04:40
@lukastaegert lukastaegert force-pushed the improve-synthetic-entry-handling branch from 2a1f487 to 8f576d7 Compare August 19, 2020 05:01
@lukastaegert lukastaegert merged commit 13e9f13 into master Aug 19, 2020
@lukastaegert lukastaegert deleted the improve-synthetic-entry-handling branch August 19, 2020 05:10
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.

[commonjs] Undeclared value gets exported
2 participants