Skip to content

Commit

Permalink
useRecoilTransactionObserver hook (#309)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #309

Initial `useRecoilTransactionObserver()` hook based on [this proposal](https://fb.quip.com/UwKaAGjK0L0E)

```
function useRecoilTransactionObserver(
  ({
    snapshot: Snapshot,
    previousSnapshot: Snapshot,
  }) => void,
): void
```

This adds support for selectors that the old `useTransactionObservation()` hook doesn't.  It contains both `snapshot` and `previousSnapshot`, but the snapshots don't yet have a mechanism for iterating the set of atoms/selectors or dirty atoms.  So, keep the old hook for now and migrate users over as we add that functionality.

Also mark `useTransactionSubscription()` as DEPRECATED, we will need to migrate the prototype dev tools support over.

Reviewed By: habond, csantos42

Differential Revision: D21737837

fbshipit-source-id: a9dd99431609e753898edfcb604b38a3508fda52
  • Loading branch information
drarmstr authored and facebook-github-bot committed Jun 16, 2020
1 parent e61871b commit ddae6de
Show file tree
Hide file tree
Showing 6 changed files with 236 additions and 26 deletions.
8 changes: 4 additions & 4 deletions src/Recoil_index.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,13 @@ const {
useRecoilCallback,
useRecoilState,
useRecoilStateLoadable,
useRecoilTransactionObserver,
useRecoilValue,
useRecoilValueLoadable,
useResetRecoilState,
useSetRecoilState,
useSetUnvalidatedAtomValues,
useTransactionObservation,
useTransactionSubscription,
useTransactionObservation_DEPRECATED,
} = require('./hooks/Recoil_Hooks');
const atom = require('./recoil_values/Recoil_atom');
const atomFamily = require('./recoil_values/Recoil_atomFamily');
Expand Down Expand Up @@ -93,8 +93,8 @@ module.exports = {
useRecoilCallback,

// Hooks for Persistence/Debugging
useTransactionObservation_UNSTABLE: useTransactionObservation,
useTransactionSubscription_UNSTABLE: useTransactionSubscription,
useRecoilTransactionObserver,
useTransactionObservation_UNSTABLE: useTransactionObservation_DEPRECATED,
useSetUnvalidatedAtomValues_UNSTABLE: useSetUnvalidatedAtomValues,

// Concurrency Helpers
Expand Down
7 changes: 2 additions & 5 deletions src/components/Recoil_RecoilRoot.react.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ function Batcher(props: {setNotifyBatcherOfChange: (() => void) => void}) {
// call useEffect in an unpredictable order sometimes.
Queue.enqueueExecution('Batcher', () => {
const storeState = storeRef.current.getState();
const {currentTree, nextTree} = storeState;
const {nextTree} = storeState;

// Ignore commits that are not because of Recoil transactions -- namely,
// because something above RecoilRoot re-rendered:
Expand All @@ -89,11 +89,8 @@ function Batcher(props: {setNotifyBatcherOfChange: (() => void) => void}) {
// Inform transaction subscribers of the transaction:
const dirtyAtoms = nextTree.dirtyAtoms;
if (dirtyAtoms.size) {
// NOTE that this passes the committed (current, aka previous) tree,
// whereas the nextTree is retrieved from storeRef by the transaction subscriber.
// (This interface can be cleaned up, TODO)
storeState.transactionSubscriptions.forEach(sub =>
sub(storeRef.current, currentTree),
sub(storeRef.current),
);
}

Expand Down
6 changes: 2 additions & 4 deletions src/core/Recoil_State.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ export type StoreState = {
nextTree: null | TreeState,

// For observing transactions:
+transactionSubscriptions: Map<number, (Store, previous: TreeState) => void>,
+transactionSubscriptions: Map<number, (Store) => void>,

// Callbacks to render external components that are subscribed to nodes
// These are executed at the end of the transaction or asynchronously.
Expand All @@ -68,9 +68,7 @@ export type StoreState = {
export type Store = $ReadOnly<{
getState: () => StoreState,
replaceState: ((TreeState) => TreeState) => void,
subscribeToTransactions: (
(Store, previous: TreeState) => void,
) => {release: () => void, ...},
subscribeToTransactions: ((Store) => void) => {release: () => void},
addTransactionMetadata: ({...}) => void,
fireNodeSubscriptions: (
updatedNodes: $ReadOnlySet<NodeKey>,
Expand Down
55 changes: 44 additions & 11 deletions src/hooks/Recoil_Hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ function useRecoilStateLoadable<T>(
return [value, setValue];
}

function useTransactionSubscription(callback: (Store, TreeState) => void) {
function useTransactionSubscription(callback: Store => void) {
const storeRef = useStoreRef();
useEffect(() => {
const sub = storeRef.current.subscribeToTransactions(callback);
Expand Down Expand Up @@ -430,7 +430,7 @@ type ExternallyVisibleAtomInfo = {
useSetUnvalidatedAtomValues hook. Useful for ignoring the useSetUnvalidatedAtomValues
transaction, to avoid loops.
*/
function useTransactionObservation(
function useTransactionObservation_DEPRECATED(
callback: ({
atomValues: Map<NodeKey, mixed>,
previousAtomValues: Map<NodeKey, mixed>,
Expand All @@ -441,16 +441,18 @@ function useTransactionObservation(
) {
useTransactionSubscription(
useCallback(
(store, previousState) => {
let nextTree = store.getState().nextTree;
if (!nextTree) {
store => {
const previousState = store.getState().currentTree;
let nextState = store.getState().nextTree;
if (!nextState) {
recoverableViolation(
'Transaction subscribers notified without a next tree being present -- this is a bug in Recoil',
'recoil',
);
nextTree = store.getState().currentTree; // attempt to trundle on
nextState = store.getState().currentTree; // attempt to trundle on
}
const atomValues = externallyVisibleAtomValuesInState(nextTree);

const atomValues = externallyVisibleAtomValuesInState(nextState);
const previousAtomValues = externallyVisibleAtomValuesInState(
previousState,
);
Expand All @@ -460,13 +462,43 @@ function useTransactionObservation(
backButton: node.options?.persistence_UNSTABLE?.backButton ?? false,
},
}));
const modifiedAtoms = new Set(nextTree.dirtyAtoms);
const modifiedAtoms = new Set(nextState.dirtyAtoms);

callback({
atomValues,
previousAtomValues,
atomInfo,
modifiedAtoms,
transactionMetadata: {...nextTree.transactionMetadata},
transactionMetadata: {...nextState.transactionMetadata},
});
},
[callback],
),
);
}

function useRecoilTransactionObserver(
callback: ({
snapshot: Snapshot,
previousSnapshot: Snapshot,
}) => void,
) {
useTransactionSubscription(
useCallback(
store => {
const previousState = store.getState().currentTree;
let nextState = store.getState().nextTree;
if (!nextState) {
recoverableViolation(
'Transaction subscribers notified without a next tree being present -- this is a bug in Recoil',
'recoil',
);
nextState = previousState; // attempt to trundle on
}

callback({
snapshot: cloneSnapshot(nextState),
previousSnapshot: cloneSnapshot(previousState),
});
},
[callback],
Expand Down Expand Up @@ -568,9 +600,10 @@ module.exports = {
useSetRecoilState,
useResetRecoilState,
useRecoilInterface: useInterface,
useTransactionSubscription,
useSnapshotWithStateChange,
useTransactionObservation,
useTransactionSubscription_DEPRECATED: useTransactionSubscription,
useTransactionObservation_DEPRECATED,
useRecoilTransactionObserver,
useGoToSnapshot,
useSetUnvalidatedAtomValues,
};
4 changes: 2 additions & 2 deletions src/hooks/__tests__/Recoil_PublicHooks-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ const {
useRecoilValueLoadable,
useSetRecoilState,
useSetUnvalidatedAtomValues,
useTransactionObservation,
useTransactionObservation_DEPRECATED,
} = require('../Recoil_Hooks');

gkx.setPass('recoil_async_selector_refactor');
Expand Down Expand Up @@ -170,7 +170,7 @@ function componentThatToggles(a, b) {
}

function ObservesTransactions({fn}) {
useTransactionObservation(fn);
useTransactionObservation_DEPRECATED(fn);
return null;
}

Expand Down
182 changes: 182 additions & 0 deletions src/hooks/__tests__/Recoil_useRecoilTransactionObserver-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,182 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @emails oncall+perf_viz
* @flow strict-local
* @format
*/
'use strict';

const React = require('React');
const {act} = require('ReactTestUtils');

const atom = require('../../recoil_values/Recoil_atom');
const selector = require('../../recoil_values/Recoil_selector');
const {
asyncSelector,
componentThatReadsAndWritesAtom,
renderElements,
} = require('../../testing/Recoil_TestingUtils');
const {useRecoilTransactionObserver} = require('../Recoil_Hooks');

function TransactionObserver({callback}) {
useRecoilTransactionObserver(callback);
return null;
}

test('Can observe atom value', async () => {
const atomA = atom({
key: 'Observe Atom A',
default: 'DEFAULT A',
});

const atomB = atom({
key: 'Observe Atom B',
default: 'DEFAULT B',
});

const [WriteAtomA, setAtomA, resetA] = componentThatReadsAndWritesAtom(atomA);
const [WriteAtomB, setAtomB] = componentThatReadsAndWritesAtom(atomB);

const transactions = [];
renderElements(
<>
<TransactionObserver
callback={({snapshot, previousSnapshot}) => {
transactions.push({snapshot, previousSnapshot});
}}
/>
<WriteAtomA />
<WriteAtomB />
</>,
);

act(() => setAtomB('SET B'));

expect(transactions.length).toEqual(1);
await expect(transactions[0].snapshot.getPromise(atomA)).resolves.toEqual(
'DEFAULT A',
);
await expect(
transactions[0].previousSnapshot.getPromise(atomA),
).resolves.toEqual('DEFAULT A');
await expect(transactions[0].snapshot.getPromise(atomB)).resolves.toEqual(
'SET B',
);
await expect(
transactions[0].previousSnapshot.getPromise(atomB),
).resolves.toEqual('DEFAULT B');

act(() => setAtomA('SET A'));

expect(transactions.length).toEqual(2);
await expect(transactions[1].snapshot.getPromise(atomA)).resolves.toEqual(
'SET A',
);
await expect(
transactions[1].previousSnapshot.getPromise(atomA),
).resolves.toEqual('DEFAULT A');
await expect(transactions[1].snapshot.getPromise(atomB)).resolves.toEqual(
'SET B',
);
await expect(
transactions[1].previousSnapshot.getPromise(atomB),
).resolves.toEqual('SET B');

act(() => resetA());

expect(transactions.length).toEqual(3);
await expect(transactions[2].snapshot.getPromise(atomA)).resolves.toEqual(
'DEFAULT A',
);
await expect(
transactions[2].previousSnapshot.getPromise(atomA),
).resolves.toEqual('SET A');
await expect(transactions[2].snapshot.getPromise(atomB)).resolves.toEqual(
'SET B',
);
await expect(
transactions[2].previousSnapshot.getPromise(atomB),
).resolves.toEqual('SET B');
});

test('Can observe selector value', async () => {
const atomA = atom({
key: 'Observe Atom for Selector',
default: 'DEFAULT',
});

const selectorA = selector({
key: 'Observer Selector As',
get: ({get}) => `SELECTOR ${get(atomA)}`,
});

const [WriteAtom, setAtom] = componentThatReadsAndWritesAtom(atomA);

const transactions = [];
renderElements(
<>
<TransactionObserver
callback={({snapshot, previousSnapshot}) => {
transactions.push({snapshot, previousSnapshot});
}}
/>
<WriteAtom />
</>,
);

act(() => setAtom('SET'));

expect(transactions.length).toEqual(1);
await expect(transactions[0].snapshot.getPromise(atomA)).resolves.toEqual(
'SET',
);
await expect(
transactions[0].previousSnapshot.getPromise(atomA),
).resolves.toEqual('DEFAULT');
await expect(transactions[0].snapshot.getPromise(selectorA)).resolves.toEqual(
'SELECTOR SET',
);
await expect(
transactions[0].previousSnapshot.getPromise(selectorA),
).resolves.toEqual('SELECTOR DEFAULT');
});

test('Can observe async selector value', async () => {
const atomA = atom({
key: 'Observe Atom for Async Selector',
default: 'DEFAULT',
});
const [WriteAtom, setAtom] = componentThatReadsAndWritesAtom(atomA);

const [selectorA, resolveA] = asyncSelector();

const transactions = [];
renderElements(
<>
<TransactionObserver
callback={({snapshot, previousSnapshot}) => {
transactions.push({snapshot, previousSnapshot});
}}
/>
<WriteAtom />
</>,
);

act(() => setAtom('SET'));

expect(transactions.length).toEqual(1);
expect(transactions[0].snapshot.getLoadable(selectorA).state).toEqual(
'loading',
);

act(() => resolveA('RESOLVE'));

expect(transactions.length).toEqual(1);
await expect(transactions[0].snapshot.getPromise(selectorA)).resolves.toEqual(
'RESOLVE',
);
});

0 comments on commit ddae6de

Please sign in to comment.