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

Add dom.data during hydration to avoid redundantly adding text… #2238

Merged
merged 1 commit into from
Jan 8, 2020

Conversation

JoviDeCroock
Copy link
Member

@JoviDeCroock JoviDeCroock commented Jan 7, 2020

Due to the dom not being available as a node diffChildren will double add this text if we don't set the dom.data

I have been looking at solving it in diffChildren but the problem persists itself like this:

LOG: 'oldDom', hello bar
LOG: 'newChldren', ['hello ', 'bar']
LOG: 'oldChldren', []
LOG: 'excess', [hello bar]

So we effectively come in at the first newChild and diff it with oldDom, next up we have no oldDom and we just append it to the text node.

A potential perf improvement would be to check if the parent.innerHTML === props this would allow us to skip childDiffing for that text node (if it does not contain any dom nodes).

Fixes #2237

@jchip
Copy link

jchip commented Jan 7, 2020

wow! that was fast!!! thanks.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.002%) to 97.749% when pulling a0ee655 on fix-hydrate-text-nodes into f695f5a on master.

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.

👍 wooosh! nice!

Copy link
Member

@marvinhagemeister marvinhagemeister left a comment

Choose a reason for hiding this comment

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

Good catch 🙌💯

@marvinhagemeister marvinhagemeister changed the title Add dom.data during hydration to avoid redundantly adding text nodes Add dom.data during hydration to avoid redundantly adding text… Jan 8, 2020
@marvinhagemeister marvinhagemeister merged commit 2bc0b5f into master Jan 8, 2020
@marvinhagemeister marvinhagemeister deleted the fix-hydrate-text-nodes branch January 8, 2020 08:13
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.

10.2.0 hydrate strange behavior using JSDOM
5 participants