Skip to content

Remove redundant check in updateChildren - #117

Open
dy wants to merge 1 commit into
choojs:masterfrom
dy:patch-1
Open

dy wants to merge 1 commit into
choojs:masterfrom
dy:patch-1

Conversation

@dy

@dy dy commented Feb 19, 2020

Copy link
Copy Markdown

This check is handled by more generic following condition.

@goto-bus-stop

Copy link
Copy Markdown
Member

Hmm, I think this might be a performance optimization to avoid having to walk through all the children in the common case? Most of the time your tree keeps a similar shape, and an update doesn't move elements around, so it avoids calling same() for every sibling element then. I don't know what the actual impact is though.

@dy

dy commented Mar 2, 2020

Copy link
Copy Markdown
Author

https://github.com/choojs/nanomorph/blob/master/index.js#L127-L132

Not sure initializing a loop is perf hit - condition check looks like the cheapest thing possible. I'd speculate this clause is kept as part of the mindflow, without realizing the generalized case. Also it's a bit easier to understand the way it is now. But there doesn't seem to be any optimization, just a couple ifs and break. In fact that adds some bytes to bundle.

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