Material-ui: `useSynchronousEffect` is not React-Hot-Loader compatible

[theKashey]

Source issue - https://github.com/gaearon/react-hot-loader/issues/1262

One of the biggest issues for React-Hot-Loader in the last past months were hooks, which were not working properly, especially from the hot-loading point of view.

Version 4.9.0 introduced the fix for it via adding one more argument to every hook with dependencies, like useEffect.

The Problem

useSynchronousEffect is split into two parts - apply the effect, and remove it.

• remove effect uses useEffect, and got called during hot update
• apply effect uses internal deps list, and not called on hot update
• the result - styles are gone.

function useSynchronousEffect(func, values) {
  const ref = React.useRef([]); // THIS REF WOULD BE PRESERVED
  let output;

  // THIS WOULD NOT BE RE-RUN ON HOOK UPDATE
  if (ref.current.length !== values.length) {
    ref.current = values;
    output = func();
  } else {
    for (let i = 0; i < values.length; i += 1) {
      if (values[i] !== ref.current[i]) {
        ref.current = values;
        output = func();
        break;
      }
    }
  }

  React.useEffect(
    () => () => {
      if (output) {
        output();      //  WILL BE RESET ON EFFECT RE-RUN
      }
    },
    values, // eslint-disable-line react-hooks/exhaustive-deps
  );
}

This issue should be solved regardless of the nature of hooks hot reloading - a special logic like this should be a bit more reliable.

• values.length expected to never be changed during the component life cycle (rules of hooks)
• you may convert useSynchronousEffect to useEffect on clientside (the best option)
• you may use useMemo to check stored values, but it still not quite stable, as long useMemo light drop the cache, and it makes this hooks more complex.

function useSynchronousEffect(func, values) {
  const lastRef = React.useRef([]);
  const currentValues = React.useMemo(values=> values, values); // value would be reset on hot-update
  let output;
  if(ref.current !== currentValues){
    ref.current = values;
    output = func();
  }

  React.useEffect(
    () => output,
    values, // eslint-disable-line react-hooks/exhaustive-deps
  );
}

[theKashey]

Let me elaborate more on this:

• there are two "data sources" in React, which _should_ not be dropped (to maintain the visible state) - setState, and useRef.
• useMemo, useCallback _should_ be considered as derived data generators and could be rerun at any time.
• useEffect in any form is a side effect with not deterministic execution moment (no execution on a server side for example), thus should be _good to have_.
• constructors and destructors always should be paired.
• only useEffect might have a destructor.

PS: useState theoretically could be updated on HMR, but there is no way to update(not reset) useRef.

Solutions

1. "Key" based

function useSynchronousEffect(func, values) {
  const [key] = useRef({});
  let output;
  // store "generation" key. Just returns a new object every time
  const currentKey = useMemo(() => {}, values); // eslint-disable-line react-hooks/exhaustive-deps
  // "the first render", or "memo dropped the value"
  if(key.current !== currentKey){
    ref.current = currentKey;
    output = func();
  }

  React.useEffect(
    () => output, // there is no need of a complex construction here.
    [currentKey], // eslint-disable-line react-hooks/exhaustive-deps
    // ^ use the same variable here.
  );
}

1. Isomorphic

Use effect on SSR, and switch to useEffect on CSR.

const useSynchronousEffect = isServer ? (effect, props) => effect() : useEffect

The problem - there is no way to detect is this "Server" or "Not". It's not bound to the environment, but to the __react command__ used to kick off the render. Probably it should be

function useSynchronousEffect(func, values) {
  const isServer = React.useContext(IsServer); // But still - who will set this value?
   isServer ? effect : useEffect(effect, values);
}

[oliviertassinari]

I'm looking into it.

[Naismith]

@oliviertassinari Hey Oliver, have you had any chance to look into this yet? This has been bugging me in the current project I am working on.

[theKashey]

I've doublechecked this behavior with upcoming React Fresh, and it would be the same.
So we have to find a solution, compatible with these constraints, but I am not sure that MUI shall do it by their own, as long as any project might need that _Synchronous Effect_, in the same way MUI needs it.

[oliviertassinari]

@theKashey Thanks for checking! I haven't looked into it yet, to be honest, but I hope we can release a v4.1.1 tomorrow with a fix and the TypeScript icon regression fix. This hook logic also has an issue with the Concurrent Mode, the problem is related.

[oliviertassinari]

It's fixed in https://github.com/mui-org/material-ui/pull/16195 and will be soon released in Material-UI v4.1.1.