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

Proposal: Add AsyncComputed #77

Merged
merged 11 commits into from
Oct 8, 2024

Conversation

justinfagnani
Copy link
Contributor

@justinfagnani justinfagnani commented Jul 6, 2024

Sorry for not opening an issue first, but I had this implementation of an AsyncComputed that I thought could be discussed either for inclusion or as a direction for signalFunction() or a future Relay implementation.

Conceptually AsyncComputed is very similar to signalFunction() or Relays, so there's a ton of overlap. I wrote this while trying to wrap my head around tc39/proposal-signals#30 and the signalFunction(). It's partly based on the shape of the Task utility that we have in Lit.

As I wrote in tc39/proposal-signals#30 (comment), AsyncComputed has these features:

  • It takes a Promise-returning computation function with an AbortSignal argument: (signal: AbortSignal) => Promise<T>
  • The function is called within an internal Signal.Computed, so all synchronous signal access is tracked
  • When the internal computed runs, it re-runs the compute function, pre-empting any pending run and aborting their abort signals.
  • It has signal-backed getters for its state:
    • state: one of 'initial', 'pending', 'complete', or 'error'. state will synchronously change to 'pending' when dependency signals are dirtied.
    • value: the last completion value if the last completion type was 'complete', otherwise undefined
    • error: the last error if the last completion type was 'error', otherwise undefined
  • It has a complete getter that returns a Promise that settles when the AsyncComputed does. If a new run is triggered before the previous completes, the same Promise instance is kept. If a new run is started after a completion, a new Promise is created.
  • It has a get() method that returns the latest completion value or throws if the last completion was an error.
  • It has a run() method to manually read from the internal computed and trigger a run. Otherwise the computed is read from the various getters.

I found this API shape and implementation to be relatively simple, and I'm starting to use it throughout my code base.

Compared to signalFunction() I think this mainly has a slightly smaller public API, doesn't use SignalAsyncData, and has AbortSignal support. The completion promise behavior is different - AsyncComputed's complete promise will be shared among concurrent/preempted runs and only settled by the most recent run.

Compared to Relays I think the differences are that there are no consumer APIs for altering the state, and instead of passing get and set functions to the compute function, it relies on the return value of the compute function, and there's no update/destroy concept inside the compute function.

I think it's worth noting that utilities like this are also similar in some ways to libraries like TanStack Query, which are very feature-rich with things like caching, debouncing, etc. I think most of those feature can be implemented as part of the compute function. It may be helpful to pass the AsyncComputed instance back to the compute function to use as a key for caches or WeakMaps that store last invocation time for debouncing. Implementing debouncing inside the compute function would also require some kind of conditional gate on a run - either a shouldRun function in the options, or supporting an AbortError being thrown synchronously from the compute function as a way to cancel preemption.

@@ -38,6 +38,7 @@ jobs:
steps:
- uses: actions/checkout@v4
- uses: wyvox/action-setup-pnpm@v3
with: { node-version: 22 }
Copy link
Collaborator

Choose a reason for hiding this comment

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

why specify this? 🤔

we could add an .nvmrc or .node-version, perhaps?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To get Promise.withResolvers().

It's of course only a few lines to do what withResolvers() does, but withResolvers() is so nice and I figured this library is pretty cutting edge, so...

Copy link
Collaborator

Choose a reason for hiding this comment

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

oh yes, I mean, setting node 22 is great!

Will also need to update:

Copy link
Collaborator

Choose a reason for hiding this comment

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

The inline docs here are wonderful!

src/async-computed.ts Outdated Show resolved Hide resolved
src/async-computed.ts Show resolved Hide resolved
src/async-computed.ts Outdated Show resolved Hide resolved
/**
* Creates a new AsyncComputed signal.
*
* @param fn The function that performs the asynchronous computation. Any
Copy link

Choose a reason for hiding this comment

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

While there can't be fully automatic tracking for any async signal reads in the computation, it could be supported via an API. Consider:

fn: (abort: AbortSignal, track<K>: (() => K) => K) => Promise<T>.

The track function would simply pass through the returned value but internally it would do so via a computed which could then be watched by the watcher to trigger a re-run.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I definitely considered that - it's something we could add later.

It's also debatable whether the current behavior is better than a separately tracked sync function, similar to how reaction() works. The argument for the current behavior is that there are some nice ergonomic benefits from writing just one function, and not having to fashion all the state you care about into a single return value.

src/async-computed.ts Outdated Show resolved Hide resolved
/**
* Creates a new AsyncComputed signal.
*
* @param fn The function that performs the asynchronous computation. Any
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I definitely considered that - it's something we could add later.

It's also debatable whether the current behavior is better than a separately tracked sync function, similar to how reaction() works. The argument for the current behavior is that there are some nice ergonomic benefits from writing just one function, and not having to fashion all the state you care about into a single return value.

@justinfagnani justinfagnani marked this pull request as ready for review October 8, 2024 01:57
@justinfagnani
Copy link
Contributor Author

@NullVoxPopuli this is ready for review now!

@NullVoxPopuli NullVoxPopuli merged commit 52903a7 into proposal-signals:main Oct 8, 2024
4 checks passed
@NullVoxPopuli NullVoxPopuli added the enhancement New feature or request label Oct 8, 2024
@github-actions github-actions bot mentioned this pull request Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants