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

move keyed each diffing into a shared helper #1256

Merged
merged 3 commits into from
Mar 18, 2018
Merged

Conversation

Rich-Harris
Copy link
Member

This is a follow-up to both #1249 that starts to make good on the promise of #1173. It moves the logic for updating a keyed each block into a shared function.

While this does mean that a standalone component with a keyed each block will become slightly larger (because the function needs to accommodate e.g. blocks with and without transitions), there are substantial benefits in return: an app with two or more keyed each blocks will shrink (in terms of filesize, startup time and memory usage), and performance should improve because the function will warm up quicker if it's called from multiple places.

Granted, this is all fairly obvious from the point of view of a traditional framework.

An equally big win, in my view, is that the codebase itself becomes somewhat simpler and easier to reason about.

@Rich-Harris
Copy link
Member Author

Looks like the new algorithm breaks in some very specific cases. Looking into it...

@codecov-io
Copy link

codecov-io commented Mar 18, 2018

Codecov Report

Merging #1256 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1256      +/-   ##
==========================================
- Coverage   91.89%   91.87%   -0.02%     
==========================================
  Files         126      126              
  Lines        4552     4543       -9     
  Branches     1484     1477       -7     
==========================================
- Hits         4183     4174       -9     
  Misses        153      153              
  Partials      216      216
Impacted Files Coverage Δ
src/generators/nodes/EachBlock.ts 98.03% <ø> (-0.11%) ⬇️

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 10600eb...ffa45dd. Read the comment docs.

@Rich-Harris Rich-Harris changed the title [WIP] more keyed each diffing into a shared helper [WIP] move keyed each diffing into a shared helper Mar 18, 2018
@Rich-Harris Rich-Harris mentioned this pull request Mar 18, 2018
@Rich-Harris Rich-Harris merged commit 35a7fc6 into master Mar 18, 2018
@Rich-Harris Rich-Harris deleted the each-keyed-helper branch March 18, 2018 23:58
@Rich-Harris Rich-Harris changed the title [WIP] move keyed each diffing into a shared helper move keyed each diffing into a shared helper Mar 19, 2018
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.

2 participants