From 95192bf02e205616524f5619683ba4772891606d Mon Sep 17 00:00:00 2001 From: Jason Miller Date: Tue, 30 Mar 2021 12:05:29 -0400 Subject: [PATCH 1/4] Bugfix: fix route flashing for routes that render fragments --- packages/preact-iso/router.js | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/packages/preact-iso/router.js b/packages/preact-iso/router.js index d8707ab69..f8350b082 100644 --- a/packages/preact-iso/router.js +++ b/packages/preact-iso/router.js @@ -78,15 +78,18 @@ export function Router(props) { const prevChildren = useRef(); const pending = useRef(); - let reverse = false; if (url !== cur.current.url) { - reverse = true; pending.current = null; prev.current = cur.current; prevChildren.current = curChildren.current; // old uses the pending promise ref to know whether to render prevChildren.current.props.pending = pending; cur.current = loc; + + // Hi! Wondering what this horrid line is for? That's totally reasonable, it is gross. + // It prevents the old route from being remounted because it got shifted in the children Array. + // @ts-ignore-next + if (this.__v && this.__v.__k) this.__v.__k.reverse(); } curChildren.current = useMemo(() => { @@ -120,11 +123,7 @@ export function Router(props) { } else commit(); }, [url]); - // Hi! Wondering what this horrid line is for? That's totally reasonable, it is gross. - // It prevents the old route from being remounted because it got shifted in the children Array. - if (reverse && this.__v && this.__v.__k) this.__v.__k.reverse(); - - return [curChildren.current, prevChildren.current]; + return [prevChildren.current, curChildren.current]; } function Committer({ pending, children }) { From 2d0bb8a64783c32cfdaad05563b6293649059ef9 Mon Sep 17 00:00:00 2001 From: Jason Miller Date: Tue, 30 Mar 2021 12:22:33 -0400 Subject: [PATCH 2/4] Create odd-singers-explode.md --- .changeset/odd-singers-explode.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/odd-singers-explode.md diff --git a/.changeset/odd-singers-explode.md b/.changeset/odd-singers-explode.md new file mode 100644 index 000000000..a9c577fd1 --- /dev/null +++ b/.changeset/odd-singers-explode.md @@ -0,0 +1,5 @@ +--- +"preact-iso": patch +--- + +Bugfix: fix route flashing for routes that render fragments From 2e759c0d43239c5d6638222097f12298cc3b006f Mon Sep 17 00:00:00 2001 From: Jason Miller Date: Tue, 30 Mar 2021 12:25:08 -0400 Subject: [PATCH 3/4] fix duplicated preact dep in preact-iso router tests --- packages/preact-iso/test/router.test.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/preact-iso/test/router.test.js b/packages/preact-iso/test/router.test.js index a8b7ec446..b0caf7df0 100644 --- a/packages/preact-iso/test/router.test.js +++ b/packages/preact-iso/test/router.test.js @@ -1,5 +1,6 @@ import { jest, describe, it, beforeEach, expect } from '@jest/globals'; -import { h, html, render } from 'htm/preact'; +import { h, render } from 'preact'; +import { html } from 'htm/preact'; import { LocationProvider, Router, useLocation } from '../router.js'; import lazy, { ErrorBoundary } from '../lazy.js'; From c6daf81cf69d87c59794b1d9bc2023dd70614d9e Mon Sep 17 00:00:00 2001 From: Jason Miller Date: Tue, 30 Mar 2021 17:43:24 -0400 Subject: [PATCH 4/4] include fragments in route flickering test --- packages/preact-iso/test/router.test.js | 32 ++++++++++++++----------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/packages/preact-iso/test/router.test.js b/packages/preact-iso/test/router.test.js index b0caf7df0..a5b8210a1 100644 --- a/packages/preact-iso/test/router.test.js +++ b/packages/preact-iso/test/router.test.js @@ -109,9 +109,13 @@ describe('Router', () => { }); it('should wait for asynchronous routes', async () => { - const A = jest.fn(groggy(() => html`

A

`, 10)); - const B = jest.fn(groggy(() => html`

B

`, 10)); - const C = jest.fn(groggy(() => html`

C

`, 10)); + const route = name => html` +

${name}

+

hello

+ `; + const A = jest.fn(groggy(() => route('A'), 1)); + const B = jest.fn(groggy(() => route('B'), 1)); + const C = jest.fn(groggy(() => html`

C

`, 1)); let loc; render( html` @@ -135,28 +139,28 @@ describe('Router', () => { expect(A).toHaveBeenCalledWith({ path: '/', query: {} }, expect.anything()); A.mockClear(); - await sleep(20); + await sleep(10); - expect(scratch).toHaveProperty('innerHTML', '

A

'); + expect(scratch).toHaveProperty('innerHTML', '

A

hello

'); expect(A).toHaveBeenCalledWith({ path: '/', query: {} }, expect.anything()); A.mockClear(); loc.route('/b'); - expect(scratch).toHaveProperty('innerHTML', '

A

'); + expect(scratch).toHaveProperty('innerHTML', '

A

hello

'); expect(A).not.toHaveBeenCalled(); await sleep(1); - expect(scratch).toHaveProperty('innerHTML', '

A

'); + expect(scratch).toHaveProperty('innerHTML', '

A

hello

'); // We should never re-invoke while loading (that would be a remount of the old route): expect(A).not.toHaveBeenCalled(); expect(B).toHaveBeenCalledWith({ path: '/b', query: {} }, expect.anything()); B.mockClear(); - await sleep(20); + await sleep(10); - expect(scratch).toHaveProperty('innerHTML', '

B

'); + expect(scratch).toHaveProperty('innerHTML', '

B

hello

'); expect(A).not.toHaveBeenCalled(); expect(B).toHaveBeenCalledWith({ path: '/b', query: {} }, expect.anything()); @@ -165,7 +169,7 @@ describe('Router', () => { loc.route('/c?1'); loc.route('/c'); - expect(scratch).toHaveProperty('innerHTML', '

B

'); + expect(scratch).toHaveProperty('innerHTML', '

B

hello

'); expect(B).not.toHaveBeenCalled(); await sleep(1); @@ -174,13 +178,13 @@ describe('Router', () => { loc.route('/c?2'); loc.route('/c'); - expect(scratch).toHaveProperty('innerHTML', '

B

'); + expect(scratch).toHaveProperty('innerHTML', '

B

hello

'); // We should never re-invoke while loading (that would be a remount of the old route): expect(B).not.toHaveBeenCalled(); expect(C).toHaveBeenCalledWith({ path: '/c', query: {} }, expect.anything()); C.mockClear(); - await sleep(20); + await sleep(10); expect(scratch).toHaveProperty('innerHTML', '

C

'); expect(B).not.toHaveBeenCalled(); @@ -193,7 +197,7 @@ describe('Router', () => { loc.route('/b'); await sleep(1); - expect(scratch).toHaveProperty('innerHTML', '

B

'); + expect(scratch).toHaveProperty('innerHTML', '

B

hello

'); expect(C).not.toHaveBeenCalled(); // expect(B).toHaveBeenCalledTimes(1); expect(B).toHaveBeenCalledWith({ path: '/b', query: {} }, expect.anything()); @@ -202,7 +206,7 @@ describe('Router', () => { loc.route('/'); await sleep(1); - expect(scratch).toHaveProperty('innerHTML', '

A

'); + expect(scratch).toHaveProperty('innerHTML', '

A

hello

'); expect(B).not.toHaveBeenCalled(); // expect(A).toHaveBeenCalledTimes(1); expect(A).toHaveBeenCalledWith({ path: '/', query: {} }, expect.anything());