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

(fix) - should reorder memoized children #1980

Merged
merged 2 commits into from
Oct 7, 2019
Merged

Conversation

JoviDeCroock
Copy link
Member

@JoviDeCroock JoviDeCroock commented Oct 5, 2019

So apparently the fix introduced in the other PR where we make children point to their parents make this line irrelevant.

Save bytes + fix code 🎉

-15B

@coveralls
Copy link

coveralls commented Oct 5, 2019

Coverage Status

Coverage increased (+0.1%) to 99.767% when pulling fe26385 on fix/memoizedChildren into 4db53a3 on master.

@marvinhagemeister
Copy link
Member

Oh that'd be really cool 👍💯

We should double check with the rest cases from #1879 just to be sure that we don't introduce any regressions

@JoviDeCroock
Copy link
Member Author

@marvinhagemeister can confirm the demo still works

@@ -90,7 +90,7 @@ export function diff(parentDom, newVNode, oldVNode, context, isSvg, excessDomChi
c.state = c._nextState;
c._dirty = false;
c._vnode = newVNode;
newVNode._dom = oldDom!=null ? oldDom!==oldVNode._dom ? oldDom : oldVNode._dom : null;
newVNode._dom = oldVNode._dom;
Copy link
Member

Choose a reason for hiding this comment

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

This line always confused me 😄 anyway. Good catch!

Copy link
Member

@cristianbote cristianbote left a comment

Choose a reason for hiding this comment

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

🎉 awesome

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.

5 participants