Skip to content

Commit

Permalink
fix: useSyncExternalStoreExtra (facebook#22500)
Browse files Browse the repository at this point in the history
* move setting memoizedSnapshot

* Revert "move setting memoizedSnapshot"

This reverts commit f013206.

* add failed tests

* Revert "Revert "move setting memoizedSnapshot""

This reverts commit cb43c4f.
  • Loading branch information
dai-shi authored and zhengjitf committed Apr 15, 2022
1 parent 258952a commit d1088fe
Show file tree
Hide file tree
Showing 2 changed files with 90 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -818,4 +818,93 @@ describe('Shared useSyncExternalStore behavior (shim and built-in)', () => {
'Sibling: 1',
]);
});

describe('selector and isEqual error handling in extra', () => {
let ErrorBoundary;
beforeAll(() => {
spyOnDev(console, 'warn');
ErrorBoundary = class extends React.Component {
state = {error: null};
static getDerivedStateFromError(error) {
return {error};
}
render() {
if (this.state.error) {
return <Text text={this.state.error.message} />;
}
return this.props.children;
}
};
});

it('selector can throw on update', async () => {
const store = createExternalStore({a: 'a'});
const selector = state => state.a.toUpperCase();

function App() {
const a = useSyncExternalStoreExtra(
store.subscribe,
store.getState,
null,
selector,
);
return <Text text={a} />;
}

const container = document.createElement('div');
const root = createRoot(container);
await act(() =>
root.render(
<ErrorBoundary>
<App />
</ErrorBoundary>,
),
);

expect(container.textContent).toEqual('A');

await act(() => {
store.set({});
});
expect(container.textContent).toEqual(
"Cannot read property 'toUpperCase' of undefined",
);
});

it('isEqual can throw on update', async () => {
const store = createExternalStore({a: 'A'});
const selector = state => state.a;
const isEqual = (left, right) => left.a.trim() === right.a.trim();

function App() {
const a = useSyncExternalStoreExtra(
store.subscribe,
store.getState,
null,
selector,
isEqual,
);
return <Text text={a} />;
}

const container = document.createElement('div');
const root = createRoot(container);
await act(() =>
root.render(
<ErrorBoundary>
<App />
</ErrorBoundary>,
),
);

expect(container.textContent).toEqual('A');

await act(() => {
store.set({});
});
expect(container.textContent).toEqual(
"Cannot read property 'trim' of undefined",
);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ export function useSyncExternalStoreExtra<Snapshot, Selection>(
}

// The snapshot has changed, so we need to compute a new selection.
memoizedSnapshot = nextSnapshot;
const nextSelection = selector(nextSnapshot);

// If a custom isEqual function is provided, use that to check if the data
Expand All @@ -87,6 +86,7 @@ export function useSyncExternalStoreExtra<Snapshot, Selection>(
return prevSelection;
}

memoizedSnapshot = nextSnapshot;
memoizedSelection = nextSelection;
return nextSelection;
};
Expand Down

0 comments on commit d1088fe

Please sign in to comment.