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

perf(statusTile): faster array creation and lodash lazy loading #335

Merged
merged 1 commit into from Dec 31, 2017

Conversation

@haroldtreen
Copy link
Contributor

@haroldtreen haroldtreen commented Dec 31, 2017

Description

Reduces the activation time for prettier-atom from 248ms down to ~60ms.

image

Changes

  • Removes the array spread operator from atomInterface. This stops babel-runtime from requiring large polyfills (babel-runtime/helpers/toConsumableArray). Saves ~70ms.
  • Lazy loads lodash/fp/flow in editorInterface. This function is only used for getCurrentDir, but gets loaded whenever editorInterface is loaded anywhere. Save ~120ms.
@codecov-io
Copy link

@codecov-io codecov-io commented Dec 31, 2017

Codecov Report

Merging #335 into master will decrease coverage by 0.97%.
The diff coverage is 30%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #335      +/-   ##
==========================================
- Coverage   78.49%   77.52%   -0.98%     
==========================================
  Files          30       30              
  Lines         479      485       +6     
  Branches       48       49       +1     
==========================================
  Hits          376      376              
- Misses         89       94       +5     
- Partials       14       15       +1
Impacted Files Coverage Δ
src/editorInterface/index.js 78.26% <28.57%> (-16.48%) ⬇️
src/atomInterface/index.js 88% <33.33%> (-3.67%) ⬇️

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 156a8e1...5f90f45. Read the comment docs.

Copy link
Collaborator

@robwise robwise left a comment

LGTM, congrats on your first PR on the repo!

}
return flow;
};

This comment has been minimized.

@robwise

robwise Dec 31, 2017
Collaborator

Maybe we will be able to revert this in a future PR if we implement babel-plugin-lodash? Or at least we can clean up a bit by implementing that import-lazy lib?

This comment has been minimized.

@haroldtreen

haroldtreen Dec 31, 2017
Author Contributor

Ya, I think there's a larger story to be tackled of "What's the pattern for lazy loading?".

This mirrors what's being used in main.js, but it can become a bit verbose.
Another PR for import-lazy would be interesting.

@robwise
Copy link
Collaborator

@robwise robwise commented Dec 31, 2017

@haroldtreen Before I merge, can you rebase and then run nps build to update the dist files and amend the commit?

@haroldtreen haroldtreen force-pushed the haroldtreen:optimize-activation-time branch from 563e257 to 900f5f9 Dec 31, 2017
Remove need for ES6 array polyfills and only import lodash once required

re #330
@haroldtreen haroldtreen force-pushed the haroldtreen:optimize-activation-time branch from 900f5f9 to 5f90f45 Dec 31, 2017
@robwise robwise merged commit 88d0687 into prettier:master Dec 31, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@haroldtreen
Copy link
Contributor Author

@haroldtreen haroldtreen commented Dec 31, 2017

Wooo! Thanks for the speedy response @robwise 🙌 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants