Skip to content

Fallback to render when subtrees are inserted on hydration#2438

Merged
JoviDeCroock merged 2 commits intomasterfrom
hydrate-fallback
Apr 3, 2020
Merged

Fallback to render when subtrees are inserted on hydration#2438
JoviDeCroock merged 2 commits intomasterfrom
hydrate-fallback

Conversation

@JoviDeCroock
Copy link
Copy Markdown
Member

@JoviDeCroock JoviDeCroock commented Apr 3, 2020

When we see a newly inserted tree, we fallback to non-hydration mode for that specific subtree. This was brought up in a slack issue.

We'll have to introduce a /debug warning for this as well in a separate PR.

@JoviDeCroock JoviDeCroock changed the title hydrate fallback Fallback to render when subtrees are inserted on hydration Apr 3, 2020
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 3, 2020

Size Change: +13 B (0%)

Total Size: 38.7 kB

Filename Size Change
dist/preact.js 3.72 kB +3 B (0%)
dist/preact.min.js 3.72 kB +3 B (0%)
dist/preact.module.js 3.74 kB +4 B (0%)
dist/preact.umd.js 3.79 kB +3 B (0%)
ℹ️ View Unchanged
Filename Size Change
compat/dist/compat.js 3.12 kB 0 B
compat/dist/compat.module.js 3.14 kB 0 B
compat/dist/compat.umd.js 3.17 kB 0 B
debug/dist/debug.js 3 kB 0 B
debug/dist/debug.module.js 2.98 kB 0 B
debug/dist/debug.umd.js 3.08 kB 0 B
devtools/dist/devtools.js 175 B 0 B
devtools/dist/devtools.module.js 185 B 0 B
devtools/dist/devtools.umd.js 252 B 0 B
hooks/dist/hooks.js 1.05 kB 0 B
hooks/dist/hooks.module.js 1.08 kB 0 B
hooks/dist/hooks.umd.js 1.12 kB 0 B
test-utils/dist/testUtils.js 437 B 0 B
test-utils/dist/testUtils.module.js 439 B 0 B
test-utils/dist/testUtils.umd.js 515 B 0 B

compressed-size-action

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 3, 2020

Coverage Status

Coverage remained the same at 99.8% when pulling 21547cb on hydrate-fallback into a299d1d on master.

Comment thread src/diff/index.js Outdated
}

if (dom == null) {
isHydrating = false;
Copy link
Copy Markdown
Member Author

@JoviDeCroock JoviDeCroock Apr 3, 2020

Choose a reason for hiding this comment

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

Might be cheaper to reuse the null assignment of line 313 and set isHydrating to null instead WDYT?

Copy link
Copy Markdown
Member

@developit developit Apr 3, 2020

Choose a reason for hiding this comment

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

I think we could just move this to 312/314 without changing its type to null. Terser will be able to merge the statements better there.

Copy link
Copy Markdown
Member

@developit developit left a comment

Choose a reason for hiding this comment

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

Fantastic. I have been using this modification locally, but it would have been be a bit before I would have been able to untangle it.

Copy link
Copy Markdown
Member

@andrewiggins andrewiggins left a comment

Choose a reason for hiding this comment

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

LGTM! Love the new tests 🕺

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.

3 participants