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

Request for Backward Compatibility - React.useId and React.useSyncExternalStore #162

Open
kimskovhusandersen opened this issue Jul 26, 2023 · 3 comments

Comments

@kimskovhusandersen
Copy link

kimskovhusandersen commented Jul 26, 2023

Could you kindly consider adding backward compatibility for React.useId and React.useSyncExternalStore?

These hooks were introduced in React 18, but it would be incredibly beneficial for developers still using older React versions to be able to the library without having to upgrade to React 18.

Thank you for your time and consideration. I appreciate the effort you put into making cmdk a great library.

@kimskovhusandersen kimskovhusandersen changed the title Request for Backward Compatibility - React.useId and React.useSyncExternalStore in CMDK Library Request for Backward Compatibility - React.useId and React.useSyncExternalStore Jul 26, 2023
@phsantiago
Copy link
Contributor

phsantiago commented Jul 31, 2023

I'm facing the same problem here. You can monkey patch like this:

import { Command as CommandPrimitive } from 'cmdk';

function is(x: any, y: any) {
  return (
    (x === y && (x !== 0 || 1 / x === 1 / y)) || (x !== x && y !== y) // eslint-disable-line no-self-compare
  );
}
let counter = 0;
(React as any).useId = () => 'cmdk' + (counter++).toString(32);
function checkIfSnapshotChanged<T>(inst: {
  value: T;
  getSnapshot: () => T;
}): boolean {
  const latestGetSnapshot = inst.getSnapshot;
  const prevValue = inst.value;
  try {
    const nextValue = latestGetSnapshot();
    return !is(prevValue, nextValue);
  } catch (error) {
    return true;
  }
}
function useSyncExternalStore<T>(subscribe: any, getSnapshot: () => T): T {
  const value = getSnapshot();
  const [{ inst }, forceUpdate] = React.useState({
    inst: { value, getSnapshot },
  });

  React.useLayoutEffect(() => {
    inst.value = value;
    inst.getSnapshot = getSnapshot;

    if (checkIfSnapshotChanged(inst)) {
      forceUpdate({ inst });
    }
  }, [subscribe, value, getSnapshot]);

  React.useEffect(() => {
    if (checkIfSnapshotChanged(inst)) {
      forceUpdate({ inst });
    }
    const handleStoreChange = () => {
      if (checkIfSnapshotChanged(inst)) {
        forceUpdate({ inst });
      }
    };
    return subscribe(handleStoreChange);
  }, [subscribe]);

  return value;
}
(React as any).useSyncExternalStore = useSyncExternalStore;

I see radix adding compatibility this way:
https://github.com/radix-ui/primitives/blob/eca6babd188df465f64f23f3584738b85dba610e/packages/react/id/src/id.tsx

We can do the same here.

@rafaell-lycan
Copy link

@kimskovhusandersen you can also patch with use-sync-external-store as described above to avoid this problem.

@MateoKruk
Copy link
Contributor

@rafaell-lycan @phsantiago @kimskovhusandersen I created a PR to fix the use of useSyncExternalStore

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