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

Comment denoted hydration #4636

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Comment denoted hydration #4636

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ec7276d
Comment denoted hydration
JoviDeCroock Jan 16, 2025
b564d3f
Make it work
JoviDeCroock Jan 18, 2025
4988f25
Golfies
JoviDeCroock Jan 18, 2025
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
292 changes: 236 additions & 56 deletions compat/test/browser/suspense-hydration.test.js
View file
Open in desktop
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();
});
});
});
});
9 changes: 9 additions & 0 deletions src/diff/children.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,15 @@ export function diffChildren(
oldDom = insert(childVNode, oldDom, parentDom);
} else if (typeof childVNode.type == 'function' && result !== UNDEFINED) {
oldDom = result;
let toResolve = 0;
while ((oldDom && oldDom.nodeType == 8) || toResolve > 0) {
if (oldDom && oldDom.nodeType == 8 && oldDom.data == '$s') {
toResolve++;
} else if (oldDom && oldDom.nodeType == 8 && oldDom.data == '/$s') {
toResolve--;
}
oldDom = oldDom.nextSibling;
}
} else if (newDom) {
oldDom = newDom.nextSibling;
}
Expand Down
51 changes: 46 additions & 5 deletions src/diff/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,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 @@ -290,15 +290,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
Loading
Loading