diff --git a/packages/core/src/createDynamicSelector.ts b/packages/core/src/createDynamicSelector.ts index 2ace79b..b6519d4 100644 --- a/packages/core/src/createDynamicSelector.ts +++ b/packages/core/src/createDynamicSelector.ts @@ -31,7 +31,7 @@ const createDefaultCache = (): DynamicSelectorResultCache => { const defaultSelectorOptions: DynamicSelectorOptions = { compareResult: shallowEqual, getKeyForParams: JSON.stringify, - onException: null, + onError: null, createResultCache: createDefaultCache, }; diff --git a/packages/core/src/dynamicSelectorForState.ts b/packages/core/src/dynamicSelectorForState.ts index 6370420..cd5f7ba 100644 --- a/packages/core/src/dynamicSelectorForState.ts +++ b/packages/core/src/dynamicSelectorForState.ts @@ -80,7 +80,7 @@ const dynamicSelectorForState = ( debug, displayName, getKeyForParams, - onException, + onError, } = options ? { ...defaultSelectorOptions, ...options } : defaultSelectorOptions; const resultCache: DynamicSelectorResultCache = createResultCache(); @@ -209,11 +209,12 @@ const dynamicSelectorForState = ( nextResult[RESULT_ENTRY__HAS_RETURN_VALUE] = true; } catch (e) { nextResult[RESULT_ENTRY__ERROR] = e; + debugAbortedRun(debugInfo); - if (onException) { - // If the onException callback returns anything, we'll use that as the return value -- but we still + if (onError) { + // If the onError callback returns anything, we'll use that as the return value -- but we still // track the error. - nextResult[RESULT_ENTRY__RETURN_VALUE] = onException( + nextResult[RESULT_ENTRY__RETURN_VALUE] = onError( e, [state, params, ...otherArgs], outerFn, @@ -226,9 +227,7 @@ const dynamicSelectorForState = ( popCallStackEntry(); // We were able to run without error -- but is our "new" result actually new? - if (!nextResult[RESULT_ENTRY__HAS_RETURN_VALUE]) { - debugAbortedRun(debugInfo); - } else if ( + if ( compareResult && previousResult && previousResult[RESULT_ENTRY__HAS_RETURN_VALUE] && @@ -239,10 +238,12 @@ const dynamicSelectorForState = ( ) ) { canUsePreviousResult = true; - debugPhantomRun(debugInfo); + if (!nextResult[RESULT_ENTRY__ERROR]) { + debugPhantomRun(debugInfo); + } // Carry over the *exact* return value we had before nextResult[RESULT_ENTRY__RETURN_VALUE] = previousResult[RESULT_ENTRY__RETURN_VALUE]; - } else { + } else if (!nextResult[RESULT_ENTRY__ERROR]) { debugFullRun(debugInfo); } } @@ -253,7 +254,12 @@ const dynamicSelectorForState = ( // We still need to register this selectorFn as a dependency of the parent (if any). if (recordDependencies) { parentCaller[RESULT_ENTRY__CALL_DEPENDENCIES].push( - createCallDependency(outerFn, params, nextResult[RESULT_ENTRY__RETURN_VALUE]), + createCallDependency( + outerFn, + params, + nextResult[RESULT_ENTRY__RETURN_VALUE], + !allowExecution, + ), ); } @@ -283,7 +289,7 @@ const dynamicSelectorForState = ( popCallStackEntry(); } - // It's okay to have *both* an error *and* a return value (although that's rare: it only happens if onException + // It's okay to have *both* an error *and* a return value (although that's rare: it only happens if onError // returned a value) if (!result[RESULT_ENTRY__HAS_RETURN_VALUE] && result[RESULT_ENTRY__ERROR]) { throw result[RESULT_ENTRY__ERROR]; @@ -298,8 +304,10 @@ const dynamicSelectorForState = ( * DO NOT USE THIS. * This lets selectors bypass the wrappers internally, when appropriate. It shouldn't be called from outside of * this file, except in tests. + * + * "dc" = "DepCheck" */ - outerFn._callDirect = evaluateSelector; + outerFn._dc = evaluateSelector; /** * DO NOT USE THIS. @@ -322,22 +330,25 @@ const dynamicSelectorForState = ( const evaluateSelectorReadOnly = ( args: DynamicSelectorArgsWithState | DynamicSelectorArgsWithoutState, ): DynamicSelectorResultEntry => { - const parentCaller = getTopCallStackEntry(); const argsWithState = addStateToArguments(args); - const rootResult = createResultEntry(stateOptions, argsWithState[0], false, false); - if (parentCaller && parentCaller[RESULT_ENTRY__RECORD_DEPENDENCIES]) { - // If our parent is checking our cache result, record our dependencies onto the parent instead - rootResult[RESULT_ENTRY__RECORD_DEPENDENCIES] = true; - rootResult[RESULT_ENTRY__STATE_DEPENDENCIES] = - parentCaller[RESULT_ENTRY__STATE_DEPENDENCIES]; - rootResult[RESULT_ENTRY__CALL_DEPENDENCIES] = parentCaller[RESULT_ENTRY__CALL_DEPENDENCIES]; - } pushCallStackEntry(rootResult); const result = evaluateSelector(...argsWithState); popCallStackEntry(); + const parentCaller = getTopCallStackEntry(); + if (parentCaller && parentCaller[RESULT_ENTRY__RECORD_DEPENDENCIES]) { + parentCaller[RESULT_ENTRY__CALL_DEPENDENCIES].push( + createCallDependency( + outerFn, + argsWithState[1], + result[RESULT_ENTRY__RETURN_VALUE], + false, + ), + ); + } + return result; }; diff --git a/packages/core/src/internals/debugInfo.ts b/packages/core/src/internals/debugInfo.ts index adc69a2..1efab31 100644 --- a/packages/core/src/internals/debugInfo.ts +++ b/packages/core/src/internals/debugInfo.ts @@ -1,3 +1,5 @@ +/* istanbul ignore file */ + // Because this is ONLY used in dev mode, it's stored as a normal object instead of an array export type DynamicSelectorDebugInfo = { _verbose?: boolean | string; diff --git a/packages/core/src/internals/dependencies.ts b/packages/core/src/internals/dependencies.ts index 59fd4f8..700be5c 100644 --- a/packages/core/src/internals/dependencies.ts +++ b/packages/core/src/internals/dependencies.ts @@ -22,18 +22,22 @@ export type DynamicSelectorCallDependency = [ DynamicSelectorParams, /* returnValue */ any, + /* isReadOnly */ + boolean, ]; // These keys just make the CallDependency code easier to read export const CALL_DEPENDENCY__SELECTOR_FN = 0 as const; export const CALL_DEPENDENCY__PARAMS = 1 as const; export const CALL_DEPENDENCY__RETURN_VALUE = 2 as const; +export const CALL_DEPENDENCY__IS_READONLY = 3 as const; const createCallDependency = ( selectorFn: DynamicSelectorFn, params: DynamicSelectorParams, returnValue: any, -): DynamicSelectorCallDependency => [selectorFn, params, returnValue]; + isReadOnly: boolean, +): DynamicSelectorCallDependency => [selectorFn, params, returnValue, isReadOnly]; const hasAnyStateDependencyChanged = ( getFn: DynamicSelectorStateGetFn, @@ -73,6 +77,7 @@ const hasAnyCallDependencyChanged = ( dependencySelectorFn, dependencyParams, dependencyReturnValue, + dependencyIsReadOnly, ] = previousCallDependencies[i]; /* istanbul ignore next */ @@ -81,16 +86,34 @@ const hasAnyCallDependencyChanged = ( const checkType_selectorFn: DynamicSelectorCallDependency[typeof CALL_DEPENDENCY__SELECTOR_FN] = dependencySelectorFn; const checkType_params: DynamicSelectorCallDependency[typeof CALL_DEPENDENCY__PARAMS] = dependencyParams; const checkType_dependencyReturnValue: DynamicSelectorCallDependency[typeof CALL_DEPENDENCY__RETURN_VALUE] = dependencyReturnValue; - console.log({ checkType_selectorFn, checkType_params, checkType_dependencyReturnValue }); + const checkType_dependencyIsReadOnly: DynamicSelectorCallDependency[typeof CALL_DEPENDENCY__IS_READONLY] = dependencyIsReadOnly; + console.log({ + checkType_selectorFn, + checkType_params, + checkType_dependencyReturnValue, + checkType_dependencyIsReadOnly, + }); } - // Does our dependency have anything new? Let's run it to find out. - const result = dependencySelectorFn._callDirect(state, dependencyParams, ...otherArgs); - const hasReturnValue = result[RESULT_ENTRY__HAS_RETURN_VALUE]; - const newReturnValue = result[RESULT_ENTRY__RETURN_VALUE]; + // Does our dependency have anything new? + const temporarilyBlockExecution = dependencyIsReadOnly && allowExecution; + if (temporarilyBlockExecution) { + // Temporarily add *another* dummy entry to block execution + pushCallStackEntry(createDepCheckEntry(false)); + } + + // Run it and check the result + const result = dependencySelectorFn._dc(state, dependencyParams, ...otherArgs); + + if (temporarilyBlockExecution) { + popCallStackEntry(); + } - if (!hasReturnValue || newReturnValue !== dependencyReturnValue) { - // Something either failed to return a value, or it returned something new. + if ( + !result[RESULT_ENTRY__HAS_RETURN_VALUE] || + result[RESULT_ENTRY__RETURN_VALUE] !== dependencyReturnValue + ) { + // It either failed to return a value, or it returned something new. // (We use strict equality -- not compareResult -- because compareResult was already used to decide whether to // return the exact prior value) popCallStackEntry(); diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index 95ecb81..e6cc2e2 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -38,10 +38,10 @@ export type DynamicSelectorOptions = { /* Generates a unique ID for the selector's params */ getKeyForParams: (params?: DynamicSelectorParams) => string; /* Called if the selector function throws an exception */ - onException: + onError: | (( error: Error, - args: DynamicSelectorArgsWithState, + args: [StateType, DynamicSelectorParams | any, ...Array], selectorFn: DynamicSelectorFn, ) => void) | null; @@ -97,7 +97,7 @@ export type DynamicSelectorFn = (( ...args: DynamicSelectorArgsWithState | DynamicSelectorArgsWithoutState ) => ReturnType) & { _innerFn: DynamicSelectorInnerFn; - _callDirect: DynamicSelectorFnWithState; + _dc: DynamicSelectorFnWithState; _resultCache: DynamicSelectorResultCache; displayName: string; getDebugInfo: (params?: DynamicSelectorParams) => DynamicSelectorDebugInfo; diff --git a/packages/core/tests/hasCachedResults.test.ts b/packages/core/tests/cachedResults.test.ts similarity index 100% rename from packages/core/tests/hasCachedResults.test.ts rename to packages/core/tests/cachedResults.test.ts diff --git a/packages/core/tests/exceptions.test.ts b/packages/core/tests/exceptions.test.ts new file mode 100644 index 0000000..e6caf2d --- /dev/null +++ b/packages/core/tests/exceptions.test.ts @@ -0,0 +1,112 @@ +import { createDynamicSelector } from '../src'; +import DebugInfoCheckUtil from './util/debugInfoCheckUtil'; + +describe('exceptions', () => { + test('throws exceptions when uncaught', () => { + const childSelector = createDynamicSelector((getState) => { + return getState('a'); + }); + const parentSelector = createDynamicSelector( + (_getState, { multiplier }: { multiplier: number }) => { + const result = childSelector() * multiplier; + if (result % 2) { + throw new Error('Odd number!'); + } + return result; + }, + ); + + const childSelectorCheck = new DebugInfoCheckUtil(childSelector); + const parentSelectorCheck10 = new DebugInfoCheckUtil(parentSelector, { multiplier: 10 }); + const parentSelectorCheck11 = new DebugInfoCheckUtil(parentSelector, { multiplier: 11 }); + + let state = { a: 1 }; + + expect(parentSelector(state, { multiplier: 10 })).toEqual(10); + parentSelectorCheck10.expectInvoked('run'); + childSelectorCheck.expectInvoked('run'); + + expect(() => parentSelector(state, { multiplier: 11 })).toThrowError('Odd number!'); + parentSelectorCheck11.expectInvoked('aborted'); + childSelectorCheck.expectInvoked('skipped'); + + // '10' and child are still cached, '11' is not + expect(parentSelector.hasCachedResult(state, { multiplier: 10 })).toEqual(true); + expect(parentSelector.getCachedResult(state, { multiplier: 10 })).toEqual(10); + expect(childSelector.hasCachedResult(state)).toEqual(true); + }); + + test('catch exception thrown by child', () => { + const childSelector = createDynamicSelector((getState) => { + const result = getState('a'); + if (result % 2) { + throw new Error('Odd number!'); + } + return result; + }); + const parentSelector = createDynamicSelector( + (_getState, { multiplier }: { multiplier: number }) => { + let result = 0; + try { + result = childSelector() * multiplier; + } catch (e) {} + + return result; + }, + ); + + let state = { a: 2 }; + + expect(parentSelector(state, { multiplier: 10 })).toEqual(20); + + state = { a: 3 }; + + expect(parentSelector(state, { multiplier: 10 })).toEqual(0); + // Parent is cached, child is not + expect(parentSelector.hasCachedResult(state, { multiplier: 10 })).toEqual(true); + expect(parentSelector.getCachedResult(state, { multiplier: 10 })).toEqual(0); + expect(childSelector.hasCachedResult(state)).toEqual(false); + }); + + test('parent overrides exceptions with onError', () => { + const childSelector = createDynamicSelector((getState) => { + return getState('a'); + }); + const parentSelector = createDynamicSelector( + (_getState, { multiplier }: { multiplier: number }) => { + const result = childSelector() * multiplier; + if (result % 2) { + throw new Error('Odd number!'); + } + return result; + }, + { + onError: (_error, [_state, { multiplier }]) => { + // Silly recovery: use the multiplier as the return value + return multiplier; + }, + }, + ); + + const childSelectorCheck = new DebugInfoCheckUtil(childSelector); + const parentSelectorCheck10 = new DebugInfoCheckUtil(parentSelector, { multiplier: 10 }); + const parentSelectorCheck11 = new DebugInfoCheckUtil(parentSelector, { multiplier: 11 }); + + let state = { a: 3 }; + + expect(parentSelector(state, { multiplier: 10 })).toEqual(30); + parentSelectorCheck10.expectInvoked('run'); + childSelectorCheck.expectInvoked('run'); + + expect(parentSelector(state, { multiplier: 11 })).toEqual(11); + parentSelectorCheck11.expectInvoked('aborted'); + childSelectorCheck.expectInvoked('skipped'); + + // Since we overrode the value, everything is cached + expect(parentSelector.hasCachedResult(state, { multiplier: 10 })).toEqual(true); + expect(parentSelector.getCachedResult(state, { multiplier: 10 })).toEqual(30); + expect(parentSelector.hasCachedResult(state, { multiplier: 11 })).toEqual(true); + expect(parentSelector.getCachedResult(state, { multiplier: 11 })).toEqual(11); + expect(childSelector.hasCachedResult(state)).toEqual(true); + }); +});