tc39 / proposal-signals

A proposal to add signals to JavaScript.
MIT License
2.95k stars 54 forks source link

Convenience methods #32

Open littledan opened 7 months ago

littledan commented 7 months ago

Should the Signal API include any convenience methods? Possibilities:

Would any other convenience methods be useful? The current API omits all of these on purpose, with the goal of being minimal.

benlesh commented 5 months ago

From another discussion: There's a question about whether or not we should have a way to read a value if it's a signal or a value... or make sure it's a signal first etc. Similar to Promise.resolve(promiseOrValue) should there be a Signal.of(signalOrValue) or Signal.read(signalOrValue)?

The use case would be something like a component that accepted a property that could either be a signal or a plain value. Let's say the author wants to encourage the use of signals for managing dynamically changing state, but the state doesn't always need to dynamically change. For example with a "layout component".

NullVoxPopuli commented 2 months ago

potentially also, localCopy, which allows state-forking, but re-sets itself with the upstream value if that changes.

Demo / behaviors here:

(some context: https://twitter.com/nullvoxpopuli/status/1770862533378707571 )

jods4 commented 2 months ago

peek has been there since early KO and is a simple, more efficient untrack when reading a single Signal/Computed.

signal.peek() and computed.peek() behave like .get() does when called outside a computing context, i.e. they return the current value without tracking the read.

EisenbergEffect commented 2 months ago

I think wee need untrack() because there are scenarios when an entire block of operations needs to be performed and where the code doesn't have direct access to the signals. But, it might be that peek could be added as an additional API.

justinfagnani commented 2 months ago

I have a utility that deals in a single signal and needs to peek it. Having a Signal.subtle.peek(signal) function would be great for that.

WebReflection commented 2 months ago

FWIWI I've resolved the peek() case as such and it seems to work well:

class State extends Signal.State {
  peek() {
    return untrack(() => this.get());
  }
}

The isSignal also seems easy to implement via a couple of instanceof operations but it might poison arbitrary libraries built on top of this API so I think we're better off without it (i.e. I am not extending Signal.State but I am returning signals and these should not fail expectations).

If effect and batch would be in somehow in a least convoluted, yet effective, way, this API would be overly awesome!

jods4 commented 2 months ago

@EisenbergEffect Yes, I wasn't suggesting that peek replaces untrack. A way to completely escape a computing context is required (see effectScope discussions in Vue for example).

But sometimes all you need is reading one signal and untrack(() => this.get()) is not great DX, nor very efficient compared to peek implementation.

WebReflection commented 2 months ago

@jods4 untrack is not about just one signal or computed, it's about running something potentially huge ... peek() as it is in Preact is for each single signal/computed and it's an explicit user intent to not track that particular change.

Preact has untrack too so there is also previous work around this difference which is very important.

Accordingly, I wouldn't mix the two use-cases or needs, untrack is fine as it is and it can be used to abstract peek already, peek as State or Computed method might be welcome but it's a different use case than untrack: peek is atomic, untrack doesn't need you to change code around a signal so I hope we cna either have both or just have untrack and provide that abstraction like I did already so that users can explicitly peek single values or untrack callbacks with potentially dozen signals in it.

edit also let's keep in mind this API proposal is for libraries authors ... the DX argument is hence weak as long as the desired result is achievable and untrack allows peek to exist, if a library author wants to provide that ability ... if we go in features-creep-land this API won't ever reach the Web, which would be unfortunate, as it's already awesome as it is and it has it all behind the scene.

jods4 commented 2 months ago

@WebReflection that's exactly what I said, not sure how you understood my previous messages. I said untrack is required and cited an example why in the context of Vue.

the DX argument is hence weak as long as the desired result is achievable and untrack allows peek to exist, if a library author wants to provide that ability ... if we go in features-creep-land this API won't ever reach the Web, which would be unfortunate, as it's already awesome as it is and it has it all behind the scene.

Yes, anyone can build peek as untrack(() => signal.get()). Doing so they create a closure and call a function that has to change the computing context using a stack, before calling that closure in a try/catch/finally block.

On the other hand, built-in peek on signal is basically peek() { return this.#value }.

I'm not sure that's a lot of feature creep, especially considering that most (all?) "signal" implementation for the past 10 years have exposed a peek in their core set of features.

WebReflection commented 2 months ago

checking internals of the polyfill, it could indeed just return the internal symbol value and avoid extra stacks. Apologies I've misread your previous comment, I've thought you preferred peek() over untrack(), my bad. Agreed having both would be nice.

EisenbergEffect commented 2 months ago

Great conversation here! Thanks for the clarification @WebReflection and @jods4! QQ: If we have peek, is there any use case for a poke as well, to set the value?

WebReflection commented 2 months ago

@EisenbergEffect I feel like that's a stretch too far ... or better, I don't understand the use case for it, it feels surprise prone to me as "a sneaky update nobody should notice" and that to me won't lead to better code ... .peek() is an explicit opt-out to subscription, .poke(value) as opt-out to reactivity in general feels odd (imho).

if we take previous art work around the topic, I am not sure which library exposes that to date but if they do, it's also easily an attack vector to me if these libraries land in undesired places where untrusted code starts poking around foreign code ... it's chaos easy to generate, while .peek() remains into the "observable field", if that makes sense (and the code owner can trust).


edit to some extend, think .poke(value) as an easy way to dispatchEvent nobody can listen to, and .peek() as an easy way to just get .value ... the former has catastrophic effects if not well orchestrated accordingly, the latest is always harm-free for the rest of the logic.

jods4 commented 2 months ago

@EisenbergEffect RE poke: In all these years I don't think I've ever encountered a use-case for setting a value without triggering signal. It feels dangerous because all dependencies think they're clean although they're not.

"Triggering" computeds or bindings even though they "have not changed" is a topic that comes up quite often in UI frameworks. It can often be worked around with better designs or by introducing missing signals (even just a counter that's incremented when a refresh is desired).

WebReflection commented 2 months ago

agreed and on a second though, untrack is fine to exist and has tons of use cases already, .peek() is the "atomic untrack" within a state/computed and it's fine and has tons of use cases ... .poke(...) is dirty by design and doesn't even have its own untrack like counterpart ... I would discourage exploring that field as it can't produce anything good to me, not even libraries authors ... if it lands a .poke(...) it should land with those funny comments like "YOU_ARE_FIRED_IF_YOU_INVOKE_THIS" and while I am sure there is a single edge case that makes sense in the world about it (such as, update an object reference underneat with same shape but different reference for the GC) I think is disaster prone if exposed and, once exposed, of course libraries authors will use it then ... please let's drop this idea already 😅

NullVoxPopuli commented 2 months ago

untrack is fine to exist and has tons of use cases already

do we have a list of these? would be good for a "guidance" and "recommendations" section of the docs / proposal, because there are a lot of ways to shoot one's self in the foot with reactive systems.

May be of interest:

EisenbergEffect commented 2 months ago

Thanks everyone! I was pretty sure poke was a bad idea and I couldn’t think of any use cases, but wanted to make sure I wasn’t missing something.

WebReflection commented 2 months ago

@NullVoxPopuli

do we have a list of these?

just the fact one can send errors or debug on Promise catch would be enough to me, as you never want to subscribe to those callbacks

fetch(remote.get()).catch(error => {
  untrack(() => {
    console.error(`Unable to fetch ${remote.get()}`, error);
  });
});

Sure thing for that specific use case .peek() would be even better but with callbacks around used to effect and compute having a way to introspect their values/invokes without subscribing is desirable.

NullVoxPopuli commented 1 month ago

I don't think callbacks like that are inherently subscribed to, and untrack wouldn't be needed 🤔 i am about to verify this tho in https://github.com/NullVoxPopuli/signal-utils/pull/19

More info soon!

littledan commented 1 month ago

Yeah my feeling is that events/microtask queue items would have a null currentComputed by default, and just not be tracking until you go into a Computed.

WebReflection commented 1 month ago

my example was poorly presented ... I meant that whenever signals or computed are around and you want to:

I can do a quick search in Preact repo though as I am sure they had various reasons/requests/discussions aroud this topic, if that helps anyone (or users wanting to understand when untrack and peek() are desirable)

To my DOM examples based on signals, there are cases I need to retrieve a value in sync with the rest of the flow but without subscribing the whole thing to my introspection logic, so this is another use case ... libraries' authors will need, and want, a way to not subscribe for foreign data they don't understand where it comes from or how frequently it could update.

edit ... I mean, without going too far from my own repositories I recently had complains about untracked too so I guess people use these features daily: https://github.com/WebReflection/signal/pull/6

More requests around untrack here: https://github.com/WebReflection/usignal/issues/18

This is still my own stuff, which was fully inspired by Preact signals/core

jods4 commented 1 month ago

If you're fishing for peek / untrack use cases:

Very recently I wanted to capture the initial state of reactive objects, to be able to provide a "dirty" flag. This happened in a computed that "initialised" the transactional object from the currently bound object. It's easy to understand that this computed should only be dependent on the bound object instance (if instance changes, a new transactional object is created). Because this step read all reactive object properties to capture initial values, they became dependencies and a new transactional object would be created at each change, which is completely broken. My solution: peek at reactive properties when capturing initial values, so that those reads are not reactive (it's intuitive as well: by definition initial values never change).

You can also dig into Vue effectScope RFC: https://github.com/vuejs/rfcs/pull/212 It's a broader API mainly targeted at ownership / lifetime outside of components, but one motivation for the design was to be able to escape the current computing context, see Detached Nested Scope.

Vue also exposes pauseTracking() and resumeTracking() as advanced apis, that serve an identical function as untrack().

I previously mentionned that Knockout has had peek since forever: https://knockoutjs.com/documentation/computed-dependency-tracking.html#controlling-dependencies-using-peek. The doc gives as an example a computed that makes an AJAX request based on a page number, while also keeping track of the selected item although the latter is not a depedency (no AJAX request if selected item changes).

NullVoxPopuli commented 1 month ago

Here is a userland implementation of peek (via localCopy without an effect or untrack):

EisenbergEffect commented 1 month ago

@littledan We've had the request for peek() come up in several different conversations and there's a lot of precedent for it in existing libraries. Since one of our goals is performance/memory-efficiency, this seems like a reasonable thing to add to eliminate callback/closure usage in a number of common scenarios. What do you think about adding this to both State and Computed?

WebReflection commented 1 month ago

my 2c: explicit peek() is intentional zero side-effects by design so I hope we can make it part of the specs ... poke(value) as suggested elsewhere as counter-method is undesired side-effects by design so it should likely never land in these specs.