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.

Sign up for GitHub

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

Jump to bottom

Improve hydration tests #2899

Merged
merged 5 commits into from
Jan 2, 2021
Merged

Improve hydration tests #2899

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
40b2467
Assert event listeners in hydrate tests
andrewiggins Jan 2, 2021
ebe3bc3
Add some failing suspense hydration tests
andrewiggins Jan 2, 2021
d1fe0ab
Fix suspended hydration event listeners
andrewiggins Jan 2, 2021
12eff66
Add suspense hydration test with Components
andrewiggins Jan 2, 2021
b0c44b8
Isolate test case where lazy resolves to a Fragment
andrewiggins Jan 2, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 16 additions & 3 deletions compat/src/suspense.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ export function Suspense() {
Suspense.prototype = new Component();

/**
* @this {import('./internal').SuspenseComponent}
* @param {Promise} promise The thrown promise
* @param {import('./internal').VNode<any, any>} suspendingVNode The suspending component
*/
Expand Down Expand Up @@ -105,7 +106,12 @@ Suspense.prototype._childDidSuspend = function(promise, suspendingVNode) {

const onSuspensionComplete = () => {
if (!--c._pendingSuspensionCount) {
c._vnode._children[0] = removeOriginal(c.state._suspended);
// If the suspension was during hydration we don't need to restore the
// suspended children into the _children array
if (c.state._suspended) {
c._vnode._children[0] = removeOriginal(c.state._suspended);
}

c.setState({ _suspended: (c._detachOnNextRender = null) });

let suspended;
Expand All @@ -121,7 +127,7 @@ Suspense.prototype._childDidSuspend = function(promise, suspendingVNode) {
* While in non-hydration cases the usual fallback -> component flow would occour.
*/
const wasHydrating = suspendingVNode._hydrating === true;
if (!wasHydrating && !c._pendingSuspensionCount++) {
if (!c._pendingSuspensionCount++ && !wasHydrating) {
c.setState({ _suspended: (c._detachOnNextRender = c._vnode._children[0]) });
}
promise.then(onResolved, onResolved);
Expand All @@ -131,13 +137,20 @@ Suspense.prototype.componentWillUnmount = function() {
this._suspenders = [];
};

/**
* @this {import('./internal').SuspenseComponent}
* @param {import('./internal').SuspenseComponent["props"]} props
* @param {import('./internal').SuspenseState} state
*/
Suspense.prototype.render = function(props, state) {
if (this._detachOnNextRender) {
// When the Suspense's _vnode was created by a call to createVNode
// (i.e. due to a setState further up in the tree)
// it's _children prop is null, in this case we "forget" about the parked vnodes to detach
if (this._vnode._children)
if (this._vnode._children) {
this._vnode._children[0] = detachedClone(this._detachOnNextRender);
}

this._detachOnNextRender = null;
}

Expand Down
244 changes: 243 additions & 1 deletion compat/test/browser/suspense-hydration.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,12 @@ import React, {
Suspense
} from 'preact/compat';
import { logCall, getLog, clearLog } from '../../../test/_util/logCall';
import { setupScratch, teardown } from '../../../test/_util/helpers';
import {
createEvent,
setupScratch,
teardown
} from '../../../test/_util/helpers';
import { ul, li } from '../../../test/_util/dom';
import { createLazy } from './suspense-utils';

/* eslint-env browser, mocha */
Expand All @@ -17,6 +22,11 @@ describe('suspense hydration', () => {
rerender,
unhandledEvents = [];

const List = ({ children }) => <ul>{children}</ul>;
const ListItem = ({ children, onClick = null }) => (
<li onClick={onClick}>{children}</li>
);

function onUnhandledRejection(event) {
unhandledEvents.push(event);
}
Expand Down Expand Up @@ -101,6 +111,48 @@ describe('suspense hydration', () => {
});
});

it('should properly attach event listeners when suspending while hydrating', () => {
scratch.innerHTML = '<div>Hello</div><div>World</div>';
clearLog();

const helloListener = sinon.spy();
const worldListener = sinon.spy();

const [Lazy, resolve] = createLazy();
hydrate(
<Suspense>
<Lazy />
<div onClick={worldListener}>World!</div>
</Suspense>,
scratch
);
rerender(); // Flush rerender queue to mimic what preact will really do
expect(scratch.innerHTML).to.equal('<div>Hello</div><div>World!</div>');
expect(getLog()).to.deep.equal([]);
clearLog();

scratch.querySelector('div:last-child').dispatchEvent(createEvent('click'));
expect(worldListener, 'worldListener 1').to.have.been.calledOnce;

return resolve(() => <div onClick={helloListener}>Hello</div>).then(() => {
rerender();
expect(scratch.innerHTML).to.equal('<div>Hello</div><div>World!</div>');
expect(getLog()).to.deep.equal([]);

scratch
.querySelector('div:first-child')
.dispatchEvent(createEvent('click'));
expect(helloListener, 'helloListener').to.have.been.calledOnce;

scratch
.querySelector('div:last-child')
.dispatchEvent(createEvent('click'));
expect(worldListener, 'worldListener 2').to.have.been.calledTwice;

clearLog();
});
});

it('should allow siblings to update around suspense boundary', () => {
scratch.innerHTML = '<div>Count: 0</div><div>Hello</div>';
clearLog();
Expand Down Expand Up @@ -164,6 +216,196 @@ describe('suspense hydration', () => {
});
});

it('should properly hydrate suspense with Fragment siblings', () => {
const originalHtml = ul([li(0), li(1), li(2), li(3), li(4)].join(''));

const listeners = [
sinon.spy(),
sinon.spy(),
sinon.spy(),
sinon.spy(),
sinon.spy()
];

scratch.innerHTML = originalHtml;
clearLog();

const [Lazy, resolve] = createLazy();
hydrate(
<ul>
<Fragment>
<li onClick={listeners[0]}>0</li>
<li onClick={listeners[1]}>1</li>
</Fragment>
<Suspense>
<Lazy />
</Suspense>
<Fragment>
<li onClick={listeners[3]}>3</li>
<li onClick={listeners[4]}>4</li>
</Fragment>
</ul>,
scratch
);
rerender(); // Flush rerender queue to mimic what preact will really do
expect(scratch.innerHTML).to.equal(originalHtml);
expect(getLog()).to.deep.equal([]);
expect(listeners[4]).not.to.have.been.called;

clearLog();
scratch.querySelector('li:last-child').dispatchEvent(createEvent('click'));
expect(listeners[4]).to.have.been.calledOnce;

return resolve(() => (
<Fragment>
<li onClick={listeners[2]}>2</li>
</Fragment>
)).then(() => {
rerender();
expect(scratch.innerHTML).to.equal(originalHtml);
expect(getLog()).to.deep.equal([]);
clearLog();

scratch
.querySelector('li:nth-child(3)')
.dispatchEvent(createEvent('click'));
expect(listeners[2]).to.have.been.calledOnce;

scratch
.querySelector('li:last-child')
.dispatchEvent(createEvent('click'));
expect(listeners[4]).to.have.been.calledTwice;
});
});

it('should properly hydrate suspense with Component & Fragment siblings', () => {
const originalHtml = ul([li(0), li(1), li(2), li(3), li(4)].join(''));

const listeners = [
sinon.spy(),
sinon.spy(),
sinon.spy(),
sinon.spy(),
sinon.spy()
];

scratch.innerHTML = originalHtml;
clearLog();

const [Lazy, resolve] = createLazy();
hydrate(
<List>
<Fragment>
<ListItem onClick={listeners[0]}>0</ListItem>
<ListItem onClick={listeners[1]}>1</ListItem>
</Fragment>
<Suspense>
<Lazy />
</Suspense>
<Fragment>
<ListItem onClick={listeners[3]}>3</ListItem>
<ListItem onClick={listeners[4]}>4</ListItem>
</Fragment>
</List>,
scratch
);
rerender(); // Flush rerender queue to mimic what preact will really do
expect(scratch.innerHTML).to.equal(originalHtml);
expect(getLog()).to.deep.equal([]);
expect(listeners[4]).not.to.have.been.called;

clearLog();
scratch.querySelector('li:last-child').dispatchEvent(createEvent('click'));
expect(listeners[4]).to.have.been.calledOnce;

return resolve(() => (
<Fragment>
<ListItem onClick={listeners[2]}>2</ListItem>
</Fragment>
)).then(() => {
rerender();
expect(scratch.innerHTML).to.equal(originalHtml);
expect(getLog()).to.deep.equal([]);
clearLog();

scratch
.querySelector('li:nth-child(3)')
.dispatchEvent(createEvent('click'));
expect(listeners[2]).to.have.been.calledOnce;

scratch
.querySelector('li:last-child')
.dispatchEvent(createEvent('click'));
expect(listeners[4]).to.have.been.calledTwice;
});
});

it.skip('should properly hydrate suspense when resolves to a Fragment', () => {
Copy link
Member Author

Choose a reason for hiding this comment

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

Currently not supported but I didn't realize that until after I wrote the tests so since I wrote it I figured I'd add it so that maybe one day we can enable it.

const originalHtml = ul(
[li(0), li(1), li(2), li(3), li(4), li(5)].join('')
);

const listeners = [
sinon.spy(),
sinon.spy(),
sinon.spy(),
sinon.spy(),
sinon.spy(),
sinon.spy()
];

scratch.innerHTML = originalHtml;
clearLog();

const [Lazy, resolve] = createLazy();
hydrate(
<List>
<Fragment>
<ListItem onClick={listeners[0]}>0</ListItem>
<ListItem onClick={listeners[1]}>1</ListItem>
</Fragment>
<Suspense>
<Lazy />
</Suspense>
<Fragment>
<ListItem onClick={listeners[4]}>4</ListItem>
<ListItem onClick={listeners[5]}>5</ListItem>
</Fragment>
</List>,
scratch
);
rerender(); // Flush rerender queue to mimic what preact will really do
expect(scratch.innerHTML).to.equal(originalHtml);
expect(getLog()).to.deep.equal([]);
expect(listeners[5]).not.to.have.been.called;

clearLog();
scratch.querySelector('li:last-child').dispatchEvent(createEvent('click'));
expect(listeners[5]).to.have.been.calledOnce;

return resolve(() => (
<Fragment>
<ListItem onClick={listeners[2]}>2</ListItem>
<ListItem onClick={listeners[3]}>3</ListItem>
</Fragment>
)).then(() => {
rerender();
expect(scratch.innerHTML).to.equal(originalHtml);
expect(getLog()).to.deep.equal([]);
clearLog();

scratch
.querySelector('li:nth-child(4)')
.dispatchEvent(createEvent('click'));
expect(listeners[3]).to.have.been.calledOnce;

scratch
.querySelector('li:last-child')
.dispatchEvent(createEvent('click'));
expect(listeners[5]).to.have.been.calledTwice;
});
});

// TODO:
// 1. What if props change between when hydrate suspended and suspense
// resolves?
Expand Down
Loading