useReactiveVar fails to trigger rerender

[patrickshaughnessy]

Intended outcome:
I was playing around with useReactiveVar after upgrading to Apollo v3 in a large project (source code private unfortunately).

I set up the Apollo cache and reactiveVars according to the docs:

export const cache = new InMemoryCache();

export const valueVar = makeVar('value');

Then in the component:

import {valueVar} from '../cache'

const MyComponent = () => {
  const myValue = useReactiveVar(valueVar);

  const onClick = () => {
    valueVar('newValue');
  }

  return (
    <>
      <div>value is: {myValue}</div>
      <Button onClick={onClick}>Click</Button>
    </>
  )
}

The component should have initially rendered the value is: value, then on button click, rerender with newValue.

Actual outcome:

The React component was not rerendering after a button click. Checking logs in the button click handler, I found that the value of the reactive var was updated, but didn't trigger a rerender from React:

// in button click handler
valueVar('newValue');
console.log('vars', valueVar(), myValue)

After searching around and trying a lot of different ideas, I finally found that the culprit seems to be the useReactiveVar hook.

Going back through the hook's history, I found that my code worked fine and updated correctly when I substituted the original implementation of the hook in for the current version from apollo client:

This works: #6867

export function useReactiveVar<T>(rv: ReactiveVar<T>): T {
  const value = rv();
  // We don't actually care what useState thinks the value of the variable
  // is, so we take only the update function from the returned array.
  const mute = rv.onNextChange(useState(value)[1]);
  // Once the component is unmounted, ignore future updates. Note that the
  // useEffect function returns the mute function without calling it,
  // allowing it to be called when the component unmounts. This is
  // equivalent to useEffect(() => () => mute(), []), but shorter.
  useEffect(() => mute, []);
  return value;
}

This is broken for me: #7652

export function useReactiveVar<T>(rv: ReactiveVar<T>): T {
  const value = rv();
  // We don't actually care what useState thinks the value of the variable
  // is, so we take only the update function from the returned array.
  const [, setValue] = useState(value);
  // 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]);
  // 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
  // equivalent to the following, but shorter:
  // useEffect(() => {
  //   const mute = rv.onNextChange(setValue);
  //   return () => mute();
  // }, [value])

  // We check the variable's value in this useEffect and schedule an update if
  // the value has changed. This check occurs once, on the initial render, to avoid
  // 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())
  }, []);

  return value;
}

How to reproduce the issue:

I've tried to reproduce in a code sandbox: https://codesandbox.io/s/pedantic-bush-e3fef?file=/src/App.js Seems to be working as expected though. I tried throwing in some other Apollo 2 things we're still doing like using local resolvers. I'm not really sure what could be the issue.

Honestly I'm not too sure if it's something on my end, something to do with the React version / Strict Mode (though I did try wrapping my component in StrictMode and it was the same behavior), or something else.

I figure if it's broken for a lot of people there'd be more noise about it, but I haven't been able to figure out why it's happening.

Versions

  System:
    OS: macOS 11.4
  Binaries:
    Node: 12.22.2 - ~/.nvm/versions/node/v12.22.2/bin/node
    npm: 6.14.13 - ~/.nvm/versions/node/v12.22.2/bin/npm
  Browsers:
    Chrome: 92.0.4515.159
    Safari: 14.1.1
  npmPackages:
    @apollo/client: ^3.4.8 => 3.4.8

Running React v16

[benjamn]

@patrickshaughnessy The current implementation of useReactiveVar has changed since the (second) version you copied above. If you're using @apollo/[email protected], you're using this:

export function useReactiveVar<T>(rv: ReactiveVar<T>): T {
  const value = rv();

  // We don't actually care what useState thinks the value of the variable
  // is, so we take only the update function from the returned array.
  const setValue = useState(value)[1];

  // 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(() => {
    const probablySameValue = rv();
    if (value !== probablySameValue) {
      // If the value of rv has already changed, we don't need to listen for the
      // next change, because we can report this change immediately.
      setValue(probablySameValue);
    } else {
      return rv.onNextChange(setValue);
    }
  }, [value]);

  return value;
}

Can you double-check that you're not somehow using an outdated version of useReactiveVar hook?

[patrickshaughnessy]

Hi @benjamn Thanks for the response. Yes - the second version ("broken one" for me) was just illustrating where in the history I observed the breakage

Implement useReactiveVar hook. commit worked fine somehow but after Avoid duplicate useReactiveVar listeners when rendering in React.Stri… commit, the UI was not rerendering after the button click handler updated the reactiveVar.

I created some dummy files similar to the code sandbox posted in the same repo (with the same node modules), and those also seem to be working as expected. At the moment, I still can't seem to figure out what could be happening on my end, but I guess that suggests a code issue that's likely unrelated to the useReactiveVar hook itself. We may have some proprietary modules doing something funky or maybe used some hooks wrong that's causing unexpected behavior (team is still new to hooks and Apollo).

I think it'll be better to close this issue as I can't seem to reproduce. I'll reopen or just comment if I figure it out. Thanks for your time!