Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(vow): watcher args instead of context #9556

Merged
merged 5 commits into from
Jun 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions packages/vow/src/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ export {};
* @template [T=any]
* @template [TResult1=T]
* @template [TResult2=never]
* @template [C=any] watcher context
* @template {any[]} [C=any[]] watcher args
* @typedef {object} Watcher
* @property {(value: T, context?: C) => Vow<TResult1> | PromiseVow<TResult1> | TResult1} [onFulfilled]
* @property {(reason: any) => Vow<TResult2> | PromiseVow<TResult2> | TResult2} [onRejected]
* @property {(value: T, ...args: C) => Vow<TResult1> | PromiseVow<TResult1> | TResult1} [onFulfilled]
* @property {(reason: any, ...args: C) => Vow<TResult2> | PromiseVow<TResult2> | TResult2} [onRejected]
*/
2 changes: 1 addition & 1 deletion packages/vow/src/watch-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ export const prepareWatchUtils = (zone, watch, makeVowKit) => {
}
resolver.resolve(harden(results));
},
onRejected(value, { id }) {
onRejected(value, { id, index: _index }) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stray?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope it's there to satisfy typescript, in particular

watch(vow, this.facets.watcher, { id, index });

Now that this PR correctly plumbs the args in the rejection handler in the type definitions, the extra property check of TS kicked in with the type of the args which were inferred to the narrower / common part of the fulfillment and rejection handler. Maybe there is a better way to type watch to avoid this gotcha?

const { idToVowState } = this.state;
if (!idToVowState.has(id)) {
// First rejection wins.
Expand Down
34 changes: 15 additions & 19 deletions packages/vow/src/watch.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,14 @@ const makeWatchNextStep =
* @param {Watcher<unknown, unknown, unknown> | undefined} watcher
* @param {keyof Required<Watcher>} wcb
* @param {unknown} value
* @param {unknown} [watcherContext]
* @param {unknown[]} [watcherArgs]
*/
const settle = (resolver, watcher, wcb, value, watcherContext) => {
const settle = (resolver, watcher, wcb, value, watcherArgs = []) => {
try {
let chainedValue = value;
const w = watcher && watcher[wcb];
if (w) {
chainedValue = apply(w, watcher, [value, watcherContext]);
chainedValue = apply(w, watcher, [value, ...watcherArgs]);
} else if (wcb === 'onRejected') {
throw value;
}
Expand Down Expand Up @@ -75,22 +75,22 @@ const preparePromiseWatcher = (zone, isRetryableReason, watchNextStep) =>
* @template [TResult2=never]
* @param {VowResolver<TResult1 | TResult2>} resolver
* @param {Watcher<T, TResult1, TResult2>} [watcher]
* @param {unknown} [watcherContext]
* @param {unknown[]} [watcherArgs]
*/
(resolver, watcher, watcherContext) => {
(resolver, watcher, watcherArgs) => {
const state = {
vow: /** @type {unknown} */ (undefined),
priorRetryValue: /** @type {any} */ (undefined),
resolver,
watcher,
watcherContext: harden(watcherContext),
watcherArgs: harden(watcherArgs),
};
return /** @type {Partial<typeof state>} */ (state);
},
{
/** @type {Required<PromiseWatcher>['onFulfilled']} */
onFulfilled(value) {
const { watcher, watcherContext, resolver } = this.state;
const { watcher, watcherArgs, resolver } = this.state;
if (getVowPayload(value)) {
// We've been shortened, so reflect our state accordingly, and go again.
this.state.vow = value;
Expand All @@ -100,11 +100,11 @@ const preparePromiseWatcher = (zone, isRetryableReason, watchNextStep) =>
this.state.priorRetryValue = undefined;
this.state.watcher = undefined;
this.state.resolver = undefined;
settle(resolver, watcher, 'onFulfilled', value, watcherContext);
settle(resolver, watcher, 'onFulfilled', value, watcherArgs);
},
/** @type {Required<PromiseWatcher>['onRejected']} */
onRejected(reason) {
const { vow, watcher, watcherContext, resolver, priorRetryValue } =
const { vow, watcher, watcherArgs, resolver, priorRetryValue } =
this.state;
if (vow) {
const retryValue = isRetryableReason(reason, priorRetryValue);
Expand All @@ -118,7 +118,7 @@ const preparePromiseWatcher = (zone, isRetryableReason, watchNextStep) =>
this.state.priorRetryValue = undefined;
this.state.resolver = undefined;
this.state.watcher = undefined;
settle(resolver, watcher, 'onRejected', reason, watcherContext);
settle(resolver, watcher, 'onRejected', reason, watcherArgs);
},
},
);
Expand All @@ -144,24 +144,20 @@ export const prepareWatch = (
* @template [T=any]
* @template [TResult1=T]
* @template [TResult2=never]
* @template [C=any] watcher context
* @template {any[]} [C=any[]] watcher args
* @param {ERef<T | Vow<T>>} specimenP
* @param {Watcher<T, TResult1, TResult2>} [watcher]
* @param {C} [watcherContext]
* @param {Watcher<T, TResult1, TResult2, C>} [watcher]
* @param {C} watcherArgs
*/
const watch = (specimenP, watcher, watcherContext) => {
const watch = (specimenP, watcher, ...watcherArgs) => {
/** @typedef {Exclude<TResult1, void> | Exclude<TResult2, void>} Voidless */
/** @typedef {Voidless extends never ? TResult1 : Voidless} Narrowest */
/** @type {VowKit<Narrowest>} */
const { resolver, vow } = makeVowKit();

// Create a promise watcher to track vows, retrying upon rejection as
// controlled by `isRetryableReason`.
const promiseWatcher = makePromiseWatcher(
resolver,
watcher,
watcherContext,
);
const promiseWatcher = makePromiseWatcher(resolver, watcher, watcherArgs);

// Coerce the specimen to a promise, and start the watcher cycle.
zone.watchPromise(basicE.resolve(specimenP), promiseWatcher);
Expand Down
92 changes: 88 additions & 4 deletions packages/vow/test/watch.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,28 @@ const prepareAckWatcher = (zone, t) => {
});
};

/**
* @param {Zone} zone
* @param {ExecutionContext<unknown>} t
*/
const prepareArityCheckWatcher = (zone, t) => {
return zone.exoClass(
'ArityCheckWatcher',
undefined,
expectedArgs => ({ expectedArgs }),
{
onFulfilled(value, ...args) {
t.deepEqual(args, this.state.expectedArgs);
return 'fulfilled';
},
onRejected(reason, ...args) {
t.deepEqual(args, this.state.expectedArgs);
return 'rejected';
},
},
);
};

/**
* @param {Zone} zone
* @param {ExecutionContext<unknown>} t
Expand Down Expand Up @@ -79,14 +101,76 @@ test('ack watcher - shim', async t => {
resolver3.reject(Error('disco2'));
resolver3.resolve(vow2);
t.is(
await when(
// @ts-expect-error intentional extra argument
watch(connVow3P, makeAckWatcher(packet), 'watcher context', 'unexpected'),
),
await when(watch(connVow3P, makeAckWatcher(packet), 'watcher context')),
'rejected',
);
});

/**
* @param {Zone} zone
* @param {ExecutionContext<unknown>} t
*/
test('watcher args arity - shim', async t => {
const zone = makeHeapZone();
const { watch, when, makeVowKit } = prepareVowTools(zone);
const makeArityCheckWatcher = prepareArityCheckWatcher(zone, t);

const testCases = /** @type {const} */ ({
noArgs: [],
'single arg': ['testArg'],
'multiple args': ['testArg1', 'testArg2'],
});

for (const [name, args] of Object.entries(testCases)) {
const fulfillTesterP = Promise.resolve('test');
t.is(
await when(watch(fulfillTesterP, makeArityCheckWatcher(args), ...args)),
'fulfilled',
`fulfilled promise ${name}`,
);

const rejectTesterP = Promise.reject(Error('reason'));
t.is(
await when(watch(rejectTesterP, makeArityCheckWatcher(args), ...args)),
'rejected',
`rejected promise ${name}`,
);

const { vow: vow1, resolver: resolver1 } = makeVowKit();
const vow1P = Promise.resolve(vow1);
resolver1.resolve('test');
t.is(
await when(watch(vow1, makeArityCheckWatcher(args), ...args)),
'fulfilled',
`fulfilled vow ${name}`,
);
t.is(
await when(watch(vow1P, makeArityCheckWatcher(args), ...args)),
'fulfilled',
`promise to fulfilled vow ${name}`,
);

const { vow: vow2, resolver: resolver2 } = makeVowKit();
const vow2P = Promise.resolve(vow2);
resolver2.resolve(vow1);
t.is(
await when(watch(vow2P, makeArityCheckWatcher(args), ...args)),
'fulfilled',
`promise to vow to fulfilled vow ${name}`,
);

const { vow: vow3, resolver: resolver3 } = makeVowKit();
const vow3P = Promise.resolve(vow3);
resolver3.reject(Error('disco2'));
resolver3.resolve(vow2);
t.is(
await when(watch(vow3P, makeArityCheckWatcher(args), ...args)),
'rejected',
`promise to rejected vow before also resolving to vow ${name}`,
);
}
});

test('disconnection of non-vow informs watcher', async t => {
const zone = makeHeapZone();
const { watch, when } = prepareVowTools(zone, {
Expand Down
Loading