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

Get rid of immutable.js and implement tree-shaking for broken control flow #3153

Merged
merged 35 commits into from Oct 15, 2019

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

Description

This is a big thing I have been working on the past weeks.

  1. This gets rid of our biggest bundled dependency, immutable.js. This is done by passing a mutable context along the execution path that is also capable of tracking properties of the control flow, see below, and is much faster. All in all, this should make the browser build about 60kB or 15% smaller and the tree-shaking itself about 5-10% faster, depending on the project (probably nothing one would notice as overall it is more like 2% faster, but every little bit counts).
  2. To actually add a real feature, I used the context to implement tree-shaking for dead code behind throw, return and labeled and unlabeled break and continue statements. In the end, this turned out quite a bit more work than I anticipated as there is a lot of hidden combinatorics, e.g. when different labels interact. For instance this would be tree-shaken:
var skipIt = true;
while (true) {
  if (skipIt) {
    break;
  }
  console.log('this is ignored');
}

Here, the last log would be the only thing left in the function:

function test() {
  label: {
    if (someValue) {
      break label;
      console.log('removed');
    } else {
      return;
      console.log('removed');
    }
    console.log('removed');
  }
  console.log('only this one remains');
}

test();

And similar logic for switch statements and all kinds of loops. This is heavily tested but considering the possible combinatorics, it is still possible I missed things so it would be cool if some people tried it out.

@codecov
Copy link

codecov bot commented Oct 11, 2019

Codecov Report

Merging #3153 into master will increase coverage by 0.79%.
The diff coverage is 95.52%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3153      +/-   ##
==========================================
+ Coverage   89.32%   90.12%   +0.79%     
==========================================
  Files         165      165              
  Lines        5734     5874     +140     
  Branches     1739     1790      +51     
==========================================
+ Hits         5122     5294     +172     
+ Misses        379      350      -29     
+ Partials      233      230       -3
Impacted Files Coverage Δ
src/ExternalModule.ts 97.95% <ø> (ø) ⬆️
src/ast/nodes/shared/knownGlobals.ts 100% <ø> (ø) ⬆️
src/ast/nodes/NodeType.ts 100% <ø> (ø) ⬆️
src/ast/nodes/index.ts 100% <ø> (ø) ⬆️
src/ast/nodes/TemplateElement.ts 100% <ø> (ø) ⬆️
src/ast/scopes/ReturnValueScope.ts 100% <ø> (ø) ⬆️
src/ast/variables/ArgumentsVariable.ts 100% <ø> (ø) ⬆️
src/ast/nodes/VariableDeclarator.ts 100% <ø> (ø) ⬆️
src/ast/nodes/MetaProperty.ts 100% <ø> (ø) ⬆️
src/ast/variables/GlobalVariable.ts 100% <ø> (ø) ⬆️
... and 74 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 53266e6...8949aaa. Read the comment docs.

@lukastaegert lukastaegert merged commit 4b16548 into master Oct 15, 2019
@lukastaegert lukastaegert deleted the mutable-tree-shaking branch October 15, 2019 05:43
@Andarist
Copy link
Member

Dope 😍

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.

Unreachable code not removed
2 participants