Skip to content

Commit

Permalink
Fixed an issue with useSelector always computing fresh snapshots in…
Browse files Browse the repository at this point in the history
…ternally for uninitialized services (#3500)

* Fixed an issue with `useSelector` always computing fresh snapshots internally for uninitialized services

* Update packages/xstate-react/src/useSelector.ts

Co-authored-by: David Khourshid <davidkpiano@gmail.com>

Co-authored-by: David Khourshid <davidkpiano@gmail.com>
  • Loading branch information
Andarist and davidkpiano committed Aug 5, 2022
1 parent a6c9b6d commit 0dfc6d9
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 13 deletions.
5 changes: 5 additions & 0 deletions .changeset/weak-singers-jog.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@xstate/react': patch
---

Fixed an issue with `useSelector` always computing fresh snapshots internally for uninitialized services. This avoids the internal `useSyncExternalStore` from warning about the snapshot value not being cached properly.
36 changes: 23 additions & 13 deletions packages/xstate-react/src/useSelector.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { useCallback } from 'react';
import { useCallback, useRef } from 'react';
import { useSyncExternalStoreWithSelector } from 'use-sync-external-store/shim/with-selector';
import { ActorRef, Interpreter, Subscribable } from 'xstate';
import { ActorRef, AnyState, Interpreter, Subscribable } from 'xstate';
import { isActorWithState } from './useActor';
import { getServiceSnapshot } from './utils';

Expand All @@ -9,12 +9,18 @@ function isService(actor: any): actor is Interpreter<any, any, any, any> {
}

const defaultCompare = (a, b) => a === b;
const defaultGetSnapshot = (a) =>
isService(a)
? getServiceSnapshot(a)
: isActorWithState(a)
? a.state
: undefined;
const defaultGetSnapshot = (a, initialStateCacheRef) => {
if (isService(a)) {
// A status of 0 = interpreter not started
if (a.status === 0 && initialStateCacheRef.current) {
return initialStateCacheRef.current;
}
const snapshot = getServiceSnapshot(a);
initialStateCacheRef.current = a.status === 0 ? snapshot : null;
return snapshot;
}
return isActorWithState(a) ? a.state : undefined;
};

export function useSelector<
TActor extends ActorRef<any, any>,
Expand All @@ -24,8 +30,10 @@ export function useSelector<
actor: TActor,
selector: (emitted: TEmitted) => T,
compare: (a: T, b: T) => boolean = defaultCompare,
getSnapshot: (a: TActor) => TEmitted = defaultGetSnapshot
getSnapshot?: (a: TActor) => TEmitted
): T {
const initialStateCacheRef = useRef<AnyState | null>(null);

const subscribe = useCallback(
(handleStoreChange) => {
const { unsubscribe } = actor.subscribe(handleStoreChange);
Expand All @@ -34,10 +42,12 @@ export function useSelector<
[actor]
);

const boundGetSnapshot = useCallback(() => getSnapshot(actor), [
actor,
getSnapshot
]);
const boundGetSnapshot = useCallback(() => {
if (getSnapshot) {
return getSnapshot(actor);
}
return defaultGetSnapshot(actor, initialStateCacheRef);
}, [actor, getSnapshot]);

const selectedSnapshot = useSyncExternalStoreWithSelector(
subscribe,
Expand Down
35 changes: 35 additions & 0 deletions packages/xstate-react/test/useSelector.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { act, fireEvent, screen } from '@testing-library/react';
import * as React from 'react';
import {
ActorRefFrom,
AnyState,
assign,
createMachine,
interpret,
Expand All @@ -12,6 +13,12 @@ import {
import { shallowEqual, useInterpret, useMachine, useSelector } from '../src';
import { describeEachReactMode } from './utils';

const originalConsoleError = console.error;

afterEach(() => {
console.error = originalConsoleError;
});

describeEachReactMode('useSelector (%s)', ({ suiteKey, render }) => {
it('only rerenders for selected values', () => {
const machine = createMachine<{ count: number; other: number }>({
Expand Down Expand Up @@ -564,4 +571,32 @@ describeEachReactMode('useSelector (%s)', ({ suiteKey, render }) => {

expect(renders).toBe(suiteKey === 'strict' ? 2 : 1);
});

it('should compute a stable snapshot internally when selecting from uninitialized service', () => {
const child = createMachine({});
const machine = createMachine({
invoke: {
id: 'child',
src: child
}
});

const snapshots: AnyState[] = [];

function App() {
const service = useInterpret(machine);
useSelector(service, (state) => {
snapshots.push(state);
return state.children.child;
});
return null;
}

console.error = jest.fn();
render(<App />);

const [snapshot1] = snapshots;
expect(snapshots.every((s) => s === snapshot1));
expect(console.error).toHaveBeenCalledTimes(0);
});
});

0 comments on commit 0dfc6d9

Please sign in to comment.