From 3d4dfb703d443e97d366f4b81b7c9b6b36d730e6 Mon Sep 17 00:00:00 2001 From: Christopher Hiller Date: Tue, 3 Sep 2024 15:49:22 -0700 Subject: [PATCH] fix(trampoline): widen types, update README This simplifies the types such that we only really care about pulling the final result out of the generator; passing around generics for arguments was not solving any problems. Updated `README.md` to be less recursion-focused; updated/fixed example. Pulled some stuff out in response to peer review --- packages/trampoline/README.md | 27 +++--- packages/trampoline/src/trampoline.js | 24 +++--- packages/trampoline/src/types.ts | 54 ++++-------- .../test/trampoline-example.test.js | 24 +++--- packages/trampoline/test/trampoline.test-d.ts | 86 +++++++++++++------ 5 files changed, 114 insertions(+), 101 deletions(-) diff --git a/packages/trampoline/README.md b/packages/trampoline/README.md index 3f77763e44..3988bf61af 100644 --- a/packages/trampoline/README.md +++ b/packages/trampoline/README.md @@ -2,7 +2,7 @@ > Multicolor trampolining using generators (for your pleasure) -TL;DR: **@endo/trampoline** is a utility library which helps execute recursive operations in both sync and async contexts. +TL;DR: **@endo/trampoline** is a utility library which helps execute operations in both sync and async contexts (and supports recursion). ## Example Usage @@ -37,38 +37,42 @@ const findImportsAsync = async filepath => { * Recursively crawls a dependency tree to find all dependencies * * @template {string[] | Promise} TResult Type of result (list of imports) - * @param {ThunkFn} thunk Function which reads a file and returns its imports + * @param {(filepath: string) => TResult} finder Function which reads a file and returns its imports * @param {string} filename File to start from; entry point * @returns {Generator} Generator yielding list of imports */ -function* loadRecursive(thunk, filename) { - let specifiers = yield thunk(filename); +function* findAllImports(finder, filename) { + // it doesn't matter if finder is sync or async! + let specifiers = yield finder(filename); // pretend there's some de-duping, caching, - // scrubbing, etc. happening here! + // scrubbing, etc. happening here for (const specifier of specifiers) { - specifiers = [...specifiers, ...(yield* loadRecursive(thunk, specifier))]; + // it's okay to be recursive + specifiers = [...specifiers, ...(yield* findAllImports(finder, specifier))]; } return specifiers; } // results are an array of all imports found in some.js' dependency tree -const asyncResult = await trampoline(loadRecursive, readAsync, './some.js'); -const syncResult = syncTrampoline(loadRecursive, readSync, './some.js'); +const asyncResult = await trampoline(findAllImports, findImports, './some.js'); + +// same thing, but synchronously +const syncResult = syncTrampoline(findAllImports, findImportsAsync, './some.js'); asyncResult === syncResult; // true ``` -In the above example, **@endo/trampoline** allows us to re-use the recursive operations in `loadRecursive()` for _both_ sync and async execution. An implementation _without_ **@endo/trampoline** would need to duplicate the operations into two (2) discrete recursive functions—a synchronous-colored function and an asynchronous-colored function. Over time, this situation commonly leads to diverging implementations. If that _doesn't_ sound like a big deal for _whatever you're trying to do here_, then you probably don't need **@endo/trampoline**. +In the above example, **@endo/trampoline** allows us to re-use the operations in `loadRecursive()` for _both_ sync and async execution. An implementation _without_ **@endo/trampoline** would need to duplicate the operations into two (2) discrete recursive functions—a synchronous-colored function and an asynchronous-colored function. Over time, this situation commonly leads to diverging implementations. If that _doesn't_ sound like a big deal for _whatever you're trying to do here_, then you probably don't need **@endo/trampoline**. ## What is this? -The pattern exposed by this library—known as [trampolining][]—helps manage control flow in a way that avoids deep recursion and potential stack overflows. It effectively "converts" recursive calls into a loop. This is especially helpful in a language like JavaScript [because reasons][proper-tail-calls]. +The pattern exposed by this library—known as [trampolining][]—helps manage control flow in a way that avoids deep recursion and potential stack overflows. **@endo/trampoline** provides the trampolining pattern, but in such a way that a consumer can execute _either_ synchronous _or_ asynchronous operations _paired with operations common to both_. -In other words, **@endo/trampoline** can help _reduce code duplication_ when recursive operations must be executed _in both sync and async_ contexts. +In other words, **@endo/trampoline** can help _reduce code duplication_ when operations must be executed _in both sync and async_ contexts. ## Install @@ -87,5 +91,4 @@ Apache-2.0 By using this library, you agree to indemnify and hold harmless the authors of `@endo/trampoline` from any and all losses, liabilities and risk of bodily injury _including but not limited to_ broken bones, sprains, bruises or other hematomas, fibromas, teratomas, mesotheliomas, cooties, bubonic plague, psychic trauma or warts due to the inherent danger of trampolining. [trampolining]: https://raganwald.com/2013/03/28/trampolines-in-javascript.html -[proper-tail-calls]: https://www.mgmarlow.com/words/2021-03-27-proper-tail-calls-js/ diff --git a/packages/trampoline/src/trampoline.js b/packages/trampoline/src/trampoline.js index 68afa2266d..aebeb08142 100644 --- a/packages/trampoline/src/trampoline.js +++ b/packages/trampoline/src/trampoline.js @@ -1,6 +1,6 @@ /* eslint-disable @jessie.js/safe-await-separator */ /** - * @import {AnyGenerator, SyncTrampolineGeneratorFn, AsyncTrampolineGeneratorFn } from './types.js' + * @import {TrampolineResult, SyncTrampolineResult} from './types.js' */ const { getPrototypeOf } = Object; const { bind } = Function.prototype; @@ -15,12 +15,11 @@ const generatorThrow = uncurryThis(generatorPrototype.throw); /** * Trampoline on {@link TrampolineGeneratorFn generatorFn} synchronously. * - * @template {readonly any[]} [TArgs=unknown[]] Parameters for `generatorFn` - * @template [TResult=unknown] Type of the return value of the `generatorFn` - * @template {Generator} [TGenerator=Generator] Type of the generator function - * @param {SyncTrampolineGeneratorFn} generatorFn Generator-returning function accepting any arguments - * @param {TArgs} args Arguments to pass to the generatorFn call - * @returns {TResult} + * @template {readonly any[]} TArgs Parameters for `generatorFn` + * @template {(...args: TArgs) => Generator} TFn + * @param {TFn} generatorFn Generator-returning function accepting any arguments + * @param {TArgs} args Arguments to pass to `generatorFn` + * @returns {SyncTrampolineResult} */ export function syncTrampoline(generatorFn, ...args) { const iterator = generatorFn(...args); @@ -38,12 +37,11 @@ export function syncTrampoline(generatorFn, ...args) { /** * Trampoline on {@link TrampolineGeneratorFn generatorFn} asynchronously. * - * @template {readonly any[]} [TArgs=unknown[]] Parameters for `generatorFn` - * @template [TResult=unknown] Type of the return value of the `generatorFn` - * @template {Generator} [TGenerator=Generator] Type of the generator function - * @param {AsyncTrampolineGeneratorFn} generatorFn Generator-returning function accepting any arguments - * @param {TArgs} args Arguments to pass to the generatorFn call - * @returns {Promise} + * @template {readonly any[]} TArgs Parameters for `generatorFn` + * @template {(...args: TArgs) => Generator} TFn + * @param {TFn} generatorFn Generator-returning function accepting any arguments + * @param {TArgs} args Arguments to pass to `generatorFn` + * @returns {Promise>} */ export async function trampoline(generatorFn, ...args) { const iterator = generatorFn(...args); diff --git a/packages/trampoline/src/types.ts b/packages/trampoline/src/types.ts index 2a7a497db3..0d160a1e88 100644 --- a/packages/trampoline/src/types.ts +++ b/packages/trampoline/src/types.ts @@ -1,44 +1,22 @@ -export type AnyGenerator = Generator; /** - * A function type that represents a generator function for trampolining. - * - * @template TInitial - The type of the initial value. - * @template TArg - The type of the argument passed to the thunk function. - * Defaults to `TInitial`. - * @template TResult - The type of the result produced by the thunk function. - * Defaults to `TArg`. - * @param thunk - The thunk function to be used in the generator. - * @param initial - The initial value to start the generator. - * @returns A generator that yields results of type `TResult`. + * The final output of `asyncTrampoline()`, which will be wrapped in a `Promise`. */ -export type AsyncTrampolineGeneratorFn< - TGenerator extends AnyGenerator, - TArgs extends readonly any[] = unknown[], - TResult = unknown, -> = - TGenerator extends Generator - ? (...args: TArgs) => Generator, Awaited> - : never; +export type TrampolineResult< + TGeneratorFn extends (...args: any[]) => Generator = ( + ...args: any[] + ) => Generator, +> = TGeneratorFn extends (...args: any[]) => Generator + ? TResult + : never; /** - * A function type that represents a synchronous generator function for - * trampolining. - * - * This type ensures that the result type (`TResult`) is not a `Promise`. If - * `TResult` extends `Promise`, the type resolves to `never`. - * - * @template TArgs - The type of the arguments passed to the generator function. - * Defaults to `unknown[]`. - * @template TResult - The type of the result produced by the generator function. - * Defaults to `unknown`. - * @param args - The arguments passed to the generator function. - * @returns A generator that yields results of type `TResult`. + * The final output of `syncTrampoline()` */ -export type SyncTrampolineGeneratorFn< - TGenerator extends AnyGenerator, - TArgs extends readonly any[] = unknown[], - TResult = unknown, +export type SyncTrampolineResult< + TGeneratorFn extends (...args: any[]) => Generator = ( + ...args: any[] + ) => Generator, > = - TGenerator extends Generator - ? (...args: TArgs) => Generator - : never; + TrampolineResult extends Promise + ? never + : TrampolineResult; diff --git a/packages/trampoline/test/trampoline-example.test.js b/packages/trampoline/test/trampoline-example.test.js index 61a6b8f4f7..ad0051af9f 100644 --- a/packages/trampoline/test/trampoline-example.test.js +++ b/packages/trampoline/test/trampoline-example.test.js @@ -46,17 +46,21 @@ const findImportsAsync = async filepath => findImportsInSource(filepath); /** * Recursively crawls a dependency tree to find all dependencies * - * @template {string[]|Promise} TResult - * @param {(arg: string) => TResult} thunk - * @param {string} filename - * @returns {Generator} + * @template {string[] | Promise} TResult Type of result (list of imports) + * @param {(filepath: string) => TResult} finder Function which reads a file and returns its imports + * @param {string} filename File to start from; entry point + * @returns {Generator} Generator yielding list of imports */ -function* loadRecursive(thunk, filename) { - let specifiers = yield thunk(filename); +function* findAllImports(finder, filename) { + // it doesn't matter if finder is sync or async! + let specifiers = yield finder(filename); + // pretend there's some de-duping, caching, // scrubbing, etc. happening here + for (const specifier of specifiers) { - specifiers = [...specifiers, ...(yield* loadRecursive(thunk, specifier))]; + // it's okay to be recursive + specifiers = [...specifiers, ...(yield* findAllImports(finder, specifier))]; } return specifiers; } @@ -64,16 +68,16 @@ function* loadRecursive(thunk, filename) { const expected = ['b', 'c', 'c', 'd', 'e', 'f', 'g', 'e', 'f', 'g']; test('asynchronous execution - example code', async t => { - const asyncResult = await trampoline(loadRecursive, findImportsAsync, 'a'); + const asyncResult = await trampoline(findAllImports, findImportsAsync, 'a'); t.deepEqual(asyncResult, expected); }); test('asynchronous execution w/ sync thunk - example code', async t => { - const asyncResult = await trampoline(loadRecursive, findImportsSync, 'a'); + const asyncResult = await trampoline(findAllImports, findImportsSync, 'a'); t.deepEqual(asyncResult, expected); }); test('synchronous execution - example code', t => { - const syncResult = syncTrampoline(loadRecursive, findImportsSync, 'a'); + const syncResult = syncTrampoline(findAllImports, findImportsSync, 'a'); t.deepEqual(syncResult, expected); }); diff --git a/packages/trampoline/test/trampoline.test-d.ts b/packages/trampoline/test/trampoline.test-d.ts index 893f472c20..42e6078159 100644 --- a/packages/trampoline/test/trampoline.test-d.ts +++ b/packages/trampoline/test/trampoline.test-d.ts @@ -1,19 +1,8 @@ +/* eslint-disable jsdoc/require-returns-type */ +/* eslint-disable jsdoc/require-param-type */ /* eslint-disable no-redeclare */ import { expectAssignable, expectType } from 'tsd'; -import { trampoline, syncTrampoline } from '../src/trampoline.js'; -import { - SyncTrampolineGeneratorFn, - AsyncTrampolineGeneratorFn, -} from '../src/types.js'; - -function syncHook(x: number): number { - return x; -} - -async function asyncHook(x: number): Promise { - await Promise.resolve(); - return `${x}`; -} +import { syncTrampoline, asyncTrampoline } from '../src/trampoline.js'; function* simple>( thunk: (arg: string) => TResult, @@ -23,20 +12,17 @@ function* simple>( return `${hello} world`; } -expectAssignable< - AsyncTrampolineGeneratorFn< - ReturnType, - [(arg: string) => Promise, string], - Promise - > ->(simple); +expectAssignable, string, string>>( + simple((str: string) => `${str} cruel`, 'goodbye'), +); + +expectAssignable<(...args: any[]) => Generator>(simple); expectAssignable< - SyncTrampolineGeneratorFn< - ReturnType, - [(arg: string) => string, string], - string - > + ( + thunk: () => string | Promise, + initial: string, + ) => Generator, string, string> >(simple); expectType( @@ -44,9 +30,53 @@ expectType( ); expectType>( - trampoline(simple, async (str: string) => `${str} cruel`, 'goodbye'), + asyncTrampoline(simple, async (str: string) => `${str} cruel`, 'goodbye'), ); expectType>( - trampoline(simple, (str: string) => `${str} cruel`, 'goodbye'), + asyncTrampoline(simple, (str: string) => `${str} cruel`, 'goodbye'), ); + +/** + * Generators are difficult to type. We _may know_ the order in which typed + * values are yielded from the generator, but there's no way to define this in + * TS. If multiple types are at play, we can only use a union. + * + * Further, the same applies to `TNext` (in `Generator`); + * this is the type of the `value` passed to `iterator.next(value)`. + * + * The only thing we can be confident about is `TReturn` because it only happens + * once. + * + * The generator returned from this function will always return a `boolean`, but + * everything else is a mishmash. + * + * @param fn - Some callback + * @returns A generator that yields a variety of types. + */ +function* varied, Foo = unknown>( + fn: () => TResult, +): Generator { + let regexp: RegExp | Foo = yield 'hello world'; + regexp = yield fn(); + const ignored: RegExp | Foo = yield new Date(); + return regexp instanceof RegExp ? regexp.test('hello world') : false; +} + +expectAssignable< + Generator | Date, boolean, RegExp> +>(varied(() => 42)); + +expectAssignable<(...args: any[]) => Generator>(varied); + +expectAssignable< + ( + fn: () => number | Promise, + ) => Generator, boolean, RegExp> +>(varied); + +expectType(syncTrampoline(varied, () => 42)); + +expectType>(asyncTrampoline(varied, async () => 42)); + +expectType>(asyncTrampoline(varied, () => 42));