Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't work with react 18.2 #109

Closed
liknenS opened this issue Oct 27, 2023 · 14 comments
Closed

Don't work with react 18.2 #109

liknenS opened this issue Oct 27, 2023 · 14 comments

Comments

@liknenS
Copy link

liknenS commented Oct 27, 2023

Components used useContextSelector with return save value always rerenders.

Example https://codesandbox.io/s/musing-poincare-hz2g6g?file=/src/App.js

@dai-shi
Copy link
Owner

dai-shi commented Oct 27, 2023

That's the intended behavior by design.
Wrapping with useEffect should show more intuitive result:
https://codesandbox.io/s/interesting-mccarthy-m8vzyl?file=/src/App.js

@liknenS
Copy link
Author

liknenS commented Oct 30, 2023

I have more than 1000 components with useContextSeletor. Any changes in context value trigger call render function of all components.

Even if it doesn't cause effects and update the DOM(only call render function), it takes a lot of computing resources.

I change example and add Redux example: https://codesandbox.io/s/muddy-bush-ndqqwg?file=/src/App.js
With 10 components i has next stats:

aRender: 100
aEffect: 21
aChange: 2

bRender: 22
bEffect: 21
bChange: 2

100 components:

aRender: 800
aEffect: 201
aChange: 1

bRender: 202
bEffect: 201
bChange: 1

@liknenS
Copy link
Author

liknenS commented Oct 30, 2023

one more example - symetric updates states + calc render time, 1000 nodes: https://codesandbox.io/s/muddy-bush-ndqqwg?file=/src/App.js

stats:

change: 100

aRender: 206000
aEffect: 2200
aTime: 321.1000003814697

bRender: 2400
bEffect: 2200
bTime: 11.500000953674316

@dai-shi
Copy link
Owner

dai-shi commented Oct 30, 2023

Even if it doesn't cause effects and update the DOM(only call render function), it takes a lot of computing resources.

One workaround is to useMemo.

Another would be to use subscription based state management such as Redux / Zustand / ...

You can also implement a simplified useContextSelector without concurrency support pretty easily.

Seel also #100

@dai-shi
Copy link
Owner

dai-shi commented Oct 30, 2023

import {
  createContext as createContextOrig,
  useContext as useContextOrig,
  useRef,
  useSyncExternalStore,
} from 'react';

export const createContext = (defaultValue) => {
  const context = createContextOrig();
  const ProviderOrig = context.Provider;
  context.Provider = ({ value, children }) => {
    const storeRef = useRef();
    let store = storeRef.current;
    if (!store) {
      const listeners = new Set();
      store = {
        value,
        subscribe: (l) => { listeners.add(l); return () => listeners.delete(l); },
        notify: () => listeners.forEach((l) => l()),
      }
      storeRef.current = store;
    }
    useEffect(() => {
      if (!Object.is(store.value, value)) {
        store.value = value;
        store.notify();
      }
    });
    return <ProviderOrig value={store}>{children}</ProviderOrig>
  };
  return context;
}

export const useContextSelector = (context, selector) => {
  const store = useContextOrig(context);
  return useSyncExternalStore(
    store.subscribe,
    () => selector(store.value),
  );
};

@Jayatubi
Copy link

Jayatubi commented Nov 24, 2023

import {
  createContext as createContextOrig,
  useContext as useContextOrig,
  useRef,
  useSyncExternalStore,
} from 'react';

export const createContext = (defaultValue) => {
  const context = createContextOrig();
  const ProviderOrig = context.Provider;
  context.Provider = ({ value, children }) => {
    const storeRef = useRef();
    let store = storeRef.current;
    if (!store) {
      const listeners = new Set();
      store = {
        value,
        subscribe: (l) => { listeners.add(l); return () => listeners.delete(l); },
        notify: () => listeners.forEach((l) => l()),
      }
      storeRef.current = store;
    }
    useEffect(() => {
      if (!Object.is(store.value, value)) {
        store.value = value;
        store.notify();
      }
    });
    return <ProviderOrig value={store}>{children}</ProviderOrig>
  };
  return context;
}

export const useContextSelector = (context, selector) => {
  const store = useContextOrig(context);
  return useSyncExternalStore(
    store.subscribe,
    () => selector(store.value),
  );
};

This works in React 18.2 and even works with useImmer.

@dai-shi
Copy link
Owner

dai-shi commented Nov 24, 2023

Great to hear that, because I haven't tried it. 😁

@Jayatubi
Copy link

Jayatubi commented Nov 24, 2023

However, useContextSelector could only return a single result. I was tring to get multiple entries such as const { a, b } = useContextSelector(context, (root)=>({a: root.a, b: root.b})) but it cause useSyncExternalStore enter an infinity loop.

@Jayatubi
Copy link

Jayatubi commented Nov 24, 2023

I tried to add a cache with shallowEqual, from redux, to workaround but I'm not sure if that is ok. I'm not familiar to React.

export function useContextSelector<T, Selected>(
  context: React.Context<ContextStore<T>>,
  selector: (value: T) => Selected,
) {
  const store = useContext(context);
  let cache: any;
  return useSyncExternalStore(store.subscribe, () => {
    const value = selector(store.value);
    if (!shallowEqual(cache, value)) {
      cache = value;
    }
    return cache;
  });
}

@dai-shi
Copy link
Owner

dai-shi commented Nov 24, 2023

Use https://www.npmjs.com/package/use-sync-external-store instead and pass shallowEqual.

@vincerubinetti
Copy link

vincerubinetti commented Jan 25, 2025

I've found that the useSyncExternalStore solution above works in React 19, but the package itself does not work in React 18/19. Reading the changelog and various issues around the versions of React, I'm still a little confused whether the current version of the package actually stably works with React 18+.

@vincerubinetti
Copy link

vincerubinetti commented Jan 25, 2025

Here's a cleaned up (imo) version of the (vanilla js) solution above
import {
  createContext,
  useContext,
  useEffect,
  useRef,
  useSyncExternalStore,
} from "react";

export const createContextWithSelectors = (defaultValue) => {
  const context = createContext(defaultValue);
  const OldProvider = context.Provider;
  const NewProvider = ({ value, children }) => {
    const store = useRef();
    const subscribers = useRef(new Set());
    if (!store.current) {
      store.current = {
        value,
        subscribe: (subscriber) => {
          subscribers.current.add(subscriber);
          return () => subscribers.current.delete(subscriber);
        },
      };
    }
    useEffect(() => {
      if (!Object.is(store.value, value)) {
        store.value = value;
        subscribers.current.forEach((subscriber) => subscriber());
      }
    });
    return <OldProvider value={store.current}>{children}</OldProvider>;
  };
  context.Provider = NewProvider;
  return context;
};

export const useContextSelector = (context, selector) => {
  const store = useContext(context);
  return useSyncExternalStore(store.subscribe, () => selector(store.value));
};

Note that defaultValue wasn't being used inside the function, and that listeners/subscribers could just be in its own ref, to not "pollute" the store.

One thing I'm noticing is that if you do something like this:

const { saving } = useContextSelector(SomeContext, (state) => ({ saving: state.saving }));

you get a stack overflow. I guess this is because the object you're returning from the selector is different (referentially) every time. Assuming that's the case, you could probably incorporate something like isEqual from lodash -- in place of Object.is -- to do a deep equality check, but that'd be slow.

Relatedly, @Jayatubi, if you want to check shallowly, wouldn't the place to do it be where the Object.is is? That's where the "hook" is checking for change.

@vincerubinetti
Copy link

vincerubinetti commented Jan 25, 2025

And here is a typescript version. Please let me know if you find issues. Getting rid of the as casts would be great if possible, but I found I had to do them to avoid type errors when passing the value to the provider and when using the selector hook.

import {
  createContext,
  useContext,
  useEffect,
  useRef,
  useSyncExternalStore,
  type Context,
  type Provider,
} from "react";

/** https://github.com/dai-shi/use-context-selector/issues/109 */

type Subscribe = Parameters<typeof useSyncExternalStore>[0];
type Subscriber = () => void;
type Store<Value> = { value: Value; subscribe: Subscribe };

/** useContext that supports selectors */
export const createContextWithSelectors = <Value,>(defaultValue: Value) => {
  /** create default react context */
  const context = createContext<Store<Value>>({
    /** put context value one level deep to allow addition of... */
    value: defaultValue,
    /** subscribe function to access from outside */
    subscribe: () => () => {},
  });

  /** original provider */
  const Original = context.Provider;

  /** provider to replace original one */
  const NewProvider: CallSignature<Provider<Value>> = ({ value, children }) => {
    /** non-reactive store */
    const store = useRef<Store<Value>>(null);

    /** subscribers of store */
    const subscribers = useRef(new Set<Subscriber>());

    /** initialize store */
    if (!store.current)
      store.current = {
        value,
        subscribe: (subscriber) => {
          subscribers.current.add(subscriber);
          return () => subscribers.current.delete(subscriber);
        },
      };

    /** when context value changes */
    useEffect(() => {
      if (!store.current) return;
      /** if new value is referentially different from old value */
      if (!Object.is(store.current.value, value)) {
        /** update value */
        store.current.value = value;
        /** notify subscribers of change */
        subscribers.current.forEach((subscriber) => subscriber());
      }
    }, [value]);

    /** render original provider */
    return <Original value={store.current}>{children}</Original>;
  };

  /** replace old provider with new one */
  context.Provider = NewProvider as Provider<Store<Value>>;

  return context as unknown as ReturnType<typeof createContext<Value>>;
};

/** select slice from context value */
export const useContextSelector = <Value, Slice>(
  context: Context<Value>,
  selector: (value: Value) => Slice,
) => {
  const store = useContext(context) as Store<Value>;
  return useSyncExternalStore<Slice>(store.subscribe, () =>
    selector(store.value),
  );
};

/** https://stackoverflow.com/questions/58657325/typescript-pick-call-signature-from-function-interface */
// eslint-disable-next-line @typescript-eslint/no-explicit-any
type CallSignature<T> = T extends (...args: any[]) => any
  ? (...args: Parameters<T>) => ReturnType<T>
  : never;

And if you then want to make a helper function for a particular context:

type SomeState = { a: 1, b: 2, c:3 };
export const SomeContext = createContextWithSelectors<SomeState>({} as SomeState);
export const useSomeState = <Key extends keyof SomeState>(key: Key) =>
  useContextSelector(SomeContext, (someState) => someState[key]);

useSomeState("a"); // -> 1

(Sorry for the comment spam, wanted to split them up by topic/content).

@dai-shi
Copy link
Owner

dai-shi commented Jan 27, 2025

Can anyone try react18-use #149 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants