Skip to content

Commit 552fd70

Browse files
JoviDeCroockJoviDeCroock
authored
Comment denoted hydration (#4636)
* Remove unused imports * Comment denoted hydration * Make it work * Golfies
1 parent 89308f0 commit 552fd70

File tree

3 files changed

+283
-61
lines changed

3 files changed

+283
-61
lines changed

‎compat/test/browser/suspense-hydration.test.js

+236-56
Original file line numberDiff line numberDiff line change
@@ -723,61 +723,17 @@ describe('suspense hydration', () => {
723723
});
724724
});
725725

726-
// Currently not supported. Hydration doesn't set attributes... but should it
727-
// when coming back from suspense if props were updated?
728-
it.skip('should hydrate and update attributes with latest props', () => {
729-
const originalHtml = '<p>Count: 0</p><p data-count="0">Lazy count: 0</p>';
730-
scratch.innerHTML = originalHtml;
731-
clearLog();
732-
733-
/** @type {() => void} */
734-
let increment;
735-
const [Lazy, resolve] = createLazy();
736-
function App() {
737-
const [count, setCount] = useState(0);
738-
increment = () => setCount(c => c + 1);
739-
740-
return (
741-
<Suspense>
742-
<p>Count: {count}</p>
743-
<Lazy count={count} />
744-
</Suspense>
745-
);
746-
}
747-
748-
hydrate(<App />, scratch);
749-
rerender(); // Flush rerender queue to mimic what preact will really do
750-
expect(scratch.innerHTML).to.equal(originalHtml);
751-
// Re: DOM OP below - Known issue with hydrating merged text nodes
752-
expect(getLog()).to.deep.equal(['<p>Count: .appendChild(#text)']);
753-
clearLog();
754-
755-
increment();
756-
rerender();
757-
758-
expect(scratch.innerHTML).to.equal(
759-
'<p>Count: 1</p><p data-count="0">Lazy count: 0</p>'
760-
);
761-
expect(getLog()).to.deep.equal([]);
762-
clearLog();
763-
764-
return resolve(({ count }) => (
765-
<p data-count={count}>Lazy count: {count}</p>
766-
)).then(() => {
767-
rerender();
768-
expect(scratch.innerHTML).to.equal(
769-
'<p>Count: 1</p><p data-count="1">Lazy count: 1</p>'
770-
);
771-
// Re: DOM OP below - Known issue with hydrating merged text nodes
772-
expect(getLog()).to.deep.equal(['<p>Lazy count: .appendChild(#text)']);
773-
clearLog();
774-
});
775-
});
776-
777-
// Currently not supported, but I wrote the test before I realized that so
778-
// leaving it here in case we do support it eventually
779-
it.skip('should properly hydrate suspense when resolves to a Fragment', () => {
780-
const originalHtml = ul([li(0), li(1), li(2), li(3), li(4), li(5)]);
726+
it('should properly hydrate suspense when resolves to a Fragment', () => {
727+
const originalHtml = ul([
728+
li(0),
729+
li(1),
730+
'<!--$s-->',
731+
li(2),
732+
li(3),
733+
'<!--/$s-->',
734+
li(4),
735+
li(5)
736+
]);
781737

782738
const listeners = [
783739
sinon.spy(),
@@ -809,8 +765,8 @@ describe('suspense hydration', () => {
809765
scratch
810766
);
811767
rerender(); // Flush rerender queue to mimic what preact will really do
812-
expect(scratch.innerHTML).to.equal(originalHtml);
813768
expect(getLog()).to.deep.equal([]);
769+
expect(scratch.innerHTML).to.equal(originalHtml);
814770
expect(listeners[5]).not.to.have.been.called;
815771

816772
clearLog();
@@ -839,4 +795,228 @@ describe('suspense hydration', () => {
839795
expect(listeners[5]).to.have.been.calledTwice;
840796
});
841797
});
798+
799+
it('should properly hydrate suspense when resolves to a Fragment without children', () => {
800+
const originalHtml = ul([
801+
li(0),
802+
li(1),
803+
'<!--$s-->',
804+
'<!--/$s-->',
805+
li(2),
806+
li(3)
807+
]);
808+
809+
const listeners = [sinon.spy(), sinon.spy(), sinon.spy(), sinon.spy()];
810+
811+
scratch.innerHTML = originalHtml;
812+
clearLog();
813+
814+
const [Lazy, resolve] = createLazy();
815+
hydrate(
816+
<List>
817+
<Fragment>
818+
<ListItem onClick={listeners[0]}>0</ListItem>
819+
<ListItem onClick={listeners[1]}>1</ListItem>
820+
</Fragment>
821+
<Suspense>
822+
<Lazy />
823+
</Suspense>
824+
<Fragment>
825+
<ListItem onClick={listeners[2]}>2</ListItem>
826+
<ListItem onClick={listeners[3]}>3</ListItem>
827+
</Fragment>
828+
</List>,
829+
scratch
830+
);
831+
rerender(); // Flush rerender queue to mimic what preact will really do
832+
expect(getLog()).to.deep.equal([]);
833+
expect(scratch.innerHTML).to.equal(originalHtml);
834+
expect(listeners[3]).not.to.have.been.called;
835+
836+
clearLog();
837+
scratch.querySelector('li:last-child').dispatchEvent(createEvent('click'));
838+
expect(listeners[3]).to.have.been.calledOnce;
839+
840+
return resolve(() => null).then(() => {
841+
rerender();
842+
expect(scratch.innerHTML).to.equal(originalHtml);
843+
expect(getLog()).to.deep.equal([]);
844+
clearLog();
845+
846+
scratch
847+
.querySelector('li:nth-child(2)')
848+
.dispatchEvent(createEvent('click'));
849+
expect(listeners[1]).to.have.been.calledOnce;
850+
851+
scratch
852+
.querySelector('li:last-child')
853+
.dispatchEvent(createEvent('click'));
854+
expect(listeners[3]).to.have.been.calledTwice;
855+
});
856+
});
857+
858+
it('Should hydrate a fragment with multiple children correctly', () => {
859+
scratch.innerHTML = '<!--$s--><div>Hello</div><div>World!</div><!--/$s-->';
860+
clearLog();
861+
862+
const [Lazy, resolve] = createLazy();
863+
hydrate(
864+
<Suspense>
865+
<Lazy />
866+
</Suspense>,
867+
scratch
868+
);
869+
rerender(); // Flush rerender queue to mimic what preact will really do
870+
expect(scratch.innerHTML).to.equal(
871+
'<!--$s--><div>Hello</div><div>World!</div><!--/$s-->'
872+
);
873+
expect(getLog()).to.deep.equal([]);
874+
clearLog();
875+
876+
return resolve(() => (
877+
<>
878+
<div>Hello</div>
879+
<div>World!</div>
880+
</>
881+
)).then(() => {
882+
rerender();
883+
expect(scratch.innerHTML).to.equal(
884+
'<!--$s--><div>Hello</div><div>World!</div><!--/$s-->'
885+
);
886+
expect(getLog()).to.deep.equal([]);
887+
888+
clearLog();
889+
});
890+
});
891+
892+
it('Should hydrate a fragment with no children correctly', () => {
893+
scratch.innerHTML = '<!--$s--><!--/$s--><div>Hello world</div>';
894+
clearLog();
895+
896+
const [Lazy, resolve] = createLazy();
897+
hydrate(
898+
<>
899+
<Suspense>
900+
<Lazy />
901+
</Suspense>
902+
<div>Hello world</div>
903+
</>,
904+
scratch
905+
);
906+
rerender(); // Flush rerender queue to mimic what preact will really do
907+
expect(scratch.innerHTML).to.equal(
908+
'<!--$s--><!--/$s--><div>Hello world</div>'
909+
);
910+
expect(getLog()).to.deep.equal([]);
911+
clearLog();
912+
913+
return resolve(() => null).then(() => {
914+
rerender();
915+
expect(scratch.innerHTML).to.equal(
916+
'<!--$s--><!--/$s--><div>Hello world</div>'
917+
);
918+
expect(getLog()).to.deep.equal([]);
919+
920+
clearLog();
921+
});
922+
});
923+
924+
it('Should hydrate a fragment with no children correctly deeply', () => {
925+
scratch.innerHTML =
926+
'<!--$s--><!--$s--><!--/$s--><!--/$s--><div>Hello world</div>';
927+
clearLog();
928+
929+
const [Lazy, resolve] = createLazy();
930+
const [Lazy2, resolve2] = createLazy();
931+
hydrate(
932+
<>
933+
<Suspense>
934+
<Lazy>
935+
<Suspense>
936+
<Lazy2 />
937+
</Suspense>
938+
</Lazy>
939+
</Suspense>
940+
<div>Hello world</div>
941+
</>,
942+
scratch
943+
);
944+
rerender(); // Flush rerender queue to mimic what preact will really do
945+
expect(scratch.innerHTML).to.equal(
946+
'<!--$s--><!--$s--><!--/$s--><!--/$s--><div>Hello world</div>'
947+
);
948+
expect(getLog()).to.deep.equal([]);
949+
clearLog();
950+
951+
return resolve(p => p.children).then(() => {
952+
rerender();
953+
expect(scratch.innerHTML).to.equal(
954+
'<!--$s--><!--$s--><!--/$s--><!--/$s--><div>Hello world</div>'
955+
);
956+
expect(getLog()).to.deep.equal([]);
957+
958+
clearLog();
959+
return resolve2(() => null).then(() => {
960+
rerender();
961+
expect(scratch.innerHTML).to.equal(
962+
'<!--$s--><!--$s--><!--/$s--><!--/$s--><div>Hello world</div>'
963+
);
964+
expect(getLog()).to.deep.equal([]);
965+
966+
clearLog();
967+
});
968+
});
969+
});
970+
971+
it('Should hydrate a fragment with multiple children correctly deeply', () => {
972+
scratch.innerHTML =
973+
'<!--$s--><!--$s--><p>I am</p><span>Fragment</span><!--/$s--><!--/$s--><div>Hello world</div>';
974+
clearLog();
975+
976+
const [Lazy, resolve] = createLazy();
977+
const [Lazy2, resolve2] = createLazy();
978+
hydrate(
979+
<>
980+
<Suspense>
981+
<Lazy>
982+
<Suspense>
983+
<Lazy2 />
984+
</Suspense>
985+
</Lazy>
986+
</Suspense>
987+
<div>Hello world</div>
988+
</>,
989+
scratch
990+
);
991+
rerender(); // Flush rerender queue to mimic what preact will really do
992+
expect(scratch.innerHTML).to.equal(
993+
'<!--$s--><!--$s--><p>I am</p><span>Fragment</span><!--/$s--><!--/$s--><div>Hello world</div>'
994+
);
995+
expect(getLog()).to.deep.equal([]);
996+
clearLog();
997+
998+
return resolve(p => p.children).then(() => {
999+
rerender();
1000+
expect(scratch.innerHTML).to.equal(
1001+
'<!--$s--><!--$s--><p>I am</p><span>Fragment</span><!--/$s--><!--/$s--><div>Hello world</div>'
1002+
);
1003+
expect(getLog()).to.deep.equal([]);
1004+
1005+
clearLog();
1006+
return resolve2(() => (
1007+
<>
1008+
<p>I am</p>
1009+
<span>Fragment</span>
1010+
</>
1011+
)).then(() => {
1012+
rerender();
1013+
expect(scratch.innerHTML).to.equal(
1014+
'<!--$s--><!--$s--><p>I am</p><span>Fragment</span><!--/$s--><!--/$s--><div>Hello world</div>'
1015+
);
1016+
expect(getLog()).to.deep.equal([]);
1017+
1018+
clearLog();
1019+
});
1020+
});
1021+
});
8421022
});

‎src/diff/index.js

+46-5
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,8 @@ export function diff(
6868
// If the previous diff bailed out, resume creating/hydrating.
6969
if (oldVNode._flags & MODE_SUSPENDED) {
7070
isHydrating = !!(oldVNode._flags & MODE_HYDRATE);
71-
oldDom = newVNode._dom = oldVNode._dom;
72-
excessDomChildren = [oldDom];
71+
excessDomChildren = oldVNode._component._excess;
72+
oldDom = newVNode._dom = oldVNode._dom = excessDomChildren[0];
7373
}
7474

7575
if ((tmp = options._diff)) tmp(newVNode);
@@ -288,15 +288,56 @@ export function diff(
288288
// if hydrating or creating initial tree, bailout preserves DOM:
289289
if (isHydrating || excessDomChildren != null) {
290290
if (e.then) {
291+
let shouldFallback = true,
292+
commentMarkersToFind = 0,
293+
done = false;
294+
291295
newVNode._flags |= isHydrating
292296
? MODE_HYDRATE | MODE_SUSPENDED
293297
: MODE_SUSPENDED;
294298

295-
while (oldDom && oldDom.nodeType == 8 && oldDom.nextSibling) {
296-
oldDom = oldDom.nextSibling;
299+
newVNode._component._excess = [];
300+
for (let i = 0; i < excessDomChildren.length; i++) {
301+
let child = excessDomChildren[i];
302+
if (child == null || done) continue;
303+
304+
// When we encounter a boundary with $s we are opening
305+
// a boundary, this implies that we need to bump
306+
// the amount of markers we need to find before closing
307+
// the outer boundary.
308+
// We exclude the open and closing marker from
309+
// the future excessDomChildren but any nested one
310+
// needs to be included for future suspensions.
311+
if (child.nodeType == 8 && child.data == '$s') {
312+
if (commentMarkersToFind > 0) {
313+
newVNode._component._excess.push(child);
314+
}
315+
commentMarkersToFind++;
316+
shouldFallback = false;
317+
excessDomChildren[i] = null;
318+
} else if (child.nodeType == 8 && child.data == '/$s') {
319+
commentMarkersToFind--;
320+
if (commentMarkersToFind > 0) {
321+
newVNode._component._excess.push(child);
322+
}
323+
done = commentMarkersToFind === 0;
324+
oldDom = excessDomChildren[i];
325+
excessDomChildren[i] = null;
326+
} else if (commentMarkersToFind > 0) {
327+
newVNode._component._excess.push(child);
328+
excessDomChildren[i] = null;
329+
}
330+
}
331+
332+
if (shouldFallback) {
333+
while (oldDom && oldDom.nodeType == 8 && oldDom.nextSibling) {
334+
oldDom = oldDom.nextSibling;
335+
}
336+
337+
excessDomChildren[excessDomChildren.indexOf(oldDom)] = null;
338+
newVNode._component._excess.push(oldDom);
297339
}
298340

299-
excessDomChildren[excessDomChildren.indexOf(oldDom)] = null;
300341
newVNode._dom = oldDom;
301342
} else {
302343
for (let i = excessDomChildren.length; i--; ) {

‎src/internal.d.ts

+1
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,7 @@ export interface Component<P = {}, S = {}> extends preact.Component<P, S> {
162162
constructor: ComponentType<P>;
163163
state: S; // Override Component["state"] to not be readonly for internal use, specifically Hooks
164164

165+
_excess?: PreactElement[];
165166
_dirty: boolean;
166167
_force?: boolean;
167168
_renderCallbacks: Array<() => void>; // Only class components

0 commit comments

Comments
 (0)