From fe4fa4216b33d3691c8b72c5529a91c30538e352 Mon Sep 17 00:00:00 2001 From: menelike Date: Fri, 21 Jun 2019 18:24:37 +0200 Subject: [PATCH 01/18] - rewrite useTracker in order to stay fully consistent with current withTracker behavior - compare deps in order to keep API consistency to React.useEffect() --- packages/react-meteor-data/useTracker.js | 132 +++++++++++++++++------ 1 file changed, 98 insertions(+), 34 deletions(-) diff --git a/packages/react-meteor-data/useTracker.js b/packages/react-meteor-data/useTracker.js index 2e25d3b4..0b676ecf 100644 --- a/packages/react-meteor-data/useTracker.js +++ b/packages/react-meteor-data/useTracker.js @@ -1,4 +1,4 @@ -import React, { useState, useEffect } from 'react'; +import React, { useState, useEffect, useRef } from 'react'; import { Tracker } from 'meteor/tracker'; import { Meteor } from 'meteor/meteor'; @@ -28,41 +28,105 @@ function checkCursor(data) { } } -// Forgetting the deps parameter would cause an infinite rerender loop, so we default to []. -function useTracker(reactiveFn, deps = []) { - // Note : we always run the reactiveFn in Tracker.nonreactive in case - // we are already inside a Tracker Computation. This can happen if someone calls - // `ReactDOM.render` inside a Computation. In that case, we want to opt out - // of the normal behavior of nested Computations, where if the outer one is - // invalidated or stopped, it stops the inner one too. - - const [trackerData, setTrackerData] = useState(() => { - // No side-effects are allowed when computing the initial value. - // To get the initial return value for the 1st render on mount, - // we run reactiveFn without autorun or subscriptions. - // Note: maybe when React Suspense is officially available we could - // throw a Promise instead to skip the 1st render altogether ? - const realSubscribe = Meteor.subscribe; - Meteor.subscribe = () => ({ stop: () => {}, ready: () => false }); - const initialData = Tracker.nonreactive(reactiveFn); - Meteor.subscribe = realSubscribe; - return initialData; - }); - - useEffect(() => { - // Set up the reactive computation. - const computation = Tracker.nonreactive(() => - Tracker.autorun(() => { - const data = reactiveFn(); - Meteor.isDevelopment && checkCursor(data); - setTrackerData(data); +// taken from https://github.com/facebook/react/blob/ +// 34ce57ae751e0952fd12ab532a3e5694445897ea/packages/shared/objectIs.js +function is(x, y) { + return ( + (x === y && (x !== 0 || 1 / x === 1 / y)) + || (x !== x && y !== y) // eslint-disable-line no-self-compare + ); +} + +// inspired by https://github.com/facebook/react/blob/ +// 34ce57ae751e0952fd12ab532a3e5694445897ea/packages/ +// react-reconciler/src/ReactFiberHooks.js#L307-L354 +// used to replicate dep change behavior and stay consistent +// with React.useEffect() +function areHookInputsEqual(nextDeps, prevDeps) { + if (!nextDeps || !prevDeps) { + return false; + } + + const len = nextDeps.length; + + if (prevDeps.length !== len) { + return false; + } + + for (let i = 0; i < len; i++) { + if (!is(nextDeps[i], prevDeps[i])) { + return false; + } + } + + return true; +} + +function useTracker(reactiveFn, deps) { + // 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 + if (Meteor.isServer) { + return reactiveFn(); + } + + const previousDeps = useRef(); + const computation = useRef(); + const trackerData = useRef(); + + const [, forceUpdate] = useState(); + + const dispose = () => { + if (computation.current) { + computation.current.stop(); + computation.current = null; + } + }; + + // this is called like at componentWillMount and componentWillUpdate equally + // simulates a synchronous useEffect, as a replacement for calculateData() + // if prevDeps or deps are not set shallowEqualArray always returns false + if (!areHookInputsEqual(deps, previousDeps.current)) { + dispose(); + + // Use Tracker.nonreactive in case we are inside a Tracker Computation. + // This can happen if someone calls `ReactDOM.render` inside a Computation. + // In that case, we want to opt out of the normal behavior of nested + // Computations, where if the outer one is invalidated or stopped, + // it stops the inner one. + computation.current = Tracker.nonreactive(() => ( + Tracker.autorun((c) => { + if (c.firstRun) { + const data = reactiveFn(); + Meteor.isDevelopment && checkCursor(data); + + // store the deps for comparison on next render + previousDeps.current = deps; + trackerData.current = data; + } else { + // makes sure that shallowEqualArray returns false on next render + previousDeps.current = Math.random(); + // Stop this computation instead of using the re-run. + // We use a brand-new autorun for each call to getMeteorData + // to capture dependencies on any reactive data sources that + // are accessed. The reason we can't use a single autorun + // for the lifetime of the component is that Tracker only + // re-runs autoruns at flush time, while we need to be able to + // re-call the reactive function synchronously whenever we want, e.g. + // from next render. + c.stop(); + // use Math.random() to trigger a state change to enforce a re-render + // Calling forceUpdate() triggers componentWillUpdate which + // calls the reactive function and re-renders the component. + forceUpdate(Math.random()); + } }) - ); - // On effect cleanup, stop the computation. - return () => computation.stop(); - }, deps); + )); + } + + // stop the computation on unmount only + useEffect(() => dispose, []); - return trackerData; + return trackerData.current; } // When rendering on the server, we don't want to use the Tracker. From fbc33c6888813b8f029bf84501623644c7ffb3c5 Mon Sep 17 00:00:00 2001 From: menelike Date: Fri, 21 Jun 2019 18:30:31 +0200 Subject: [PATCH 02/18] - withTracker should always recompute on re-render so deps for useTracker() can be omitted - also React.memo() already has a check for prop change so there is no need to check for changed deps again --- packages/react-meteor-data/withTracker.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-meteor-data/withTracker.jsx b/packages/react-meteor-data/withTracker.jsx index 78455467..e6e6a9f2 100644 --- a/packages/react-meteor-data/withTracker.jsx +++ b/packages/react-meteor-data/withTracker.jsx @@ -7,7 +7,7 @@ export default function withTracker(options) { const { getMeteorData, pure = true } = expandedOptions; const WithTracker = forwardRef((props, ref) => { - const data = useTracker(() => getMeteorData(props) || {}, [props]); + const data = useTracker(() => getMeteorData(props) || {}); return ; }); From c8d8645887cc1fa7c433446daa4a4694bef163b6 Mon Sep 17 00:00:00 2001 From: menelike Date: Fri, 21 Jun 2019 18:39:04 +0200 Subject: [PATCH 03/18] update Readme to reflect omitted deps behavior --- packages/react-meteor-data/README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/react-meteor-data/README.md b/packages/react-meteor-data/README.md index 51ef929c..e4d953c4 100644 --- a/packages/react-meteor-data/README.md +++ b/packages/react-meteor-data/README.md @@ -22,7 +22,7 @@ This package provides two ways to use Tracker reactive data in your React compon - a hook: `useTracker` (v2 only, requires React `^16.8`) - a higher-order component (HOC): `withTracker` (v1 and v2). -The `useTracker` hook, introduced in version 2.0.0, is slightly more straightforward to use (lets you access reactive data sources directly within your componenent, rather than adding them from an external wrapper), and slightly more performant (avoids adding wrapper layers in the React tree). But, like all React hooks, it can only be used in function components, not in class components. +The `useTracker` hook, introduced in version 2.0.0, is slightly more straightforward to use (lets you access reactive data sources directly within your componenent, rather than adding them from an external wrapper), and slightly more performant (avoids adding wrapper layers in the React tree). But, like all React hooks, it can only be used in function components, not in class components. The `withTracker` HOC can be used with all components, function or class. It is not necessary to rewrite existing applications to use the `useTracker` hook instead of the existing `withTracker` HOC. But for new components, it is suggested to prefer the `useTracker` hook when dealing with function components. @@ -33,8 +33,8 @@ You can use the `useTracker` hook to get the value of a Tracker reactive functio Arguments: - `reactiveFn`: a Tracker reactive function (with no parameters) -- `deps`: an array of "dependencies" of the reactive function, i.e. the list of values that, when changed, need to stop the current Tracker computation and start a new one - for example, the value of a prop used in a subscription or a Minimongo query; see example below. This array typically includes all variables from the outer scope "captured" in the closure passed as the 1st argument. 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, it defaults to `[]` (no dependency), and the Tracker computation will run unchanged until the component is unmounted. +- `deps`: an array of "dependencies" of the reactive function, i.e. the list of values that, when changed, need to stop the current Tracker computation and start a new one - for example, the value of a prop used in a subscription or a Minimongo query; see example below. This array typically includes all variables from the outer scope "captured" in the closure passed as the 1st argument. 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 call. ```js import { useTracker } from 'meteor/react-meteor-data'; From 44b5247a9ede8582e86a137398b041498543c070 Mon Sep 17 00:00:00 2001 From: menelike Date: Fri, 21 Jun 2019 18:47:02 +0200 Subject: [PATCH 04/18] fix code comment --- packages/react-meteor-data/useTracker.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/react-meteor-data/useTracker.js b/packages/react-meteor-data/useTracker.js index 0b676ecf..d057f7ab 100644 --- a/packages/react-meteor-data/useTracker.js +++ b/packages/react-meteor-data/useTracker.js @@ -83,8 +83,9 @@ function useTracker(reactiveFn, deps) { }; // this is called like at componentWillMount and componentWillUpdate equally - // simulates a synchronous useEffect, as a replacement for calculateData() - // if prevDeps or deps are not set shallowEqualArray always returns false + // in order to support render calls with synchronous data from the reactive computation + // if prevDeps or deps are not set areHookInputsEqual always returns false + // and the reactive functions is always called if (!areHookInputsEqual(deps, previousDeps.current)) { dispose(); From ddfb7cd61b1e564d9a5144ee67797399ccc08055 Mon Sep 17 00:00:00 2001 From: menelike Date: Fri, 21 Jun 2019 20:29:33 +0200 Subject: [PATCH 05/18] get rid of Math.random(), wasn't needed at all --- packages/react-meteor-data/useTracker.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/react-meteor-data/useTracker.js b/packages/react-meteor-data/useTracker.js index d057f7ab..d3ed1da0 100644 --- a/packages/react-meteor-data/useTracker.js +++ b/packages/react-meteor-data/useTracker.js @@ -104,8 +104,9 @@ function useTracker(reactiveFn, deps) { previousDeps.current = deps; trackerData.current = data; } else { - // makes sure that shallowEqualArray returns false on next render - previousDeps.current = Math.random(); + // makes sure that shallowEqualArray returns false + // which is always the case when prev or nextDeps are not set + previousDeps.current = null; // Stop this computation instead of using the re-run. // We use a brand-new autorun for each call to getMeteorData // to capture dependencies on any reactive data sources that From 365584f02b0a94dd6552a55827e2dc99596a6634 Mon Sep 17 00:00:00 2001 From: menelike Date: Fri, 21 Jun 2019 20:30:49 +0200 Subject: [PATCH 06/18] don't handle Meteor.isServer as it's already been taken care of in exports --- packages/react-meteor-data/useTracker.js | 6 ------ 1 file changed, 6 deletions(-) diff --git a/packages/react-meteor-data/useTracker.js b/packages/react-meteor-data/useTracker.js index d3ed1da0..adfa44a9 100644 --- a/packages/react-meteor-data/useTracker.js +++ b/packages/react-meteor-data/useTracker.js @@ -63,12 +63,6 @@ function areHookInputsEqual(nextDeps, prevDeps) { } function useTracker(reactiveFn, deps) { - // 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 - if (Meteor.isServer) { - return reactiveFn(); - } - const previousDeps = useRef(); const computation = useRef(); const trackerData = useRef(); From c3803b96cf2eb843c8df549753586d398352f201 Mon Sep 17 00:00:00 2001 From: menelike Date: Fri, 21 Jun 2019 21:02:57 +0200 Subject: [PATCH 07/18] replace Math.random() when enforcing an update --- packages/react-meteor-data/useTracker.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/react-meteor-data/useTracker.js b/packages/react-meteor-data/useTracker.js index adfa44a9..aca41184 100644 --- a/packages/react-meteor-data/useTracker.js +++ b/packages/react-meteor-data/useTracker.js @@ -62,6 +62,7 @@ function areHookInputsEqual(nextDeps, prevDeps) { return true; } +let uniqueCounter = 0; function useTracker(reactiveFn, deps) { const previousDeps = useRef(); const computation = useRef(); @@ -113,7 +114,7 @@ function useTracker(reactiveFn, deps) { // use Math.random() to trigger a state change to enforce a re-render // Calling forceUpdate() triggers componentWillUpdate which // calls the reactive function and re-renders the component. - forceUpdate(Math.random()); + forceUpdate(++uniqueCounter); } }) )); From eeb115a942c7711166c9859d4ae59ecc788d11b2 Mon Sep 17 00:00:00 2001 From: menelike Date: Sat, 22 Jun 2019 08:16:42 +0200 Subject: [PATCH 08/18] - align areHookInputsEqual with https://github.com/facebook/react/blob/d77d12510b1a1c37484d771a323e0a02cbeb9ba7/packages/react-reconciler/src/ReactFiberHooks.js#L231 https://github.com/facebook/react/blob/d77d12510b1a1c37484d771a323e0a02cbeb9ba7/packages/react-reconciler/src/ReactFiberHooks.js#L318-L329 https://github.com/facebook/react/blob/d77d12510b1a1c37484d771a323e0a02cbeb9ba7/packages/react-reconciler/src/ReactFiberHooks.js#L878 - update comments --- packages/react-meteor-data/useTracker.js | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/packages/react-meteor-data/useTracker.js b/packages/react-meteor-data/useTracker.js index aca41184..23724755 100644 --- a/packages/react-meteor-data/useTracker.js +++ b/packages/react-meteor-data/useTracker.js @@ -43,7 +43,12 @@ function is(x, y) { // used to replicate dep change behavior and stay consistent // with React.useEffect() function areHookInputsEqual(nextDeps, prevDeps) { - if (!nextDeps || !prevDeps) { + if (prevDeps === null || prevDeps === undefined) { + return false; + } + + // checking prevDeps is unnecessary as prevDeps is always the last version of nextDeps + if (!Array.isArray(nextDeps)) { return false; } @@ -100,10 +105,10 @@ function useTracker(reactiveFn, deps) { trackerData.current = data; } else { // makes sure that shallowEqualArray returns false - // which is always the case when prev or nextDeps are not set + // which is always the case when prevDeps is null previousDeps.current = null; // Stop this computation instead of using the re-run. - // We use a brand-new autorun for each call to getMeteorData + // We use a brand-new autorun for each call // to capture dependencies on any reactive data sources that // are accessed. The reason we can't use a single autorun // for the lifetime of the component is that Tracker only @@ -111,9 +116,9 @@ function useTracker(reactiveFn, deps) { // re-call the reactive function synchronously whenever we want, e.g. // from next render. c.stop(); - // use Math.random() to trigger a state change to enforce a re-render - // Calling forceUpdate() triggers componentWillUpdate which - // calls the reactive function and re-renders the component. + // use a uniqueCounter to trigger a state change to enforce a re-render + // which calls the reactive function and re-renders the component with + // new data from the reactive function. forceUpdate(++uniqueCounter); } }) From 1eb71ddd28becbccc9c89a28601b25fa74c5d5b0 Mon Sep 17 00:00:00 2001 From: menelike Date: Sat, 22 Jun 2019 08:28:33 +0200 Subject: [PATCH 09/18] warn if dep is not an array when Meteor isDevelopment --- packages/react-meteor-data/useTracker.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/react-meteor-data/useTracker.js b/packages/react-meteor-data/useTracker.js index 23724755..8b2b727e 100644 --- a/packages/react-meteor-data/useTracker.js +++ b/packages/react-meteor-data/useTracker.js @@ -49,6 +49,14 @@ function areHookInputsEqual(nextDeps, prevDeps) { // checking prevDeps is unnecessary as prevDeps is always the last version of nextDeps if (!Array.isArray(nextDeps)) { + if (Meteor.isDevelopment) { + // Use React.warn() if available (should ship in React 16.9). + const warn = React.warn || console.warn.bind(console); + warn( + 'Warning: useTracker expected an dependency value of ' + + `type array but got type of ${typeof nextDeps} instead.` + ); + } return false; } @@ -68,6 +76,7 @@ function areHookInputsEqual(nextDeps, prevDeps) { } let uniqueCounter = 0; + function useTracker(reactiveFn, deps) { const previousDeps = useRef(); const computation = useRef(); From 6b540e6d57c1d922220cf67e6ecd0771f54e12d4 Mon Sep 17 00:00:00 2001 From: menelike Date: Sat, 22 Jun 2019 10:26:55 +0200 Subject: [PATCH 10/18] fix prevDeps isArray check --- packages/react-meteor-data/useTracker.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/react-meteor-data/useTracker.js b/packages/react-meteor-data/useTracker.js index 8b2b727e..2d27b6c4 100644 --- a/packages/react-meteor-data/useTracker.js +++ b/packages/react-meteor-data/useTracker.js @@ -43,11 +43,10 @@ function is(x, y) { // used to replicate dep change behavior and stay consistent // with React.useEffect() function areHookInputsEqual(nextDeps, prevDeps) { - if (prevDeps === null || prevDeps === undefined) { + if (prevDeps === null || prevDeps === undefined || !Array.isArray(prevDeps)) { return false; } - // checking prevDeps is unnecessary as prevDeps is always the last version of nextDeps if (!Array.isArray(nextDeps)) { if (Meteor.isDevelopment) { // Use React.warn() if available (should ship in React 16.9). From 8d64e41c9168ba905e5ff9153aeb7cbf4cdc9274 Mon Sep 17 00:00:00 2001 From: menelike Date: Sat, 22 Jun 2019 11:21:21 +0200 Subject: [PATCH 11/18] warn if initial deps is not an array --- packages/react-meteor-data/useTracker.js | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/packages/react-meteor-data/useTracker.js b/packages/react-meteor-data/useTracker.js index 2d27b6c4..1bbb851a 100644 --- a/packages/react-meteor-data/useTracker.js +++ b/packages/react-meteor-data/useTracker.js @@ -134,7 +134,20 @@ function useTracker(reactiveFn, deps) { } // stop the computation on unmount only - useEffect(() => dispose, []); + useEffect(() => { + if (Meteor.isDevelopment + && deps !== null && deps !== undefined + && !Array.isArray(deps)) { + // Use React.warn() if available (should ship in React 16.9). + const warn = React.warn || console.warn.bind(console); + warn( + 'Warning: useTracker expected an initial dependency value of ' + + `type array but got type of ${typeof deps} instead.` + ); + } + + return dispose; + }, []); return trackerData.current; } From b1996a263419bf15569d4f4b30c7d51479632260 Mon Sep 17 00:00:00 2001 From: Kevin Newman Date: Mon, 1 Jul 2019 15:12:59 -0400 Subject: [PATCH 12/18] Retain synchronous render behavior for firstRun (including after deps change), but allow async and computation reuse after. --- packages/react-meteor-data/useTracker.js | 34 +++++++++--------------- 1 file changed, 13 insertions(+), 21 deletions(-) diff --git a/packages/react-meteor-data/useTracker.js b/packages/react-meteor-data/useTracker.js index 1bbb851a..21c8ad57 100644 --- a/packages/react-meteor-data/useTracker.js +++ b/packages/react-meteor-data/useTracker.js @@ -95,38 +95,30 @@ function useTracker(reactiveFn, deps) { // if prevDeps or deps are not set areHookInputsEqual always returns false // and the reactive functions is always called if (!areHookInputsEqual(deps, previousDeps.current)) { - dispose(); - // Use Tracker.nonreactive in case we are inside a Tracker Computation. // This can happen if someone calls `ReactDOM.render` inside a Computation. // In that case, we want to opt out of the normal behavior of nested // Computations, where if the outer one is invalidated or stopped, // it stops the inner one. - computation.current = Tracker.nonreactive(() => ( + Tracker.nonreactive(() => ( Tracker.autorun((c) => { + // This will capture data synchronously on first run (and after deps change). + // Additional cycles will follow the normal computation behavior. + const data = reactiveFn(); + if (Meteor.isDevelopment) checkCursor(data); + trackerData.current = data; + if (c.firstRun) { - const data = reactiveFn(); - Meteor.isDevelopment && checkCursor(data); + // if we are re-creating the computation, we need to stop the old one. + dispose(); + + // store the new computation + computation.current = c; // store the deps for comparison on next render previousDeps.current = deps; - trackerData.current = data; } else { - // makes sure that shallowEqualArray returns false - // which is always the case when prevDeps is null - previousDeps.current = null; - // Stop this computation instead of using the re-run. - // We use a brand-new autorun for each call - // to capture dependencies on any reactive data sources that - // are accessed. The reason we can't use a single autorun - // for the lifetime of the component is that Tracker only - // re-runs autoruns at flush time, while we need to be able to - // re-call the reactive function synchronously whenever we want, e.g. - // from next render. - c.stop(); - // use a uniqueCounter to trigger a state change to enforce a re-render - // which calls the reactive function and re-renders the component with - // new data from the reactive function. + // use a uniqueCounter to trigger a state change to force a re-render forceUpdate(++uniqueCounter); } }) From af6a4a0296d0df24296369c47ecb12babb4a8d4f Mon Sep 17 00:00:00 2001 From: Kevin Newman Date: Mon, 1 Jul 2019 15:17:33 -0400 Subject: [PATCH 13/18] Fix eslint errors --- packages/react-meteor-data/useTracker.js | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/packages/react-meteor-data/useTracker.js b/packages/react-meteor-data/useTracker.js index 1bbb851a..c16bb3bf 100644 --- a/packages/react-meteor-data/useTracker.js +++ b/packages/react-meteor-data/useTracker.js @@ -1,6 +1,8 @@ +/* global Meteor, Package, Tracker */ import React, { useState, useEffect, useRef } from 'react'; -import { Tracker } from 'meteor/tracker'; -import { Meteor } from 'meteor/meteor'; + +// Use React.warn() if available (should ship in React 16.9). +const warn = React.warn || console.warn.bind(console); // Warns if data is a Mongo.Cursor or a POJO containing a Mongo.Cursor. function checkCursor(data) { @@ -8,8 +10,7 @@ function checkCursor(data) { if (Package.mongo && Package.mongo.Mongo && data && typeof data === 'object') { if (data instanceof Package.mongo.Mongo.Cursor) { shouldWarn = true; - } - else if (Object.getPrototypeOf(data) === Object.prototype) { + } else if (Object.getPrototypeOf(data) === Object.prototype) { Object.keys(data).forEach((key) => { if (data[key] instanceof Package.mongo.Mongo.Cursor) { shouldWarn = true; @@ -18,8 +19,6 @@ function checkCursor(data) { } } if (shouldWarn) { - // Use React.warn() if available (should ship in React 16.9). - const warn = React.warn || console.warn.bind(console); warn( 'Warning: your reactive function is returning a Mongo cursor. ' + 'This value will not be reactive. You probably want to call ' @@ -49,8 +48,6 @@ function areHookInputsEqual(nextDeps, prevDeps) { if (!Array.isArray(nextDeps)) { if (Meteor.isDevelopment) { - // Use React.warn() if available (should ship in React 16.9). - const warn = React.warn || console.warn.bind(console); warn( 'Warning: useTracker expected an dependency value of ' + `type array but got type of ${typeof nextDeps} instead.` @@ -106,7 +103,7 @@ function useTracker(reactiveFn, deps) { Tracker.autorun((c) => { if (c.firstRun) { const data = reactiveFn(); - Meteor.isDevelopment && checkCursor(data); + if (Meteor.isDevelopment) checkCursor(data); // store the deps for comparison on next render previousDeps.current = deps; @@ -138,8 +135,6 @@ function useTracker(reactiveFn, deps) { if (Meteor.isDevelopment && deps !== null && deps !== undefined && !Array.isArray(deps)) { - // Use React.warn() if available (should ship in React 16.9). - const warn = React.warn || console.warn.bind(console); warn( 'Warning: useTracker expected an initial dependency value of ' + `type array but got type of ${typeof deps} instead.` @@ -154,8 +149,8 @@ function useTracker(reactiveFn, deps) { // 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 -function useTracker__server(reactiveFn, deps) { +function useTrackerServer(reactiveFn) { return reactiveFn(); } -export default (Meteor.isServer ? useTracker__server : useTracker); +export default (Meteor.isServer ? useTrackerServer : useTracker); From be11569d96011112a577cd448e23776dab8cbb92 Mon Sep 17 00:00:00 2001 From: Kevin Newman Date: Tue, 2 Jul 2019 17:50:57 -0400 Subject: [PATCH 14/18] disambiguate the disposition of previous computation - this works the same as before, but is easier to read --- packages/react-meteor-data/useTracker.js | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/packages/react-meteor-data/useTracker.js b/packages/react-meteor-data/useTracker.js index a9e02993..82a1b36f 100644 --- a/packages/react-meteor-data/useTracker.js +++ b/packages/react-meteor-data/useTracker.js @@ -92,12 +92,15 @@ function useTracker(reactiveFn, deps) { // if prevDeps or deps are not set areHookInputsEqual always returns false // and the reactive functions is always called if (!areHookInputsEqual(deps, previousDeps.current)) { + // if we are re-creating the computation, we need to stop the old one. + dispose(); + // Use Tracker.nonreactive in case we are inside a Tracker Computation. // This can happen if someone calls `ReactDOM.render` inside a Computation. // In that case, we want to opt out of the normal behavior of nested // Computations, where if the outer one is invalidated or stopped, // it stops the inner one. - Tracker.nonreactive(() => ( + computation.current = Tracker.nonreactive(() => ( Tracker.autorun((c) => { // This will capture data synchronously on first run (and after deps change). // Additional cycles will follow the normal computation behavior. @@ -106,12 +109,6 @@ function useTracker(reactiveFn, deps) { trackerData.current = data; if (c.firstRun) { - // if we are re-creating the computation, we need to stop the old one. - dispose(); - - // store the new computation - computation.current = c; - // store the deps for comparison on next render previousDeps.current = deps; } else { From 80fef10086f345b6f375682e9ebcb291d87b4b1c Mon Sep 17 00:00:00 2001 From: Kevin Newman Date: Thu, 4 Jul 2019 13:26:03 -0400 Subject: [PATCH 15/18] Use 1 useRef instead of multiple --- packages/react-meteor-data/useTracker.js | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/packages/react-meteor-data/useTracker.js b/packages/react-meteor-data/useTracker.js index 82a1b36f..52c4efed 100644 --- a/packages/react-meteor-data/useTracker.js +++ b/packages/react-meteor-data/useTracker.js @@ -74,16 +74,14 @@ function areHookInputsEqual(nextDeps, prevDeps) { let uniqueCounter = 0; function useTracker(reactiveFn, deps) { - const previousDeps = useRef(); - const computation = useRef(); - const trackerData = useRef(); + const { current: refs } = useRef({}); const [, forceUpdate] = useState(); const dispose = () => { - if (computation.current) { - computation.current.stop(); - computation.current = null; + if (refs.computation) { + refs.computation.stop(); + refs.computation = null; } }; @@ -91,7 +89,7 @@ function useTracker(reactiveFn, deps) { // in order to support render calls with synchronous data from the reactive computation // if prevDeps or deps are not set areHookInputsEqual always returns false // and the reactive functions is always called - if (!areHookInputsEqual(deps, previousDeps.current)) { + if (!areHookInputsEqual(deps, refs.previousDeps)) { // if we are re-creating the computation, we need to stop the old one. dispose(); @@ -100,17 +98,17 @@ function useTracker(reactiveFn, deps) { // In that case, we want to opt out of the normal behavior of nested // Computations, where if the outer one is invalidated or stopped, // it stops the inner one. - computation.current = Tracker.nonreactive(() => ( + refs.computation = Tracker.nonreactive(() => ( Tracker.autorun((c) => { // This will capture data synchronously on first run (and after deps change). // Additional cycles will follow the normal computation behavior. const data = reactiveFn(); if (Meteor.isDevelopment) checkCursor(data); - trackerData.current = data; + refs.trackerData = data; if (c.firstRun) { // store the deps for comparison on next render - previousDeps.current = deps; + refs.previousDeps = deps; } else { // use a uniqueCounter to trigger a state change to force a re-render forceUpdate(++uniqueCounter); @@ -133,7 +131,7 @@ function useTracker(reactiveFn, deps) { return dispose; }, []); - return trackerData.current; + return refs.trackerData; } // When rendering on the server, we don't want to use the Tracker. From 8ae0db41e95d818391689759521fb751cab84906 Mon Sep 17 00:00:00 2001 From: Kevin Newman Date: Thu, 4 Jul 2019 13:36:45 -0400 Subject: [PATCH 16/18] If reactiveFn will run synchrously next render, don't bother running it asynchronously --- packages/react-meteor-data/useTracker.js | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/packages/react-meteor-data/useTracker.js b/packages/react-meteor-data/useTracker.js index 52c4efed..ef37f3e6 100644 --- a/packages/react-meteor-data/useTracker.js +++ b/packages/react-meteor-data/useTracker.js @@ -93,6 +93,9 @@ function useTracker(reactiveFn, deps) { // if we are re-creating the computation, we need to stop the old one. dispose(); + // store the deps for comparison on next render + refs.previousDeps = deps; + // Use Tracker.nonreactive in case we are inside a Tracker Computation. // This can happen if someone calls `ReactDOM.render` inside a Computation. // In that case, we want to opt out of the normal behavior of nested @@ -100,16 +103,22 @@ function useTracker(reactiveFn, deps) { // it stops the inner one. refs.computation = Tracker.nonreactive(() => ( Tracker.autorun((c) => { - // This will capture data synchronously on first run (and after deps change). - // Additional cycles will follow the normal computation behavior. - const data = reactiveFn(); - if (Meteor.isDevelopment) checkCursor(data); - refs.trackerData = data; + const runReactiveFn = () => { + const data = reactiveFn(); + if (Meteor.isDevelopment) checkCursor(data); + refs.trackerData = data; + }; if (c.firstRun) { - // store the deps for comparison on next render - refs.previousDeps = deps; + // This will capture data synchronously on first run (and after deps change). + // Additional cycles will follow the normal computation behavior. + runReactiveFn(); } else { + // Only run reactiveFn if the hooks have not change, or are not falsy. + if (areHookInputsEqual(deps, refs.previousDeps)) { + runReactiveFn(); + } + // If deps have changed or are falsy, let the reactiveFn run on next render. // use a uniqueCounter to trigger a state change to force a re-render forceUpdate(++uniqueCounter); } From f68ae5be17eae82e46f391fc1cc1f4abc2ae8dd7 Mon Sep 17 00:00:00 2001 From: Kevin Newman Date: Thu, 4 Jul 2019 14:21:15 -0400 Subject: [PATCH 17/18] Dispose of the computation early, if the deps are falsy on meteor reactive changes --- packages/react-meteor-data/useTracker.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/react-meteor-data/useTracker.js b/packages/react-meteor-data/useTracker.js index ef37f3e6..7b4ab5bf 100644 --- a/packages/react-meteor-data/useTracker.js +++ b/packages/react-meteor-data/useTracker.js @@ -114,11 +114,13 @@ function useTracker(reactiveFn, deps) { // Additional cycles will follow the normal computation behavior. runReactiveFn(); } else { - // Only run reactiveFn if the hooks have not change, or are not falsy. - if (areHookInputsEqual(deps, refs.previousDeps)) { + // Only run reactiveFn if the deps or are not falsy. + if (!deps || !refs.previousDeps) { + // Dispose early, if refs are falsy - we'll rebuild and run on the next render. + dispose(); + } else { runReactiveFn(); } - // If deps have changed or are falsy, let the reactiveFn run on next render. // use a uniqueCounter to trigger a state change to force a re-render forceUpdate(++uniqueCounter); } From c58702612c5c86b6adab6193e6e6a133adf318b6 Mon Sep 17 00:00:00 2001 From: Kevin Newman Date: Fri, 5 Jul 2019 01:53:23 -0400 Subject: [PATCH 18/18] previousDeps will always equal deps at this point, so just check previousDeps --- packages/react-meteor-data/useTracker.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/react-meteor-data/useTracker.js b/packages/react-meteor-data/useTracker.js index 7b4ab5bf..fff0af45 100644 --- a/packages/react-meteor-data/useTracker.js +++ b/packages/react-meteor-data/useTracker.js @@ -114,9 +114,8 @@ function useTracker(reactiveFn, deps) { // Additional cycles will follow the normal computation behavior. runReactiveFn(); } else { - // Only run reactiveFn if the deps or are not falsy. - if (!deps || !refs.previousDeps) { - // Dispose early, if refs are falsy - we'll rebuild and run on the next render. + // If deps are falsy, stop computation and let next render handle reactiveFn. + if (!refs.previousDeps) { dispose(); } else { runReactiveFn();