Skip to content

Commit

Permalink
Comment denoted hydration (#4636)
Browse files Browse the repository at this point in the history
* Remove unused imports

* Comment denoted hydration

* Make it work

* Golfies
  • Loading branch information
JoviDeCroock committed Feb 18, 2025
1 parent b597b78 commit 49bd318
Show file tree
Hide file tree
Showing 3 changed files with 284 additions and 62 deletions.
292 changes: 236 additions & 56 deletions compat/test/browser/suspense-hydration.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -723,61 +723,17 @@ describe('suspense hydration', () => {
});
});

// Currently not supported. Hydration doesn't set attributes... but should it
// when coming back from suspense if props were updated?
it.skip('should hydrate and update attributes with latest props', () => {
const originalHtml = '<p>Count: 0</p><p data-count="0">Lazy count: 0</p>';
scratch.innerHTML = originalHtml;
clearLog();

/** @type {() => void} */
let increment;
const [Lazy, resolve] = createLazy();
function App() {
const [count, setCount] = useState(0);
increment = () => setCount(c => c + 1);

return (
<Suspense>
<p>Count: {count}</p>
<Lazy count={count} />
</Suspense>
);
}

hydrate(<App />, scratch);
rerender(); // Flush rerender queue to mimic what preact will really do
expect(scratch.innerHTML).to.equal(originalHtml);
// Re: DOM OP below - Known issue with hydrating merged text nodes
expect(getLog()).to.deep.equal(['<p>Count: .appendChild(#text)']);
clearLog();

increment();
rerender();

expect(scratch.innerHTML).to.equal(
'<p>Count: 1</p><p data-count="0">Lazy count: 0</p>'
);
expect(getLog()).to.deep.equal([]);
clearLog();

return resolve(({ count }) => (
<p data-count={count}>Lazy count: {count}</p>
)).then(() => {
rerender();
expect(scratch.innerHTML).to.equal(
'<p>Count: 1</p><p data-count="1">Lazy count: 1</p>'
);
// Re: DOM OP below - Known issue with hydrating merged text nodes
expect(getLog()).to.deep.equal(['<p>Lazy count: .appendChild(#text)']);
clearLog();
});
});

// Currently not supported, but I wrote the test before I realized that so
// leaving it here in case we do support it eventually
it.skip('should properly hydrate suspense when resolves to a Fragment', () => {
const originalHtml = ul([li(0), li(1), li(2), li(3), li(4), li(5)]);
it('should properly hydrate suspense when resolves to a Fragment', () => {
const originalHtml = ul([
li(0),
li(1),
'<!--$s-->',
li(2),
li(3),
'<!--/$s-->',
li(4),
li(5)
]);

const listeners = [
sinon.spy(),
Expand Down Expand Up @@ -809,8 +765,8 @@ describe('suspense hydration', () => {
scratch
);
rerender(); // Flush rerender queue to mimic what preact will really do
expect(scratch.innerHTML).to.equal(originalHtml);
expect(getLog()).to.deep.equal([]);
expect(scratch.innerHTML).to.equal(originalHtml);
expect(listeners[5]).not.to.have.been.called;

clearLog();
Expand Down Expand Up @@ -839,4 +795,228 @@ describe('suspense hydration', () => {
expect(listeners[5]).to.have.been.calledTwice;
});
});

it('should properly hydrate suspense when resolves to a Fragment without children', () => {
const originalHtml = ul([
li(0),
li(1),
'<!--$s-->',
'<!--/$s-->',
li(2),
li(3)
]);

const listeners = [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[2]}>2</ListItem>
<ListItem onClick={listeners[3]}>3</ListItem>
</Fragment>
</List>,
scratch
);
rerender(); // Flush rerender queue to mimic what preact will really do
expect(getLog()).to.deep.equal([]);
expect(scratch.innerHTML).to.equal(originalHtml);
expect(listeners[3]).not.to.have.been.called;

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

return resolve(() => null).then(() => {
rerender();
expect(scratch.innerHTML).to.equal(originalHtml);
expect(getLog()).to.deep.equal([]);
clearLog();

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

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

it('Should hydrate a fragment with multiple children correctly', () => {
scratch.innerHTML = '<!--$s--><div>Hello</div><div>World!</div><!--/$s-->';
clearLog();

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

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

clearLog();
});
});

it('Should hydrate a fragment with no children correctly', () => {
scratch.innerHTML = '<!--$s--><!--/$s--><div>Hello world</div>';
clearLog();

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

return resolve(() => null).then(() => {
rerender();
expect(scratch.innerHTML).to.equal(
'<!--$s--><!--/$s--><div>Hello world</div>'
);
expect(getLog()).to.deep.equal([]);

clearLog();
});
});

it('Should hydrate a fragment with no children correctly deeply', () => {
scratch.innerHTML =
'<!--$s--><!--$s--><!--/$s--><!--/$s--><div>Hello world</div>';
clearLog();

const [Lazy, resolve] = createLazy();
const [Lazy2, resolve2] = createLazy();
hydrate(
<>
<Suspense>
<Lazy>
<Suspense>
<Lazy2 />
</Suspense>
</Lazy>
</Suspense>
<div>Hello world</div>
</>,
scratch
);
rerender(); // Flush rerender queue to mimic what preact will really do
expect(scratch.innerHTML).to.equal(
'<!--$s--><!--$s--><!--/$s--><!--/$s--><div>Hello world</div>'
);
expect(getLog()).to.deep.equal([]);
clearLog();

return resolve(p => p.children).then(() => {
rerender();
expect(scratch.innerHTML).to.equal(
'<!--$s--><!--$s--><!--/$s--><!--/$s--><div>Hello world</div>'
);
expect(getLog()).to.deep.equal([]);

clearLog();
return resolve2(() => null).then(() => {
rerender();
expect(scratch.innerHTML).to.equal(
'<!--$s--><!--$s--><!--/$s--><!--/$s--><div>Hello world</div>'
);
expect(getLog()).to.deep.equal([]);

clearLog();
});
});
});

it('Should hydrate a fragment with multiple children correctly deeply', () => {
scratch.innerHTML =
'<!--$s--><!--$s--><p>I am</p><span>Fragment</span><!--/$s--><!--/$s--><div>Hello world</div>';
clearLog();

const [Lazy, resolve] = createLazy();
const [Lazy2, resolve2] = createLazy();
hydrate(
<>
<Suspense>
<Lazy>
<Suspense>
<Lazy2 />
</Suspense>
</Lazy>
</Suspense>
<div>Hello world</div>
</>,
scratch
);
rerender(); // Flush rerender queue to mimic what preact will really do
expect(scratch.innerHTML).to.equal(
'<!--$s--><!--$s--><p>I am</p><span>Fragment</span><!--/$s--><!--/$s--><div>Hello world</div>'
);
expect(getLog()).to.deep.equal([]);
clearLog();

return resolve(p => p.children).then(() => {
rerender();
expect(scratch.innerHTML).to.equal(
'<!--$s--><!--$s--><p>I am</p><span>Fragment</span><!--/$s--><!--/$s--><div>Hello world</div>'
);
expect(getLog()).to.deep.equal([]);

clearLog();
return resolve2(() => (
<>
<p>I am</p>
<span>Fragment</span>
</>
)).then(() => {
rerender();
expect(scratch.innerHTML).to.equal(
'<!--$s--><!--$s--><p>I am</p><span>Fragment</span><!--/$s--><!--/$s--><div>Hello world</div>'
);
expect(getLog()).to.deep.equal([]);

clearLog();
});
});
});
});
53 changes: 47 additions & 6 deletions src/diff/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ export function diff(
// If the previous diff bailed out, resume creating/hydrating.
if (oldVNode._flags & MODE_SUSPENDED) {
isHydrating = !!(oldVNode._flags & MODE_HYDRATE);
oldDom = newVNode._dom = oldVNode._dom;
excessDomChildren = [oldDom];
excessDomChildren = oldVNode._component._excess;
oldDom = newVNode._dom = oldVNode._dom = excessDomChildren[0];
}

if ((tmp = options._diff)) tmp(newVNode);
Expand Down Expand Up @@ -261,7 +261,7 @@ export function diff(
let renderResult = isTopLevelFragment ? tmp.props.children : tmp;

if (isTopLevelFragment) {
tmp.props.children = null;
tmp.props.children = NULL;
}

oldDom = diffChildren(
Expand Down Expand Up @@ -293,15 +293,56 @@ export function diff(
// if hydrating or creating initial tree, bailout preserves DOM:
if (isHydrating || excessDomChildren != NULL) {
if (e.then) {
let shouldFallback = true,
commentMarkersToFind = 0,
done = false;

newVNode._flags |= isHydrating
? MODE_HYDRATE | MODE_SUSPENDED
: MODE_SUSPENDED;

while (oldDom && oldDom.nodeType == 8 && oldDom.nextSibling) {
oldDom = oldDom.nextSibling;
newVNode._component._excess = [];
for (let i = 0; i < excessDomChildren.length; i++) {
let child = excessDomChildren[i];
if (child == NULL || done) continue;

// When we encounter a boundary with $s we are opening
// a boundary, this implies that we need to bump
// the amount of markers we need to find before closing
// the outer boundary.
// We exclude the open and closing marker from
// the future excessDomChildren but any nested one
// needs to be included for future suspensions.
if (child.nodeType == 8 && child.data == '$s') {
if (commentMarkersToFind > 0) {
newVNode._component._excess.push(child);
}
commentMarkersToFind++;
shouldFallback = false;
excessDomChildren[i] = NULL;
} else if (child.nodeType == 8 && child.data == '/$s') {
commentMarkersToFind--;
if (commentMarkersToFind > 0) {
newVNode._component._excess.push(child);
}
done = commentMarkersToFind === 0;
oldDom = excessDomChildren[i];
excessDomChildren[i] = NULL;
} else if (commentMarkersToFind > 0) {
newVNode._component._excess.push(child);
excessDomChildren[i] = NULL;
}
}

if (shouldFallback) {
while (oldDom && oldDom.nodeType == 8 && oldDom.nextSibling) {
oldDom = oldDom.nextSibling;
}

excessDomChildren[excessDomChildren.indexOf(oldDom)] = NULL;
newVNode._component._excess.push(oldDom);
}

excessDomChildren[excessDomChildren.indexOf(oldDom)] = NULL;
newVNode._dom = oldDom;
} else {
for (let i = excessDomChildren.length; i--; ) {
Expand Down
1 change: 1 addition & 0 deletions src/internal.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ export interface Component<P = {}, S = {}> extends preact.Component<P, S> {
constructor: ComponentType<P>;
state: S; // Override Component["state"] to not be readonly for internal use, specifically Hooks

_excess?: PreactElement[];
_dirty: boolean;
_force?: boolean;
_renderCallbacks: Array<() => void>; // Only class components
Expand Down

0 comments on commit 49bd318

Please sign in to comment.