Should set throw when called in Signal.Computed value callback?

[sorvell]

According to the readme:

Computed Signals can write to other Signals, synchronously within their callback

This seems counter to the intended purpose of Signal.Computed, to compute a value from other values, and it seems (very?) likely to introduce bugs. Therefore it seems harmful and should likely throw an exception.

Since it is currently supported, there's likely a good reason for it. At the least, this should be documented if it isn't already.

Is it supported purely for compatibility with existing frameworks or is there there an important use case or design pattern it supports?

If it is only for compatibility with existing frameworks but is otherwise considered harmful, perhaps it should throw unless a specific subtle API is used.

[fabiospampinato]

I don't know why that's supported because I didn't write the proposal, but something like that is very useful for Solid's For component, which is basically a computed that maps inputs to outputs, problem is these outputs are generated by a function, and the function receives a signal that says at what index the input value was found in the input array, so one just updates that inside the computed with each update. If you had to do this differently presumably it would be slower for nothing.

[littledan]

I definitely see the value in discouraging/restricting writing to signals within a Computed. However, it is also very useful. How else would you reproduce the antipattern of setState in useEffect in signal-land? It would be great if we could find restrictions that could be applied, maybe on an opt-in basis, but it is important to permit prototyping without an absolute prohibition here.

[sorvell]

it is important to permit prototyping without an absolute prohibition here.

Sure, restricting this may be pre-mature, and I think maybe the concern speaks more to API design, and perhaps this goes along with #136.

How else would you reproduce the antipattern of setState in useEffect in signal-land?

Bear with me... my mental model, and this may admittedly be too simplistic, is that related to signals, there are 3 important contexts of which app devs should be aware such that they have a good mental model about which to reason. Various systems expose these contexts in different ways but they almost always exist conceptually:

1. computing data: Restrictions apply because access is tracked. Signal.Computed callback is in this bucket and if these buckets all existed in the API explicitly, I think throwing on set here would make sense.
2. responding to changes (effects): This is not explicitly part of the proposal and likely the crux of the issue that prompted the quoted question above since this would be the spot to write this (useful but sometimes anti-pattern) code. Devs should be thoughtful here about avoiding a set(...) that could loop.
3. elsewhere (e.g. event handler): Freely change data and read values here, nothing is tracked and sets are inputs into the graph of changes.

[littledan]

The current signal API has Signal.subtle.currentComputed, which can be used to hang your own read/write checks off of, in conjunction with a wrapper around all signal reads/writes in your framework. But we could go with a more “declarative”/less “expressive” model by having an option in the options bag that let a computed prohibit writes within it. This option would probably be used by default in most frameworks’ computed constructor—maybe it should even be our default as well.