From 9634400d345aeea252ffe49bba692786c328ad14 Mon Sep 17 00:00:00 2001 From: Aaron Boodman Date: Tue, 7 Nov 2023 21:44:28 -1000 Subject: [PATCH 1/4] feat: Return default value whenever query function returns undefined. --- package-lock.json | 4 ++-- package.json | 2 +- src/index.test.tsx | 6 +++--- src/index.ts | 21 ++++++++++++--------- 4 files changed, 18 insertions(+), 15 deletions(-) diff --git a/package-lock.json b/package-lock.json index 5f12c77..fea9a45 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "replicache-react", - "version": "3.1.0", + "version": "4.0.0", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "replicache-react", - "version": "3.1.0", + "version": "4.0.0", "license": "ISC", "devDependencies": { "@esm-bundle/chai": "^4.3.4-fix.0", diff --git a/package.json b/package.json index 767ecd5..f01119c 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "replicache-react", - "version": "3.1.0", + "version": "4.0.0", "description": "Miscellaneous utilities for using Replicache with React", "homepage": "https://replicache.dev", "repository": "github:rocicorp/replicache-react", diff --git a/src/index.test.tsx b/src/index.test.tsx index b4e843c..12953e3 100644 --- a/src/index.test.tsx +++ b/src/index.test.tsx @@ -129,7 +129,7 @@ test('returning undefined', async () => { }, def, ); - return
{subResult === undefined ? 'undefined' : 'defined'}
; + return
{subResult}
; } const div = document.createElement('div'); @@ -141,10 +141,10 @@ test('returning undefined', async () => { }); render(, div); - expect(div.textContent).to.equal('defined'); + expect(div.textContent).to.equal('default'); await promise; await sleep(1); - expect(div.textContent).to.equal('undefined'); + expect(div.textContent).to.equal('default'); await rep.close(); }); diff --git a/src/index.ts b/src/index.ts index 5452efb..ab6e029 100644 --- a/src/index.ts +++ b/src/index.ts @@ -25,13 +25,13 @@ function doCallback() { }); } -export function useSubscribe( - r: Subscribable | null | undefined, - query: (tx: Tx) => Promise, - def: R, +export function useSubscribe( + r: Subscribable | null | undefined, + query: (tx: Tx) => Promise, + def: Default, deps: Array = [], -): R { - const [snapshot, setSnapshot] = useState(def); +) { + const [snapshot, setSnapshot] = useState(undefined); useEffect(() => { if (!r) { return; @@ -41,7 +41,7 @@ export function useSubscribe( onData: data => { // This is safe because we know that subscribe in fact can only return // `R` (the return type of query or def). - callbacks.push(() => setSnapshot(data as R)); + callbacks.push(() => setSnapshot(data as QueryRet)); if (!hasPendingCallback) { void Promise.resolve().then(doCallback); hasPendingCallback = true; @@ -51,8 +51,11 @@ export function useSubscribe( return () => { unsubscribe(); - setSnapshot(def); + setSnapshot(undefined); }; - }, [r, ...deps]); + }, [r, def, ...deps]); + if (snapshot === undefined) { + return def; + } return snapshot; } From f33d507084e36c0f5509540fce89a03f959d6a68 Mon Sep 17 00:00:00 2001 From: Aaron Boodman Date: Wed, 8 Nov 2023 17:09:45 -1000 Subject: [PATCH 2/4] Make test pass. --- src/index.test.tsx | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/index.test.tsx b/src/index.test.tsx index 12953e3..5501e1e 100644 --- a/src/index.test.tsx +++ b/src/index.test.tsx @@ -100,7 +100,18 @@ test('Batching of subscriptions', async () => { render(, div); await sleep(1); - expect(renderLog).to.deep.equal(['render A', null, 'render B', null, null]); + expect(renderLog).to.deep.equal([ + 'render A', + null, + 'render B', + null, + null, + 'render A', + null, + 'render B', + null, + null, + ]); expect(div.innerHTML).to.equal('
a:
b:
'); renderLog.length = 0; From b4abfaee013a5d922d3f5169db63f892034299bb Mon Sep 17 00:00:00 2001 From: Aaron Boodman Date: Thu, 9 Nov 2023 00:06:15 -1000 Subject: [PATCH 3/4] Remove duplicate render in test. OK so I still think the implementation of `useSubscribe` is fine. What was happening with the test was that `setState` *does* skip re-renders if the value is identical via Object.is(). When I changed the semantics of useSubscribe() I didn't change some of the test code to match. Previously the test code was manually coallescing undefined to null. Because the snapshot state inside useSubscribe now starts out as undefined rather than `def` this meant that when the query returned, it caused a re-render. --- src/index.test.tsx | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/src/index.test.tsx b/src/index.test.tsx index 5501e1e..a091495 100644 --- a/src/index.test.tsx +++ b/src/index.test.tsx @@ -76,7 +76,7 @@ test('Batching of subscriptions', async () => { const dataA = useSubscribe( rep, // TODO: Use type param to get when new Replicache is released. - async tx => ((await tx.get('a')) as string | undefined) ?? null, + async tx => (await tx.get('a')) as string | undefined, null, ); renderLog.push('render A', dataA); @@ -86,7 +86,7 @@ test('Batching of subscriptions', async () => { function B({rep, dataA}: {rep: MyRep; dataA: string | null}) { const dataB = useSubscribe( rep, - async tx => ((await tx.get('b')) as string | undefined) ?? null, + async tx => (await tx.get('b')) as string | undefined, null, ); renderLog.push('render B', dataA, dataB); @@ -100,18 +100,7 @@ test('Batching of subscriptions', async () => { render(, div); await sleep(1); - expect(renderLog).to.deep.equal([ - 'render A', - null, - 'render B', - null, - null, - 'render A', - null, - 'render B', - null, - null, - ]); + expect(renderLog).to.deep.equal(['render A', null, 'render B', null, null]); expect(div.innerHTML).to.equal('
a:
b:
'); renderLog.length = 0; From dd8bb6990a46f693eb5c3e82f823ebe19c080da2 Mon Sep 17 00:00:00 2001 From: Aaron Boodman Date: Sun, 12 Nov 2023 02:04:41 -1000 Subject: [PATCH 4/4] review feedback --- src/index.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/index.ts b/src/index.ts index ab6e029..7d14138 100644 --- a/src/index.ts +++ b/src/index.ts @@ -25,6 +25,8 @@ function doCallback() { }); } +export type RemoveUndefined = T extends undefined ? never : T; + export function useSubscribe( r: Subscribable | null | undefined, query: (tx: Tx) => Promise, @@ -57,5 +59,9 @@ export function useSubscribe( if (snapshot === undefined) { return def; } - return snapshot; + // This RemoveUndefined is just here to make the return type easier to read. + // It should be exactly equivalent to what the type would be without this. + // For some reason declaring the return type to be + // RemoveUndefined | Default doesn't typecheck. + return snapshot as RemoveUndefined; }