From 842cf4c8fab383e0618571b451f7e1621a99e365 Mon Sep 17 00:00:00 2001 From: Kevin Newman Date: Mon, 1 Feb 2021 20:04:53 -0500 Subject: [PATCH 1/8] Store data on refs in useTrackerWithDeps --- packages/react-meteor-data/package-lock.json | 36 ++++++++++---------- packages/react-meteor-data/package.json | 2 +- packages/react-meteor-data/useTracker.ts | 13 +++---- 3 files changed, 26 insertions(+), 25 deletions(-) diff --git a/packages/react-meteor-data/package-lock.json b/packages/react-meteor-data/package-lock.json index 86349087..58d550e6 100644 --- a/packages/react-meteor-data/package-lock.json +++ b/packages/react-meteor-data/package-lock.json @@ -54,9 +54,9 @@ } }, "@types/bson": { - "version": "4.0.2", - "resolved": "https://registry.npmjs.org/@types/bson/-/bson-4.0.2.tgz", - "integrity": "sha512-+uWmsejEHfmSjyyM/LkrP0orfE2m5Mx9Xel4tXNeqi1ldK5XMQcDsFkBmLDtuyKUbxj2jGDo0H240fbCRJZo7Q==", + "version": "4.0.3", + "resolved": "https://registry.npmjs.org/@types/bson/-/bson-4.0.3.tgz", + "integrity": "sha512-mVRvYnTOZJz3ccpxhr3wgxVmSeiYinW+zlzQz3SXWaJmD1DuL05Jeq7nKw3SnbKmbleW5qrLG5vdyWe/A9sXhw==", "requires": { "@types/node": "*" } @@ -67,9 +67,9 @@ "integrity": "sha512-rr+OQyAjxze7GgWrSaJwydHStIhHq2lvY3BOC2Mj7KnzI7XK0Uw1TOOdI9lDoajEbSWLiYgoo4f1R51erQfhPQ==" }, "@types/connect": { - "version": "3.4.33", - "resolved": "https://registry.npmjs.org/@types/connect/-/connect-3.4.33.tgz", - "integrity": "sha512-2+FrkXY4zllzTNfJth7jOqEHC+enpLeGslEhpnTAkg21GkRrWV4SsAtqchtT4YS9/nODBU2/ZfsBY2X4J/dX7A==", + "version": "3.4.34", + "resolved": "https://registry.npmjs.org/@types/connect/-/connect-3.4.34.tgz", + "integrity": "sha512-ePPA/JuI+X0vb+gSWlPKOY0NdNAie/rPUqX2GUPpbZwiKTkSPhjXWuee47E4MtE54QVzGCQMQkAL6JhV2E1+cQ==", "requires": { "@types/node": "*" } @@ -97,9 +97,9 @@ } }, "@types/meteor": { - "version": "1.4.42", - "resolved": "https://registry.npmjs.org/@types/meteor/-/meteor-1.4.42.tgz", - "integrity": "sha512-gaYksL5mAhvEAgGYZyz1HOYTSZx4Fg3RG9yA3qSGi0bFbvut3lbTVORU7Ry2Z29n//ghgt+grxVmCIXBPjfcLw==", + "version": "1.4.66", + "resolved": "https://registry.npmjs.org/@types/meteor/-/meteor-1.4.66.tgz", + "integrity": "sha512-gBKrGYqe3AFkP50eI2ELQa3Y7gjtjXEGEagUvcxTfhAf5LkHBzxm1FO9hBlVrJXyVYaoH5PDExaHMYgTwq+hcw==", "requires": { "@types/connect": "*", "@types/mongodb": "*", @@ -108,18 +108,18 @@ } }, "@types/mongodb": { - "version": "3.5.5", - "resolved": "https://registry.npmjs.org/@types/mongodb/-/mongodb-3.5.5.tgz", - "integrity": "sha512-iIWBu740IPolqzpYUBdQs1T4LMiJv1CIIcJb6gX2mGtb/VmnFWONSiyUG+lJF4IUg+QEuIqpalW3eshiTCc8dQ==", + "version": "3.6.3", + "resolved": "https://registry.npmjs.org/@types/mongodb/-/mongodb-3.6.3.tgz", + "integrity": "sha512-6YNqGP1hk5bjUFaim+QoFFuI61WjHiHE1BNeB41TA00Xd2K7zG4lcWyLLq/XtIp36uMavvS5hoAUJ+1u/GcX2Q==", "requires": { "@types/bson": "*", "@types/node": "*" } }, "@types/node": { - "version": "13.11.1", - "resolved": "https://registry.npmjs.org/@types/node/-/node-13.11.1.tgz", - "integrity": "sha512-eWQGP3qtxwL8FGneRrC5DwrJLGN4/dH1clNTuLfN81HCrxVtxRjygDTUoZJ5ASlDEeo0ppYFQjQIlXhtXpOn6g==" + "version": "14.14.22", + "resolved": "https://registry.npmjs.org/@types/node/-/node-14.14.22.tgz", + "integrity": "sha512-g+f/qj/cNcqKkc3tFqlXOYjrmZA+jNBiDzbP3kH+B+otKFqAdPgVTGP1IeKRdMml/aE69as5S4FqtxAbl+LaMw==" }, "@types/prop-types": { "version": "15.7.3", @@ -162,9 +162,9 @@ } }, "@types/underscore": { - "version": "1.9.4", - "resolved": "https://registry.npmjs.org/@types/underscore/-/underscore-1.9.4.tgz", - "integrity": "sha512-CjHWEMECc2/UxOZh0kpiz3lEyX2Px3rQS9HzD20lxMvx571ivOBQKeLnqEjxUY0BMgp6WJWo/pQLRBwMW5v4WQ==" + "version": "1.10.24", + "resolved": "https://registry.npmjs.org/@types/underscore/-/underscore-1.10.24.tgz", + "integrity": "sha512-T3NQD8hXNW2sRsSbLNjF/aBo18MyJlbw0lSpQHB/eZZtScPdexN4HSa8cByYwTw9Wy7KuOFr81mlDQcQQaZ79w==" }, "@types/yargs": { "version": "15.0.4", diff --git a/packages/react-meteor-data/package.json b/packages/react-meteor-data/package.json index f2ad7573..7b2c0db3 100644 --- a/packages/react-meteor-data/package.json +++ b/packages/react-meteor-data/package.json @@ -5,7 +5,7 @@ "react-dom": "16.13.1", "react-test-renderer": "16.13.1", "@testing-library/react": "^10.0.2", - "@types/meteor": "^1.4.42", + "@types/meteor": "^1.4.66", "@types/react": "^16.9.34" } } diff --git a/packages/react-meteor-data/useTracker.ts b/packages/react-meteor-data/useTracker.ts index 01063193..b7dd121a 100644 --- a/packages/react-meteor-data/useTracker.ts +++ b/packages/react-meteor-data/useTracker.ts @@ -104,23 +104,24 @@ const useTrackerNoDeps = (reactiveFn: IReactiveFn) => { } const useTrackerWithDeps = (reactiveFn: IReactiveFn, deps: DependencyList): T => { - let [data, setData] = useState(); + const [data, setData] = useState(); - const { current: refs } = useRef({ reactiveFn }); + const { current: refs } = useRef({ reactiveFn, data }); refs.reactiveFn = reactiveFn; + refs.data = data; useMemo(() => { // To jive with the lifecycle interplay between Tracker/Subscribe, run the // reactive function in a computation, then stop it, to force flush cycle. const comp = Tracker.nonreactive( () => Tracker.autorun((c: Tracker.Computation) => { - if (c.firstRun) data = refs.reactiveFn(); + if (c.firstRun) refs.data = refs.reactiveFn(); }) ); // To avoid creating side effects in render, stop the computation immediately Meteor.defer(() => { comp.stop() }); if (Meteor.isDevelopment) { - checkCursor(data); + checkCursor(refs.data); } }, deps); @@ -135,8 +136,8 @@ const useTrackerWithDeps = (reactiveFn: IReactiveFn, deps: Dependenc } }, deps); - return data as T; -} + return refs.data as T; +}; const useTrackerClient = (reactiveFn: IReactiveFn, deps: DependencyList = null): T => (deps === null || deps === undefined || !Array.isArray(deps)) From 476acf7380a59c4d3a82cea2c27672d34b160b4f Mon Sep 17 00:00:00 2001 From: Kevin Newman Date: Mon, 1 Feb 2021 23:33:15 -0500 Subject: [PATCH 2/8] Add comparator option to useTracker --- .../react-meteor-data/useTracker.tests.js | 35 ++++++++ packages/react-meteor-data/useTracker.ts | 81 ++++++++++++------- packages/react-meteor-data/withTracker.tsx | 13 +-- 3 files changed, 97 insertions(+), 32 deletions(-) diff --git a/packages/react-meteor-data/useTracker.tests.js b/packages/react-meteor-data/useTracker.tests.js index b473e923..a1e37a21 100644 --- a/packages/react-meteor-data/useTracker.tests.js +++ b/packages/react-meteor-data/useTracker.tests.js @@ -220,6 +220,41 @@ if (Meteor.isClient) { completed(); }); + Tinytest.addAsync('useTracker - basic track', async function (test, completed) { + var container = document.createElement("DIV"); + + var x = new ReactiveVar('aaa'); + + var Foo = () => { + const data = useTracker(() => { + return { + x: x.get() + }; + }, []); + return {data.x}; + }; + + ReactDOM.render(, container); + test.equal(getInnerHtml(container), 'aaa'); + + x.set('bbb'); + await waitFor(() => { + Tracker.flush({_throwFirstError: true}); + }, { container, timeout: 250 }); + + test.equal(getInnerHtml(container), 'bbb'); + + test.equal(x._numListeners(), 1); + + await waitFor(() => { + ReactDOM.unmountComponentAtNode(container); + }, { container, timeout: 250 }); + + test.equal(x._numListeners(), 0); + + completed(); + }); + // Make sure that calling ReactDOM.render() from an autorun doesn't // associate that autorun with the mixin's autorun. When autoruns are // nested, invalidating the outer one stops the inner one, unless diff --git a/packages/react-meteor-data/useTracker.ts b/packages/react-meteor-data/useTracker.ts index b7dd121a..0684d216 100644 --- a/packages/react-meteor-data/useTracker.ts +++ b/packages/react-meteor-data/useTracker.ts @@ -31,16 +31,21 @@ function checkCursor (data: any): void { const fur = (x: number): number => x + 1; const useForceUpdate = () => useReducer(fur, 0)[1]; -interface IReactiveFn { +export interface IReactiveFn { (c?: Tracker.Computation): T } + +export interface ISkipUpdate { + (prev: T, next: T): boolean +} + type TrackerRefs = { computation?: Tracker.Computation; isMounted: boolean; trackerData: any; } -const useTrackerNoDeps = (reactiveFn: IReactiveFn) => { +const useTrackerNoDeps = (reactiveFn: IReactiveFn, skipUpdate: ISkipUpdate = null) => { const { current: refs } = useRef({ isMounted: false, trackerData: null @@ -68,7 +73,7 @@ const useTrackerNoDeps = (reactiveFn: IReactiveFn) => { checkCursor(data); } refs.trackerData = data; - } else { + } else if (!skipUpdate || !skipUpdate(refs.trackerData, reactiveFn(c))) { // For any reactive change, forceUpdate and let the next render rebuild the computation. forceUpdate(); } @@ -87,7 +92,7 @@ const useTrackerNoDeps = (reactiveFn: IReactiveFn) => { } useEffect(() => { - // Let subsequent renders know we are mounted (render is comitted). + // Let subsequent renders know we are mounted (render is committed). refs.isMounted = true; // Render is committed. Since useTracker without deps always runs synchronously, @@ -103,9 +108,8 @@ const useTrackerNoDeps = (reactiveFn: IReactiveFn) => { return refs.trackerData; } -const useTrackerWithDeps = (reactiveFn: IReactiveFn, deps: DependencyList): T => { +const useTrackerWithDeps = (reactiveFn: IReactiveFn, deps: DependencyList, skipUpdate: ISkipUpdate = null): T => { const [data, setData] = useState(); - const { current: refs } = useRef({ reactiveFn, data }); refs.reactiveFn = reactiveFn; refs.data = data; @@ -126,26 +130,38 @@ const useTrackerWithDeps = (reactiveFn: IReactiveFn, deps: Dependenc }, deps); useEffect(() => { - const computation = Tracker.nonreactive( + const computation = Tracker.nonreactive( () => Tracker.autorun((c) => { - setData(refs.reactiveFn(c)); + const data: T = refs.reactiveFn(c); + if (!skipUpdate || !skipUpdate(refs.data, data)) { + setData(data); + } }) ); return () => { computation.stop(); - } + }; }, deps); return refs.data as T; }; -const useTrackerClient = (reactiveFn: IReactiveFn, deps: DependencyList = null): T => - (deps === null || deps === undefined || !Array.isArray(deps)) - ? useTrackerNoDeps(reactiveFn) - : useTrackerWithDeps(reactiveFn, deps); +function useTrackerClient (reactiveFn: IReactiveFn, skipUpdate?: ISkipUpdate): T; +function useTrackerClient (reactiveFn: IReactiveFn, deps?: DependencyList, skipUpdate?: ISkipUpdate): T; +function useTrackerClient (reactiveFn: IReactiveFn, deps: DependencyList | ISkipUpdate = null, skipUpdate: ISkipUpdate = null): T { + if (deps === null || deps === undefined || !Array.isArray(deps)) { + if (typeof deps === "function") { + skipUpdate = deps; + } + return useTrackerNoDeps(reactiveFn, skipUpdate); + } else { + return useTrackerWithDeps(reactiveFn, deps, skipUpdate); + } +} -const useTrackerServer = (reactiveFn: IReactiveFn, deps: DependencyList): T => - Tracker.nonreactive(reactiveFn) as T; +const useTrackerServer: typeof useTrackerClient = (reactiveFn) => { + return Tracker.nonreactive(reactiveFn); +} // When rendering on the server, we don't want to use the Tracker. // We only do the first rendering on the server so we can get the data right away @@ -153,22 +169,33 @@ const useTracker = Meteor.isServer ? useTrackerServer : useTrackerClient; -const useTrackerDev = (reactiveFn: IReactiveFn, deps: DependencyList): T => { - if (typeof reactiveFn !== 'function') { +function useTrackerDev (reactiveFn, deps = null, skipUpdate = null) { + function warn (expects: string, pos: string, arg: string, type: string) { console.warn( - 'Warning: useTracker expected a function in it\'s first argument ' - + `(reactiveFn), but got type of ${typeof reactiveFn}.` + `Warning: useTracker expected a ${expects} in it\'s ${pos} argument ` + + `(${arg}), but got type of \`${type}\`.` ); } - if (deps && !Array.isArray(deps)) { - console.warn( - 'Warning: useTracker expected an array in it\'s second argument ' - + `(dependency), but got type of ${typeof deps}.` - ); + + if (typeof reactiveFn !== 'function') { + warn("function", "1st", "reactiveFn", reactiveFn); } - return useTracker(reactiveFn, deps); -} + + if (deps && skipUpdate && !Array.isArray(deps) && typeof skipUpdate === "function") { + warn("array & function", "2nd and 3rd", "deps, skipUpdate", + `${typeof deps} & ${typeof skipUpdate}`); + } else { + if (deps && !Array.isArray(deps) && typeof deps !== "function") { + warn("array or function", "2nd", "deps or skipUpdate", typeof deps); + } + if (skipUpdate && typeof skipUpdate === "function") { + warn("function", "3rd", "skipUpdate", typeof skipUpdate); + } + } + + return useTracker(reactiveFn, deps, skipUpdate); +}; export default Meteor.isDevelopment - ? useTrackerDev + ? useTrackerDev as typeof useTrackerClient : useTracker; diff --git a/packages/react-meteor-data/withTracker.tsx b/packages/react-meteor-data/withTracker.tsx index 94862f1c..d8531fdc 100644 --- a/packages/react-meteor-data/withTracker.tsx +++ b/packages/react-meteor-data/withTracker.tsx @@ -5,6 +5,7 @@ type ReactiveFn = (props: object) => any; type ReactiveOptions = { getMeteorData: ReactiveFn; pure?: boolean; + skipUpdate?: (prev: any, next: any) => boolean; } export default function withTracker(options: ReactiveFn | ReactiveOptions) { @@ -14,14 +15,16 @@ export default function withTracker(options: ReactiveFn | ReactiveOptions) { : options.getMeteorData; const WithTracker = forwardRef((props, ref) => { - const data = useTracker(() => getMeteorData(props) || {}); + const data = useTracker( + () => getMeteorData(props) || {}, + (options as ReactiveOptions).skipUpdate + ); return ( ); }); - // @ts-ignore - const { pure = true } = options; + const { pure = true } = options as ReactiveOptions; return pure ? memo(WithTracker) : WithTracker; - } -}; + }; +} From e30e2905f8bcfca095f5929ef9888b061b39584a Mon Sep 17 00:00:00 2001 From: Kevin Newman Date: Mon, 1 Feb 2021 23:34:06 -0500 Subject: [PATCH 3/8] Update @types/meteor and add fast-deep-equal --- packages/react-meteor-data/package-lock.json | 5 +++++ packages/react-meteor-data/package.json | 9 +++++---- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/react-meteor-data/package-lock.json b/packages/react-meteor-data/package-lock.json index 58d550e6..bf514a1c 100644 --- a/packages/react-meteor-data/package-lock.json +++ b/packages/react-meteor-data/package-lock.json @@ -239,6 +239,11 @@ "resolved": "https://registry.npmjs.org/dom-accessibility-api/-/dom-accessibility-api-0.4.3.tgz", "integrity": "sha512-JZ8iPuEHDQzq6q0k7PKMGbrIdsgBB7TRrtVOUm4nSMCExlg5qQG4KXWTH2k90yggjM4tTumRGwTKJSldMzKyLA==" }, + "fast-deep-equal": { + "version": "3.1.3", + "resolved": "https://registry.npmjs.org/fast-deep-equal/-/fast-deep-equal-3.1.3.tgz", + "integrity": "sha512-f3qQ9oQy9j2AhBe/H9VC91wLmKBCCU/gDOnKNAYG5hswO7BLKj09Hc5HYNz9cGI++xlpDCIgDaitVs03ATR84Q==" + }, "has-flag": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/has-flag/-/has-flag-4.0.0.tgz", diff --git a/packages/react-meteor-data/package.json b/packages/react-meteor-data/package.json index 7b2c0db3..ff4701c6 100644 --- a/packages/react-meteor-data/package.json +++ b/packages/react-meteor-data/package.json @@ -1,11 +1,12 @@ { "name": "react-meteor-data", "dependencies": { - "react": "16.13.1", - "react-dom": "16.13.1", - "react-test-renderer": "16.13.1", "@testing-library/react": "^10.0.2", "@types/meteor": "^1.4.66", - "@types/react": "^16.9.34" + "@types/react": "^16.9.34", + "fast-deep-equal": "^3.1.3", + "react": "16.13.1", + "react-dom": "16.13.1", + "react-test-renderer": "16.13.1" } } From fa875bd8e57bd4bcf48b691dff8d5e7e3f32dce0 Mon Sep 17 00:00:00 2001 From: Kevin Newman Date: Thu, 29 Apr 2021 00:23:15 -0400 Subject: [PATCH 4/8] Fix immediate rerender causes undefined (and add tests) --- .../react-meteor-data/useTracker.tests.js | 42 +++++++++++++++++++ packages/react-meteor-data/useTracker.ts | 10 ++++- 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/packages/react-meteor-data/useTracker.tests.js b/packages/react-meteor-data/useTracker.tests.js index a1e37a21..d9af64af 100644 --- a/packages/react-meteor-data/useTracker.tests.js +++ b/packages/react-meteor-data/useTracker.tests.js @@ -96,6 +96,48 @@ if (Meteor.isClient) { completed(); }); + Tinytest.addAsync('useTracker - immediate rerender does not result in `undefined`', async function (test, completed) { + /** + * In cases where a state change causes rerender before the render is + * committed, useMemo will only run on the first render. This can cause the + * value to get lost (unexpected undefined), if we aren't careful. + */ + const container = document.createElement("DIV"); + const reactiveDict = new ReactiveDict(); + let value; + let renders = 0; + const Test = () => { + renders++; + const [num, setNum] = useState(0); + value = useTracker(() => { + reactiveDict.setDefault('key', 'initial'); + return reactiveDict.get('key'); + }, []); + if (num === 0) { + setNum(1); + } + return {value}; + }; + + let rerender, unmount; + const TestContainer = () => { + [, rerender] = useReducer((x) => x + 1, 0); + const [mounted, setMounted] = useState(true); + unmount = () => setMounted(false); + return {mounted ? : null}; + }; + + ReactDOM.render(, container); + test.equal(value, 'initial', 'values should be "initial" and not undefined'); + + // wait for useEffect + await waitFor(() => {}, { container, timeout: 250 }); + + test.equal(value, 'initial', 'values should still be "initial" after mount'); + + completed(); + }); + const depsTester = async (test, mode = 'normal') => { const container = document.createElement("DIV"); diff --git a/packages/react-meteor-data/useTracker.ts b/packages/react-meteor-data/useTracker.ts index 0684d216..e498ee2a 100644 --- a/packages/react-meteor-data/useTracker.ts +++ b/packages/react-meteor-data/useTracker.ts @@ -110,9 +110,14 @@ const useTrackerNoDeps = (reactiveFn: IReactiveFn, skipUpdate: ISkip const useTrackerWithDeps = (reactiveFn: IReactiveFn, deps: DependencyList, skipUpdate: ISkipUpdate = null): T => { const [data, setData] = useState(); - const { current: refs } = useRef({ reactiveFn, data }); + const { current: refs } = useRef({ reactiveFn, data, isMounted: false }); refs.reactiveFn = reactiveFn; - refs.data = data; + + // Only update refs.data when deps change, or the component is mounted. + // This prevents an unexpected value of `undefined` on immediate rerenders. + if (refs.isMounted) { + refs.data = data; + } useMemo(() => { // To jive with the lifecycle interplay between Tracker/Subscribe, run the @@ -130,6 +135,7 @@ const useTrackerWithDeps = (reactiveFn: IReactiveFn, deps: Dependenc }, deps); useEffect(() => { + refs.isMounted = true; const computation = Tracker.nonreactive( () => Tracker.autorun((c) => { const data: T = refs.reactiveFn(c); From 53b585f022e19a38661f82c0912d4d4a6d62a2ce Mon Sep 17 00:00:00 2001 From: Kevin Newman Date: Thu, 29 Apr 2021 01:33:24 -0400 Subject: [PATCH 5/8] Add tests for skipUpdate (and fix errors) --- .../react-meteor-data/useTracker.tests.js | 137 ++++++++++++------ packages/react-meteor-data/useTracker.ts | 14 +- .../react-meteor-data/withTracker.tests.js | 49 +++++++ 3 files changed, 156 insertions(+), 44 deletions(-) diff --git a/packages/react-meteor-data/useTracker.tests.js b/packages/react-meteor-data/useTracker.tests.js index d9af64af..9ba4b6bd 100644 --- a/packages/react-meteor-data/useTracker.tests.js +++ b/packages/react-meteor-data/useTracker.tests.js @@ -96,48 +96,6 @@ if (Meteor.isClient) { completed(); }); - Tinytest.addAsync('useTracker - immediate rerender does not result in `undefined`', async function (test, completed) { - /** - * In cases where a state change causes rerender before the render is - * committed, useMemo will only run on the first render. This can cause the - * value to get lost (unexpected undefined), if we aren't careful. - */ - const container = document.createElement("DIV"); - const reactiveDict = new ReactiveDict(); - let value; - let renders = 0; - const Test = () => { - renders++; - const [num, setNum] = useState(0); - value = useTracker(() => { - reactiveDict.setDefault('key', 'initial'); - return reactiveDict.get('key'); - }, []); - if (num === 0) { - setNum(1); - } - return {value}; - }; - - let rerender, unmount; - const TestContainer = () => { - [, rerender] = useReducer((x) => x + 1, 0); - const [mounted, setMounted] = useState(true); - unmount = () => setMounted(false); - return {mounted ? : null}; - }; - - ReactDOM.render(, container); - test.equal(value, 'initial', 'values should be "initial" and not undefined'); - - // wait for useEffect - await waitFor(() => {}, { container, timeout: 250 }); - - test.equal(value, 'initial', 'values should still be "initial" after mount'); - - completed(); - }); - const depsTester = async (test, mode = 'normal') => { const container = document.createElement("DIV"); @@ -227,6 +185,61 @@ if (Meteor.isClient) { completed(); }); + async function testSkipUpdate (test, deps) { + /** + * In cases where a state change causes rerender before the render is + * committed, useMemo will only run on the first render. This can cause the + * value to get lost (unexpected undefined), if we aren't careful. + */ + const container = document.createElement("DIV"); + const reactiveDict = new ReactiveDict(); + let value; + let renders = 0; + const skipUpdate = (prev, next) => { + // only update when second changes, not first + return prev.second === next.second; + }; + const Test = () => { + renders++; + value = useTracker( + () => { + reactiveDict.setDefault('key', { first: 0, second: 0 }); + return reactiveDict.get('key'); + }, + deps || skipUpdate, + deps ? skipUpdate : undefined + ); + return {JSON.stringify(value)}; + }; + + ReactDOM.render(, container); + test.equal(renders, 1, 'Should have rendered only once'); + + // wait for useEffect + await waitFor(() => {}, { container, timeout: 250 }); + test.equal(renders, 1, 'Should have rendered only once after mount'); + + reactiveDict.set('key', { first: 1, second: 0 }); + await waitFor(() => {}, { container, timeout: 250 }); + + test.equal(renders, 1, "Should still have rendered only once"); + + reactiveDict.set('key', { first: 1, second: 1 }); + await waitFor(() => {}, { container, timeout: 250 }); + + test.equal(renders, 2, "Should have rendered a second time"); + } + + Tinytest.addAsync('useTracker - skipUpdate prevents rerenders', async function (test, completed) { + await testSkipUpdate(test, []); + completed(); + }); + + Tinytest.addAsync('useTracker (no deps) - skipUpdate prevents rerenders', async function (test, completed) { + await testSkipUpdate(test); + completed(); + }); + Tinytest.addAsync('useTracker (no deps) - basic track', async function (test, completed) { var container = document.createElement("DIV"); @@ -721,6 +734,46 @@ if (Meteor.isClient) { } ]); + Tinytest.addAsync('useTracker - immediate rerender does not result in `undefined`', async function (test, completed) { + /** + * In cases where a state change causes rerender before the render is + * committed, useMemo will only run on the first render. This can cause the + * value to get lost (unexpected undefined), if we aren't careful. + */ + const container = document.createElement("DIV"); + const reactiveDict = new ReactiveDict(); + let value; + const Test = () => { + const [num, setNum] = useState(0); + value = useTracker(() => { + reactiveDict.setDefault('key', 'initial'); + return reactiveDict.get('key'); + }, []); + if (num === 0) { + setNum(1); + } + return {value}; + }; + + let rerender, unmount; + const TestContainer = () => { + [, rerender] = useReducer((x) => x + 1, 0); + const [mounted, setMounted] = useState(true); + unmount = () => setMounted(false); + return {mounted ? : null}; + }; + + ReactDOM.render(, container); + test.equal(value, 'initial', 'values should be "initial" and not undefined'); + + // wait for useEffect + await waitFor(() => {}, { container, timeout: 250 }); + + test.equal(value, 'initial', 'values should still be "initial" after mount'); + + completed(); + }); + // Tinytest.add( // "useTracker - print warning if return cursor from useTracker", // function (test) { diff --git a/packages/react-meteor-data/useTracker.ts b/packages/react-meteor-data/useTracker.ts index e498ee2a..ffa93174 100644 --- a/packages/react-meteor-data/useTracker.ts +++ b/packages/react-meteor-data/useTracker.ts @@ -97,7 +97,17 @@ const useTrackerNoDeps = (reactiveFn: IReactiveFn, skipUpdate: ISkip // Render is committed. Since useTracker without deps always runs synchronously, // forceUpdate and let the next render recreate the computation. - forceUpdate(); + if (!skipUpdate) { + forceUpdate(); + } else { + Tracker.nonreactive(() => Tracker.autorun((c: Tracker.Computation) => { + refs.computation = c; + if (!skipUpdate(refs.trackerData, reactiveFn(c))) { + // For any reactive change, forceUpdate and let the next render rebuild the computation. + forceUpdate(); + } + })); + } // stop the computation on unmount return () =>{ @@ -194,7 +204,7 @@ function useTrackerDev (reactiveFn, deps = null, skipUpdate = null) { if (deps && !Array.isArray(deps) && typeof deps !== "function") { warn("array or function", "2nd", "deps or skipUpdate", typeof deps); } - if (skipUpdate && typeof skipUpdate === "function") { + if (skipUpdate && typeof skipUpdate !== "function") { warn("function", "3rd", "skipUpdate", typeof skipUpdate); } } diff --git a/packages/react-meteor-data/withTracker.tests.js b/packages/react-meteor-data/withTracker.tests.js index 31f666bd..faa3f156 100644 --- a/packages/react-meteor-data/withTracker.tests.js +++ b/packages/react-meteor-data/withTracker.tests.js @@ -12,6 +12,55 @@ const getInnerHtml = function (elem) { }; if (Meteor.isClient) { + Tinytest.addAsync('withTracker - skipUpdate prevents rerenders', async function (test, completed) { + /** + * In cases where a state change causes rerender before the render is + * committed, useMemo will only run on the first render. This can cause the + * value to get lost (unexpected undefined), if we aren't careful. + */ + const container = document.createElement("DIV"); + const reactiveDict = new ReactiveDict(); + let value; + let renders = 0; + const skipUpdate = (prev, next) => { + // only update when second changes, not first + return prev.value.second === next.value.second; + }; + const Test = withTracker({ + pure: true, + getMeteorData: () => { + reactiveDict.setDefault('key', { first: 0, second: 0 }); + return { + value: reactiveDict.get('key') + }; + }, + skipUpdate: skipUpdate, + })((props) => { + console.log(props) + renders++; + return {JSON.stringify(props.value)}; + }); + + ReactDOM.render(, container); + test.equal(renders, 1, 'Should have rendered only once'); + + // wait for useEffect + await waitFor(() => {}, { container, timeout: 250 }); + test.equal(renders, 1, 'Should have rendered only once after mount'); + + reactiveDict.set('key', { first: 1, second: 0 }); + await waitFor(() => {}, { container, timeout: 250 }); + + test.equal(renders, 1, "Should still have rendered only once"); + + reactiveDict.set('key', { first: 1, second: 1 }); + await waitFor(() => {}, { container, timeout: 250 }); + + test.equal(renders, 2, "Should have rendered a second time"); + + completed(); + }); + Tinytest.addAsync('withTracker - basic track', async function (test, completed) { var container = document.createElement("DIV"); From 854cba3555e27a0bb939cd8f230b19f2a1269d49 Mon Sep 17 00:00:00 2001 From: Kevin Newman Date: Thu, 29 Apr 2021 12:08:04 -0400 Subject: [PATCH 6/8] Simplify useTrackerWithDeps and add additional tests --- .../react-meteor-data/useTracker.tests.js | 33 ++++++++++++++----- packages/react-meteor-data/useTracker.ts | 22 ++++++------- 2 files changed, 36 insertions(+), 19 deletions(-) diff --git a/packages/react-meteor-data/useTracker.tests.js b/packages/react-meteor-data/useTracker.tests.js index 9ba4b6bd..128e403f 100644 --- a/packages/react-meteor-data/useTracker.tests.js +++ b/packages/react-meteor-data/useTracker.tests.js @@ -743,33 +743,50 @@ if (Meteor.isClient) { const container = document.createElement("DIV"); const reactiveDict = new ReactiveDict(); let value; - const Test = () => { + let renderCount = 0; + const Test = ({ afterMountInc = false }) => { + renderCount++; const [num, setNum] = useState(0); value = useTracker(() => { reactiveDict.setDefault('key', 'initial'); return reactiveDict.get('key'); }, []); if (num === 0) { + reactiveDict.set('key', 'secondary'); setNum(1); } + if (afterMountInc && num !== 2) { + reactiveDict.set('key', 'third'); + setNum(2); + } return {value}; }; - let rerender, unmount; + const strict = 2; + let afterMountInc, setAfterMountInc; const TestContainer = () => { - [, rerender] = useReducer((x) => x + 1, 0); - const [mounted, setMounted] = useState(true); - unmount = () => setMounted(false); - return {mounted ? : null}; + [afterMountInc, setAfterMountInc] = useState(false); + return ; }; ReactDOM.render(, container); - test.equal(value, 'initial', 'values should be "initial" and not undefined'); + test.equal(value, 'secondary', 'value should be "secondary" and not undefined'); + test.equal(renderCount, 2 * strict, "Should have rendered twice before mount"); // wait for useEffect await waitFor(() => {}, { container, timeout: 250 }); - test.equal(value, 'initial', 'values should still be "initial" after mount'); + test.equal(value, 'secondary', 'value should still be "secondary" after mount'); + test.equal(renderCount, 3 * strict, "Should have rendered 3 times after mount"); + + renderCount = 0; + // trigger after mount immediate rerender + setAfterMountInc(true); + + await waitFor(() => {}, { container, timeout: 250 }); + + test.equal(value, 'third', 'value should still be "third" after immediate rerender after mount'); + test.equal(renderCount, 3 * strict, "Should have rendered 3 times"); completed(); }); diff --git a/packages/react-meteor-data/useTracker.ts b/packages/react-meteor-data/useTracker.ts index ffa93174..b32e61c6 100644 --- a/packages/react-meteor-data/useTracker.ts +++ b/packages/react-meteor-data/useTracker.ts @@ -119,22 +119,22 @@ const useTrackerNoDeps = (reactiveFn: IReactiveFn, skipUpdate: ISkip } const useTrackerWithDeps = (reactiveFn: IReactiveFn, deps: DependencyList, skipUpdate: ISkipUpdate = null): T => { - const [data, setData] = useState(); - const { current: refs } = useRef({ reactiveFn, data, isMounted: false }); - refs.reactiveFn = reactiveFn; + const forceUpdate = useForceUpdate(); - // Only update refs.data when deps change, or the component is mounted. - // This prevents an unexpected value of `undefined` on immediate rerenders. - if (refs.isMounted) { - refs.data = data; - } + const { current: refs } = useRef<{ + reactiveFn: IReactiveFn; + data?: T; + }>({ reactiveFn }); + + // keep reactiveFn ref fresh + refs.reactiveFn = reactiveFn; useMemo(() => { // To jive with the lifecycle interplay between Tracker/Subscribe, run the // reactive function in a computation, then stop it, to force flush cycle. const comp = Tracker.nonreactive( () => Tracker.autorun((c: Tracker.Computation) => { - if (c.firstRun) refs.data = refs.reactiveFn(); + refs.data = refs.reactiveFn(); }) ); // To avoid creating side effects in render, stop the computation immediately @@ -145,12 +145,12 @@ const useTrackerWithDeps = (reactiveFn: IReactiveFn, deps: Dependenc }, deps); useEffect(() => { - refs.isMounted = true; const computation = Tracker.nonreactive( () => Tracker.autorun((c) => { const data: T = refs.reactiveFn(c); if (!skipUpdate || !skipUpdate(refs.data, data)) { - setData(data); + refs.data = data; + forceUpdate(); } }) ); From 756b9c44d30cb2cba4c88b12900083209f54fe82 Mon Sep 17 00:00:00 2001 From: Kevin Newman Date: Thu, 29 Apr 2021 12:08:34 -0400 Subject: [PATCH 7/8] Simplify and centralize checkCursor usage --- packages/react-meteor-data/useTracker.ts | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/packages/react-meteor-data/useTracker.ts b/packages/react-meteor-data/useTracker.ts index b32e61c6..2f75d579 100644 --- a/packages/react-meteor-data/useTracker.ts +++ b/packages/react-meteor-data/useTracker.ts @@ -68,11 +68,7 @@ const useTrackerNoDeps = (reactiveFn: IReactiveFn, skipUpdate: ISkip refs.computation = c; if (c.firstRun) { // Always run the reactiveFn on firstRun - const data = reactiveFn(c); - if (Meteor.isDevelopment) { - checkCursor(data); - } - refs.trackerData = data; + refs.trackerData = reactiveFn(c); } else if (!skipUpdate || !skipUpdate(refs.trackerData, reactiveFn(c))) { // For any reactive change, forceUpdate and let the next render rebuild the computation. forceUpdate(); @@ -139,9 +135,6 @@ const useTrackerWithDeps = (reactiveFn: IReactiveFn, deps: Dependenc ); // To avoid creating side effects in render, stop the computation immediately Meteor.defer(() => { comp.stop() }); - if (Meteor.isDevelopment) { - checkCursor(refs.data); - } }, deps); useEffect(() => { @@ -209,7 +202,9 @@ function useTrackerDev (reactiveFn, deps = null, skipUpdate = null) { } } - return useTracker(reactiveFn, deps, skipUpdate); + const data = useTracker(reactiveFn, deps, skipUpdate); + checkCursor(data); + return data; }; export default Meteor.isDevelopment From 9bf5eceba7f7267f51524404aec8476690101262 Mon Sep 17 00:00:00 2001 From: Kevin Newman Date: Thu, 29 Apr 2021 19:17:09 -0400 Subject: [PATCH 8/8] Update Readme - add for skipUpdate --- packages/react-meteor-data/README.md | 110 ++++++++++++++++++++++++++- 1 file changed, 107 insertions(+), 3 deletions(-) diff --git a/packages/react-meteor-data/README.md b/packages/react-meteor-data/README.md index 1b76be9a..c2c6dd29 100644 --- a/packages/react-meteor-data/README.md +++ b/packages/react-meteor-data/README.md @@ -30,13 +30,26 @@ The `withTracker` HOC can be used with all components, function or class based. It is not necessary to rewrite existing applications to use the `useTracker` hook instead of the existing `withTracker` HOC. -#### `useTracker(reactiveFn, deps)` hook +#### `useTracker(reactiveFn)` basic hook -You can use the `useTracker` hook to get the value of a Tracker reactive function in your (function) components. The reactive function will get re-run whenever its reactive inputs change, and the component will re-render with the new value. +You can use the `useTracker` hook to get the value of a Tracker reactive function in your React "function components." The reactive function will get re-run whenever its reactive inputs change, and the component will re-render with the new value. + +`useTracker` manages its own state, and causes re-renders when necessary. There is no need to call React state setters from inside your `reactiveFn`. Instead, return the values from your `reactiveFn` and assign those to variables directly. When the `reactiveFn` updates, the variables will be updated, and the React component will re-render. Arguments: - `reactiveFn`: A Tracker reactive function (receives the current computation). -- `deps`: An optional array of "dependencies" of the reactive function. This is very similar to how the `deps` argument for [React's built-in `useEffect`, `useCallback` or `useMemo` hooks](https://reactjs.org/docs/hooks-reference.html) work. If omitted, the Tracker computation will be recreated on every render (Note: `withTracker` has always done this). If provided, the computation will be retained, and reactive updates after the first run will run asynchronously from the react render cycle. This array typically includes all variables from the outer scope "captured" in the closure passed as the 1st argument. For example, the value of a prop used in a subscription or a Minimongo query; see example below. + +The basic way to use `useTracker` is to simply pass it a reactive function, with no further fuss. This is the preferred configuration in many cases. + +#### `useTracker(reacitveFn, deps)` hook with deps + +You can pass an optional deps array as a second value. When provided, the computation will be retained, and reactive updates after the first run will run asynchronously from the react render execution frame. This array typically includes all variables from the outer scope "captured" in the closure passed as the 1st argument. For example, the value of a prop used in a subscription or a minimongo query; see example below. + +This should be considered a low level optimization step for cases where your computations are somewhat long running - like a complex minimongo query. In many cases it's safe and even preferred to omit deps and allow the computation to run synchronously with render. + +Arguments: +- `reactiveFn` +- `deps`: An optional array of "dependencies" of the reactive function. This is very similar to how the `deps` argument for [React's built-in `useEffect`, `useCallback` or `useMemo` hooks](https://reactjs.org/docs/hooks-reference.html) work. ```js import { useTracker } from 'meteor/react-meteor-data'; @@ -87,6 +100,44 @@ function Foo({ listId }) { "react-hooks/exhaustive-deps": ["warn", { "additionalHooks": "useTracker|useSomeOtherHook|..." }] ``` +#### `useTracker(reactiveFn, deps, skipUpdate)` or `useTracker(reactiveFn, skipUpdate)` + +You may optionally pass a function as a second or third argument. The `skipUpdate` function can evaluate the return value of `reactiveFn` for changes, and control re-renders in sensitive cases. *Note:* This is not meant to be used iwth a deep compare (even fast-deep-equals), as in many cases that may actually lead to worse performance than allowing React to do it's thing. But as an example, you could use this to compare an `updatedAt` field between updates, or a subset of specific fields, if you aren't using the entire document in a subscription. As always with any optimization, measure first, then optimize second. Make sure you really need this before implementing it. + +Arguments: +- `reactiveFn` +- `deps?` - optional - you may omit this, or pass a "falsy" value. +- `skipUpdate` - A function which receives two arguments: `(prev, next) => (prev === next)`. `prev` and `next` will match the type or data shape as that returned by `reactiveFn`. Note: A return value of `true` means the update will be "skipped". `false` means re-render will occur as normal. So the function should be looking for equivalence. + +```jsx +import { useTracker } from 'meteor/react-meteor-data'; + +// React function component. +function Foo({ listId }) { + const tasks = useTracker( + () => Tasks.find({ listId }).fetch(), [listId], + (prev, next) => { + // prev and next will match the type returned by the reactiveFn + return prev.every((doc, i) => ( + doc._id === next[i] && doc.updatedAt === next[i] + )) && prev.length === next.length; + } + ); + + return ( +

Hello {currentUser.username}

+
+ Here is the Todo list {listId}: +
    + {tasks.map(task => ( +
  • {task.label}
    • + ))} +
+
+ ); +} +``` + #### `withTracker(reactiveFn)` higher-order component You can use the `withTracker` HOC to wrap your components and pass them additional props values from a Tracker reactive function. The reactive function will get re-run whenever its reactive inputs change, and the wrapped component will re-render with the new values for the additional props. @@ -127,6 +178,59 @@ The returned component will, when rendered, render `Foo` (the "lower-order" comp For more information, see the [React article](http://guide.meteor.com/react.html) in the Meteor Guide. +#### `withTracker({ reactiveFn, pure, skipUpdate })` advanced container config + +The `withTracker` HOC can receive a config object instead of a simple reactive function. + +- `getMeteorData` - The `reactiveFn`. +- `pure` - `true` by default. Causes the resulting Container to be wrapped with React's `memo()`. +- `skipUpdate` - A function which receives two arguments: `(prev, next) => (prev === next)`. `prev` and `next` will match the type or data shape as that returned by `reactiveFn`. Note: A return value of `true` means the update will be "skipped". `false` means re-render will occur as normal. So the function should be looking for equivalence. + +```js +import { withTracker } from 'meteor/react-meteor-data'; + +// React component (function or class). +function Foo({ listId, currentUser, listLoading, tasks }) { + return ( +

Hello {currentUser.username}

+ {listLoading ? +
Loading
: +
+ Here is the Todo list {listId}: +
    {tasks.map(task =>
  • {task.label}
    • )}
+ ( + doc._id === next[i] && doc.updatedAt === next[i] + )) + && prev.tasks.length === next.tasks.length + ); + } +})(Foo); +``` + ### Concurrent Mode, Suspense and Error Boundaries There are some additional considerations to keep in mind when using Concurrent Mode, Suspense and Error Boundaries, as each of these can cause React to cancel and discard (toss) a render, including the result of the first run of your reactive function. One of the things React developers often stress is that we should not create "side-effects" directly in the render method or in functional components. There are a number of good reasons for this, including allowing the React runtime to cancel renders. Limiting the use of side-effects allows features such as concurrent mode, suspense and error boundaries to work deterministically, without leaking memory or creating rogue processes. Care should be taken to avoid side effects in your reactive function for these reasons. (Note: this caution does not apply to Meteor specific side-effects like subscriptions, since those will be automatically cleaned up when `useTracker`'s computation is disposed.)