Skip to content

Commit

Permalink
use useLayoutEffect instead of useEffect in useReactiveVar (fixes #8012)
Browse files Browse the repository at this point in the history
  • Loading branch information
brainkim committed Apr 23, 2021
1 parent 8e1cf16 commit 412bb4d
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 19 deletions.
17 changes: 3 additions & 14 deletions src/cache/inmemory/reactiveVars.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,6 @@ export type ReactiveListener<T> = (value: T) => any;
// called in Policies#readField.
export const cacheSlot = new Slot<ApolloCache<any>>();

// A listener function could in theory cause another listener to be added
// to the set while we're iterating over it, so it's important to commit
// to the original elements of the set before we begin iterating. See
// iterateObserversSafely for another example of this pattern.
function consumeAndIterate<T>(set: Set<T>, callback: (item: T) => any) {
if (set.size) {
const items: T[] = [];
set.forEach(item => items.push(item));
set.clear();
items.forEach(callback);
}
}

const cacheInfoMap = new WeakMap<ApolloCache<any>, {
vars: Set<ReactiveVar<any>>;
dep: OptimisticDependencyFunction<ReactiveVar<any>>;
Expand Down Expand Up @@ -79,7 +66,9 @@ export function makeVar<T>(value: T): ReactiveVar<T> {
broadcast(cache);
});
// Finally, notify any listeners added via rv.onNextChange.
consumeAndIterate(listeners, listener => listener(value));
const oldListeners = Array.from(listeners);
listeners.clear();
oldListeners.forEach(listener => listener(value));
}
} else {
// When reading from the variable, obtain the current cache from
Expand Down
8 changes: 3 additions & 5 deletions src/react/hooks/useReactiveVar.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useState, useEffect } from 'react';
import { useState, useEffect, useLayoutEffect } from 'react';
import { ReactiveVar } from '../../core';

export function useReactiveVar<T>(rv: ReactiveVar<T>): T {
Expand All @@ -9,7 +9,7 @@ export function useReactiveVar<T>(rv: ReactiveVar<T>): T {
// We subscribe to variable updates on initial mount and when the value has
// changed. This avoids a subtle bug in React.StrictMode where multiple listeners
// are added, leading to inconsistent updates.
useEffect(() => rv.onNextChange(setValue), [value]);
useLayoutEffect(() => rv.onNextChange(setValue), [value]);
// Once the component is unmounted, ignore future updates. Note that the
// above useEffect function returns a mute function without calling it,
// allowing it to be called when the component unmounts. This is
Expand All @@ -24,9 +24,7 @@ export function useReactiveVar<T>(rv: ReactiveVar<T>): T {
// a useEffect higher in the component tree changing a variable's value
// before the above useEffect can set the onNextChange handler. Note that React
// will not schedule an update if setState is called with the same value as before.
useEffect(() => {
setValue(rv())
}, []);
useEffect(() => setValue(rv()), []);

return value;
}

0 comments on commit 412bb4d

Please sign in to comment.