Skip to content

Commit

Permalink
Merge 6e6623e into 0fee56d
Browse files Browse the repository at this point in the history
  • Loading branch information
spautz committed Oct 14, 2020
2 parents 0fee56d + 6e6623e commit e88e74c
Show file tree
Hide file tree
Showing 7 changed files with 181 additions and 33 deletions.
2 changes: 1 addition & 1 deletion packages/core/src/createDynamicSelector.ts
Expand Up @@ -31,7 +31,7 @@ const createDefaultCache = (): DynamicSelectorResultCache => {
const defaultSelectorOptions: DynamicSelectorOptions = {
compareResult: shallowEqual,
getKeyForParams: JSON.stringify,
onException: null,
onError: null,
createResultCache: createDefaultCache,
};

Expand Down
53 changes: 32 additions & 21 deletions packages/core/src/dynamicSelectorForState.ts
Expand Up @@ -80,7 +80,7 @@ const dynamicSelectorForState = <StateType = any>(
debug,
displayName,
getKeyForParams,
onException,
onError,
} = options ? { ...defaultSelectorOptions, ...options } : defaultSelectorOptions;

const resultCache: DynamicSelectorResultCache = createResultCache();
Expand Down Expand Up @@ -209,11 +209,12 @@ const dynamicSelectorForState = <StateType = any>(
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,
Expand All @@ -226,9 +227,7 @@ const dynamicSelectorForState = <StateType = any>(
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] &&
Expand All @@ -239,10 +238,12 @@ const dynamicSelectorForState = <StateType = any>(
)
) {
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);
}
}
Expand All @@ -253,7 +254,12 @@ const dynamicSelectorForState = <StateType = any>(
// 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,
),
);
}

Expand Down Expand Up @@ -283,7 +289,7 @@ const dynamicSelectorForState = <StateType = any>(
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];
Expand All @@ -298,8 +304,10 @@ const dynamicSelectorForState = <StateType = any>(
* 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.
Expand All @@ -322,22 +330,25 @@ const dynamicSelectorForState = <StateType = any>(
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;
};

Expand Down
2 changes: 2 additions & 0 deletions 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;
Expand Down
39 changes: 31 additions & 8 deletions packages/core/src/internals/dependencies.ts
Expand Up @@ -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,
Expand Down Expand Up @@ -73,6 +77,7 @@ const hasAnyCallDependencyChanged = (
dependencySelectorFn,
dependencyParams,
dependencyReturnValue,
dependencyIsReadOnly,
] = previousCallDependencies[i];

/* istanbul ignore next */
Expand All @@ -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();
Expand Down
6 changes: 3 additions & 3 deletions packages/core/src/types.ts
Expand Up @@ -38,10 +38,10 @@ export type DynamicSelectorOptions<ReturnType = any, StateType = any> = {
/* 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<StateType>,
args: [StateType, DynamicSelectorParams | any, ...Array<any>],
selectorFn: DynamicSelectorFn,
) => void)
| null;
Expand Down Expand Up @@ -97,7 +97,7 @@ export type DynamicSelectorFn<ReturnType = any> = ((
...args: DynamicSelectorArgsWithState | DynamicSelectorArgsWithoutState
) => ReturnType) & {
_innerFn: DynamicSelectorInnerFn<ReturnType>;
_callDirect: DynamicSelectorFnWithState<ReturnType>;
_dc: DynamicSelectorFnWithState<ReturnType>;
_resultCache: DynamicSelectorResultCache;
displayName: string;
getDebugInfo: (params?: DynamicSelectorParams) => DynamicSelectorDebugInfo;
Expand Down
File renamed without changes.
112 changes: 112 additions & 0 deletions 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);
});
});

0 comments on commit e88e74c

Please sign in to comment.