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

Make sure labels are always included when not tree-shaking #3492

Merged
merged 1 commit into from Apr 9, 2020

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 #3473

Description

This PR makes sure

@rollup-bot
Copy link
Collaborator

rollup-bot commented Apr 9, 2020

Thank you for your contribution! ❤️

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

npm install rollup/rollup#gh-3473-always-include-labels

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

@codecov
Copy link

codecov bot commented Apr 9, 2020

Codecov Report

Merging #3492 into master will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3492      +/-   ##
==========================================
+ Coverage   95.94%   95.96%   +0.01%     
==========================================
  Files         174      174              
  Lines        5950     5947       -3     
  Branches     1752     1752              
==========================================
- Hits         5709     5707       -2     
+ Misses        124      123       -1     
  Partials      117      117              
Impacted Files Coverage Δ
src/ast/variables/Variable.ts 100.00% <ø> (+4.16%) ⬆️
src/Chunk.ts 99.78% <100.00%> (-0.01%) ⬇️
src/Module.ts 98.86% <100.00%> (-0.01%) ⬇️
src/ast/nodes/BreakStatement.ts 100.00% <100.00%> (ø)
src/ast/nodes/ContinueStatement.ts 100.00% <100.00%> (ø)
src/ast/nodes/ExportDefaultDeclaration.ts 96.55% <100.00%> (ø)
src/ast/nodes/Identifier.ts 100.00% <100.00%> (ø)
src/ast/nodes/LabeledStatement.ts 100.00% <100.00%> (ø)
src/ast/nodes/MemberExpression.ts 98.27% <100.00%> (ø)
src/ast/nodes/shared/FunctionNode.ts 100.00% <100.00%> (ø)
... and 3 more

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 027e1e5...1aa6f1a. Read the comment docs.

…iables, always inlcude labels when not treeshaking, resolves #3473
@lukastaegert lukastaegert force-pushed the gh-3473-always-include-labels branch from aebebb3 to 1aa6f1a Compare April 9, 2020 20:32
@lukastaegert lukastaegert merged commit 302c322 into master Apr 9, 2020
@lukastaegert lukastaegert deleted the gh-3473-always-include-labels branch April 9, 2020 21:04
@guybedford
Copy link
Contributor

Amazing, thanks for tracking this one down. Complete edge case I know, but hope the other fix was useful too.

@lukastaegert
Copy link
Member Author

It was also a potential source for future problems

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.

Label being incorrectly removed with treeshaking disabled
3 participants