From f7cc45fc1d98e492db6fa71961c2409ce47ece97 Mon Sep 17 00:00:00 2001 From: Douglas Armstrong Date: Fri, 10 Jul 2020 17:11:05 -0700 Subject: [PATCH] Async Atom effect initialization (#448) Summary: Pull Request resolved: https://github.com/facebookexperimental/Recoil/pull/448 Explore async atoms by adding subset of functionality to initialize an atom via a Promise with an atom effect. This follows the semantics in [RFC Async Atoms](https://fb.quip.com/TUAhAfJI1CfJ). If an atom is initialized to a pending async value, then set to a new value before the promise resolves, the resolved async value is ignored. If the promise is rejected, then the atom will enter an error state and throw as appropriate during render. Reviewed By: csantos42 Differential Revision: D22271167 fbshipit-source-id: 735a6a21882759d68eb70980abb64d10e532921a --- src/core/Recoil_RecoilValueInterface.js | 43 +++++-- src/recoil_values/Recoil_atom.js | 80 ++++++++++--- .../__tests__/Recoil_atom-test.js | 106 +++++++++++++++++- 3 files changed, 202 insertions(+), 27 deletions(-) diff --git a/src/core/Recoil_RecoilValueInterface.js b/src/core/Recoil_RecoilValueInterface.js index b77a39cd5..1f7484298 100644 --- a/src/core/Recoil_RecoilValueInterface.js +++ b/src/core/Recoil_RecoilValueInterface.js @@ -14,7 +14,10 @@ import type {Loadable} from '../adt/Recoil_Loadable'; import type {ValueOrUpdater} from '../recoil_values/Recoil_selector'; import type {AtomValues, Store, TreeState} from './Recoil_State'; -const {mapByDeletingMultipleFromMap} = require('../util/Recoil_CopyOnWrite'); +const { + mapByDeletingFromMap, + mapByDeletingMultipleFromMap, +} = require('../util/Recoil_CopyOnWrite'); const mapMap = require('../util/Recoil_mapMap'); const nullthrows = require('../util/Recoil_nullthrows'); const recoverableViolation = require('../util/Recoil_recoverableViolation'); @@ -110,16 +113,41 @@ function setRecoilValue( store.fireNodeSubscriptions(writtenNodes, 'enqueue'); saveDependencyMapToStore(depMap, store, state.version); - const nonvalidatedAtoms = mapByDeletingMultipleFromMap( - state.nonvalidatedAtoms, - writtenNodes, - ); - return { ...state, dirtyAtoms: unionSets(state.dirtyAtoms, writtenNodes), atomValues: applyAtomValueWrites(state.atomValues, writes), - nonvalidatedAtoms, + nonvalidatedAtoms: mapByDeletingMultipleFromMap( + state.nonvalidatedAtoms, + writtenNodes, + ), + }; + }), + ), + ); +} + +function setRecoilValueLoadable( + store: Store, + recoilValue: AbstractRecoilValue, + loadable: Loadable, +): void { + const {key} = recoilValue; + Tracing.trace('set RecoilValue', key, () => + store.replaceState( + Tracing.wrap(state => { + const writtenNode = new Set([key]); + + store.fireNodeSubscriptions(writtenNode, 'enqueue'); + + return { + ...state, + dirtyAtoms: unionSets(state.dirtyAtoms, writtenNode), + atomValues: applyAtomValueWrites( + state.atomValues, + new Map([[key, loadable]]), + ), + nonvalidatedAtoms: mapByDeletingFromMap(state.nonvalidatedAtoms, key), }; }), ), @@ -184,6 +212,7 @@ module.exports = { RecoilState, getRecoilValueAsLoadable, setRecoilValue, + setRecoilValueLoadable, setUnvalidatedRecoilValue, subscribeToRecoilValue, isRecoilValue, diff --git a/src/recoil_values/Recoil_atom.js b/src/recoil_values/Recoil_atom.js index c276e83d3..dd7376fc6 100644 --- a/src/recoil_values/Recoil_atom.js +++ b/src/recoil_values/Recoil_atom.js @@ -65,14 +65,21 @@ import type {AtomValues, NodeKey, Store, TreeState} from '../core/Recoil_State'; // @fb-only: const {scopedAtom} = require('Recoil_ScopedAtom'); -const {loadableWithValue} = require('../adt/Recoil_Loadable'); +const { + loadableWithError, + loadableWithPromise, + loadableWithValue, +} = require('../adt/Recoil_Loadable'); const { DEFAULT_VALUE, DefaultValue, registerNode, } = require('../core/Recoil_Node'); const {isRecoilValue} = require('../core/Recoil_RecoilValue'); -const {setRecoilValue} = require('../core/Recoil_RecoilValueInterface'); +const { + setRecoilValue, + setRecoilValueLoadable, +} = require('../core/Recoil_RecoilValueInterface'); const deepFreezeValue = require('../util/Recoil_deepFreezeValue'); const expectationViolation = require('../util/Recoil_expectationViolation'); const isPromise = require('../util/Recoil_isPromise'); @@ -100,7 +107,7 @@ export type AtomEffect = ({ // Call synchronously to initialize value or async to change it later setSelf: ( - T | DefaultValue | ((T | DefaultValue) => T | DefaultValue), + T | DefaultValue | Promise | ((T | DefaultValue) => T | DefaultValue), ) => void, resetSelf: () => void, @@ -132,6 +139,25 @@ function baseAtom(options: BaseAtomOptions): RecoilState { | void | [DependencyMap, Loadable] = undefined; + function wrapPendingPromise(store: Store, promise: Promise): Promise { + const wrappedPromise = promise + .then(value => { + const state = store.getState().nextTree ?? store.getState().currentTree; + if (state.atomValues.get(key)?.contents === wrappedPromise) { + setRecoilValue(store, node, value); + } + return value; + }) + .catch(error => { + const state = store.getState().nextTree ?? store.getState().currentTree; + if (state.atomValues.get(key)?.contents === wrappedPromise) { + setRecoilValueLoadable(store, node, loadableWithError(error)); + } + throw error; + }); + return wrappedPromise; + } + function initAtom( store: Store, initState: TreeState, @@ -143,16 +169,18 @@ function baseAtom(options: BaseAtomOptions): RecoilState { store.getState().knownAtoms.add(key); // Run Atom Effects - let initValue: T | DefaultValue = DEFAULT_VALUE; + let initValue: T | DefaultValue | Promise = DEFAULT_VALUE; if (options.effects_UNSTABLE != null) { let duringInit = true; function setSelf( - valueOrUpdater: T | DefaultValue | (T => T | DefaultValue), + valueOrUpdater: T | DefaultValue | Promise | (T => T | DefaultValue), ) { if (duringInit) { const currentValue: T = - initValue instanceof DefaultValue ? options.default : initValue; + initValue instanceof DefaultValue || isPromise(initValue) + ? options.default + : initValue; initValue = typeof valueOrUpdater === 'function' ? // cast to any because we can't restrict type from being a function itself without losing support for opaque types @@ -160,23 +188,34 @@ function baseAtom(options: BaseAtomOptions): RecoilState { (valueOrUpdater: any)(currentValue) : valueOrUpdater; } else { + if (isPromise(valueOrUpdater)) { + throw new Error( + 'Setting atoms to async values is not implemented.', + ); + } setRecoilValue(store, node, valueOrUpdater); } } const resetSelf = () => setSelf(DEFAULT_VALUE); function onSet(handler: (T | DefaultValue, T | DefaultValue) => void) { - store.subscribeToTransactions(asyncStore => { - const state = asyncStore.getState(); + store.subscribeToTransactions(currentStore => { + const state = currentStore.getState(); const nextState = state.nextTree ?? state.currentTree; const prevState = state.currentTree; - const newValue: T | DefaultValue = nextState.atomValues.has(key) - ? nullthrows(nextState.atomValues.get(key)).valueOrThrow() - : DEFAULT_VALUE; - const oldValue: T | DefaultValue = prevState.atomValues.has(key) - ? nullthrows(prevState.atomValues.get(key)).valueOrThrow() - : DEFAULT_VALUE; - handler(newValue, oldValue); + const newLoadable = nextState.atomValues.get(key); + if (newLoadable == null || newLoadable.state === 'hasValue') { + const newValue: T | DefaultValue = + newLoadable != null ? newLoadable.contents : DEFAULT_VALUE; + const oldLoadable = prevState.atomValues.get(key); + const oldValue: T | DefaultValue = + oldLoadable == null + ? options.default + : oldLoadable.state === 'hasValue' + ? oldLoadable.contents + : DEFAULT_VALUE; // TODO This isn't actually valid, use as a placeholder for now. + handler(newValue, oldValue); + } }, key); } @@ -189,9 +228,16 @@ function baseAtom(options: BaseAtomOptions): RecoilState { // Mutate initial state in place since we know there are no other subscribers // since we are the ones initializing on first use. - if (!(initValue instanceof DefaultValue)) { - initState.atomValues.set(key, loadableWithValue(initValue)); + initState.atomValues.set( + key, + isPromise(initValue) + ? // TODO Temp disable Flow due to pending selector_NEW refactor using LoadablePromise + loadableWithPromise( + (wrapPendingPromise(store, initValue): $FlowFixMe), + ) + : loadableWithValue(initValue), + ); } } diff --git a/src/recoil_values/__tests__/Recoil_atom-test.js b/src/recoil_values/__tests__/Recoil_atom-test.js index 0b820bb91..dad432e10 100644 --- a/src/recoil_values/__tests__/Recoil_atom-test.js +++ b/src/recoil_values/__tests__/Recoil_atom-test.js @@ -26,6 +26,7 @@ const { const { ReadsAtom, componentThatReadsAndWritesAtom, + flushPromisesAndTimers, makeStore, renderElements, } = require('../../testing/Recoil_TestingUtils'); @@ -283,6 +284,104 @@ describe('Effects', () => { expect(c.textContent).toEqual(''); }); + test('set promise', async () => { + let resolveAtom; + let validated; + const myAtom = atom({ + key: 'atom effect init set promise', + default: 'DEFAULT', + effects_UNSTABLE: [ + ({setSelf, onSet}) => { + setSelf( + new Promise(resolve => { + resolveAtom = resolve; + }), + ); + onSet(value => { + expect(value).toEqual('RESOLVE'); + validated = true; + }); + }, + ], + }); + + const c = renderElements(); + expect(c.textContent).toEqual('loading'); + + act(() => resolveAtom?.('RESOLVE')); + await flushPromisesAndTimers(); + act(() => undefined); + expect(c.textContent).toEqual('"RESOLVE"'); + expect(validated).toEqual(true); + }); + + // NOTE: This test throws an expected error + test('reject promise', async () => { + let rejectAtom; + let validated = false; + const myAtom = atom({ + key: 'atom effect init reject promise', + default: 'DEFAULT', + effects_UNSTABLE: [ + ({setSelf, onSet}) => { + setSelf( + new Promise((_resolve, reject) => { + rejectAtom = reject; + }), + ); + onSet(() => { + validated = true; + }); + }, + ], + }); + + const c = renderElements(); + expect(c.textContent).toEqual('loading'); + + act(() => rejectAtom?.(new Error('REJECT'))); + await flushPromisesAndTimers(); + act(() => undefined); + expect(c.textContent).toEqual('error'); + expect(validated).toEqual(false); + }); + + test('overwrite promise', async () => { + let resolveAtom; + let validated; + const myAtom = atom({ + key: 'atom effect init overwrite promise', + default: 'DEFAULT', + effects_UNSTABLE: [ + ({setSelf, onSet}) => { + setSelf( + new Promise(resolve => { + resolveAtom = resolve; + }), + ); + onSet(value => { + expect(value).toEqual('OVERWRITE'); + validated = true; + }); + }, + ], + }); + + const [ReadsWritesAtom, setAtom] = componentThatReadsAndWritesAtom(myAtom); + const c = renderElements(); + expect(c.textContent).toEqual('loading'); + + act(() => setAtom('OVERWRITE')); + await flushPromisesAndTimers(); + expect(c.textContent).toEqual('"OVERWRITE"'); + + // Resolving after atom is set to another value will be ignored. + act(() => resolveAtom?.('RESOLVE')); + await flushPromisesAndTimers(); + expect(c.textContent).toEqual('"OVERWRITE"'); + expect(validated).toEqual(true); + }); + test('once per root', () => { let inited = 0; const myAtom = atom({ @@ -315,20 +414,21 @@ describe('Effects', () => { test('onSet', () => { const sets = {a: 0, b: 0}; - const observer = key => newValue => { + const observer = key => (newValue, oldValue) => { + expect(oldValue).toEqual(sets[key]); sets[key]++; expect(newValue).toEqual(sets[key]); }; const atomA = atom({ key: 'atom effect onSet A', - default: 'A', + default: 0, effects_UNSTABLE: [({onSet}) => onSet(observer('a'))], }); const atomB = atom({ key: 'atom effect onSet B', - default: 'B', + default: 0, effects_UNSTABLE: [({onSet}) => onSet(observer('b'))], });