Skip to content

Commit e1523e1

Browse files
JoviDeCroockJoviDeCroock
committed
Comment denoted hydration (#4636)
* Remove unused imports * Comment denoted hydration * Make it work * Golfies
1 parent 8aceecd commit e1523e1

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
@@ -69,8 +69,8 @@ export function diff(
6969
// If the previous diff bailed out, resume creating/hydrating.
7070
if (oldVNode._flags & MODE_SUSPENDED) {
7171
isHydrating = !!(oldVNode._flags & MODE_HYDRATE);
72-
oldDom = newVNode._dom = oldVNode._dom;
73-
excessDomChildren = [oldDom];
72+
excessDomChildren = oldVNode._component._excess;
73+
oldDom = newVNode._dom = oldVNode._dom = excessDomChildren[0];
7474
}
7575

7676
if ((tmp = options._diff)) tmp(newVNode);
@@ -293,15 +293,56 @@ export function diff(
293293
// if hydrating or creating initial tree, bailout preserves DOM:
294294
if (isHydrating || excessDomChildren != NULL) {
295295
if (e.then) {
296+
let shouldFallback = true,
297+
commentMarkersToFind = 0,
298+
done = false;
299+
296300
newVNode._flags |= isHydrating
297301
? MODE_HYDRATE | MODE_SUSPENDED
298302
: MODE_SUSPENDED;
299303

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

304-
excessDomChildren[excessDomChildren.indexOf(oldDom)] = NULL;
305346
newVNode._dom = oldDom;
306347
} else {
307348
for (let i = excessDomChildren.length; i--; ) {

‎src/internal.d.ts

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

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

0 commit comments

Comments
 (0)