Skip to content

Fix additional let/var init bugs#4177

Merged
lukastaegert merged 4 commits intorollup:masterfrom
kzc:additional-variable-init-fixes
Jul 15, 2021
Merged

Fix additional let/var init bugs#4177
lukastaegert merged 4 commits intorollup:masterfrom
kzc:additional-variable-init-fixes

Conversation

@kzc
Copy link
Copy Markdown
Contributor

@kzc kzc commented Jul 12, 2021

Fix additional variable state change bugs including blockless if statement var declarations and let/var use before declaration within same function that do not cross function scopes.

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

Fixes several outstanding var/let init issues mentioned in #4148 without any code pessimization other than for previously incorrect rollup output, and with minimal overhead.

including blockless if statement `var` declarations
and let/var use before declaration within same function
@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 12, 2021

Codecov Report

Merging #4177 (ff8a078) into master (004f800) will increase coverage by 7.06%.
The diff coverage is 99.04%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4177      +/-   ##
==========================================
+ Coverage   91.27%   98.33%   +7.06%     
==========================================
  Files         170      202      +32     
  Lines        5923     7227    +1304     
  Branches     1794     2114     +320     
==========================================
+ Hits         5406     7107    +1701     
+ Misses        311       58     -253     
+ Partials      206       62     -144     
Impacted Files Coverage Δ
browser/path.ts 76.92% <ø> (ø)
cli/run/timings.ts 0.00% <0.00%> (ø)
src/ast/CallOptions.ts 100.00% <ø> (ø)
src/ast/ExecutionContext.ts 100.00% <ø> (ø)
src/ast/nodes/ExpressionStatement.ts 87.50% <ø> (ø)
src/ast/nodes/NewExpression.ts 100.00% <ø> (ø)
src/ast/nodes/NodeType.ts 100.00% <ø> (ø)
src/ast/nodes/ObjectExpression.ts 100.00% <ø> (+9.58%) ⬆️
src/ast/nodes/ObjectPattern.ts 88.23% <ø> (+1.56%) ⬆️
src/ast/nodes/Program.ts 100.00% <ø> (ø)
... and 317 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 ccdf124...ff8a078. Read the comment docs.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jul 14, 2021

Thank you for your contribution! ❤️

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

npm install kzc/rollup#additional-variable-init-fixes

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

Copy link
Copy Markdown
Member

@lukastaegert lukastaegert left a comment

Choose a reason for hiding this comment

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

I took the liberty of replacing the removed example in the docs with one that is still failing, otherwise we can merge this for now.

@kzc
Copy link
Copy Markdown
Contributor Author

kzc commented Jul 14, 2021

Works for me.

Comment thread docs/999-big-list-of-options.md Outdated
@lukastaegert lukastaegert merged commit 7c014fb into rollup:master Jul 15, 2021
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.

2 participants