preactjs / signals

Manage state with style in every framework
https://preactjs.com/blog/introducing-signals/
MIT License
3.79k stars 93 forks source link

Feature Request: Keep signal in sync with input-like fields. #300

Closed MicahZoltu closed 10 months ago

MicahZoltu commented 1 year ago

I find myself creating copies of default components like input and select that essentially are just doing this:

function Input({ value }: { value: Signal<string> }) {
    return <input value={value} onInput={event => value.value = event.currentTarget.value }/>
}

It would be nice if Preact Signals automatically stayed in sync with input-like elements when the signal is passed directly (rather than the signal's value).

rschristian commented 1 year ago

I feel that supporting two-way data binding would be a massive, massive change for the ecosystem, as it'll alter the underlying data flow fundamentals that people have come to know and expect.

Two vs one-way data binding is a key characteristic of frameworks -- I don't think it can be altered without partially redefining the framework itself.

MicahZoltu commented 1 year ago

If there was (somehow) a way to do this that was not a breaking change, would you otherwise be in favor of it? Are there other concerns you have with such a mechanism besides the breaking change aspect?

rschristian commented 1 year ago

This is certainly just my opinion, I don't know how others feel, but I'm less concerned with the breaking change aspect. React (and to some extent, Preact, though I think we're no where near as staunch on this) are built upon the one-way data flow model, and to change that, even in a major in an add-on library like this, would be massive. It's more an issue of general readability/understanding in the ecosystem at large for me.

For example, you should be able to toss a React dev into an app built with Signals without much issue -- sure, they might not grasp the strengths of Signals without reading the docs, but it's fairly idiomatic state primitive that acts like most others you'll find in the ecosystem. We commonly get asked "How is this different to something?" as it looks quite familiar, despite being rather unique. I think that's what makes Signals so popular.

However, if we were to introduce two-way data binding, that's a huge step away from what's familiar and known in the (P)react ecosystem. I don't think that would be expected behavior, and no amount of docs are really going to make up for a library doing something wildly unexpected.

MicahZoltu commented 1 year ago

I have been thinking on this lately and perhaps the issue is that I'm alone in almost always having value={value} onInput={event => value = event.currentTarget.value }? Are there other patterns people use for dealing with the incoming value of input-type elements?

If everyone does it similar to this way, then it seems like the one-way-binding is just a facade and in reality there is a two-way binding that just looks like a one-way binding.

Counterargument to myself: If some people do one-way binding with input elements, then changing value={value} to be two-way would remove their ability to do so, which would be bad even if one-way binding is uncommon.

Perhaps a new property could be added to input-like elements that is specifically for two-way binding (only supports a Signal<T> rather than a raw value).

dcporter commented 1 year ago

I feel that supporting two-way data binding would be a massive, massive change for the ecosystem

@rschristian I agree, but I don't think this suggestion rises to that description. It's just sugar over the boilerplate-y value={value} onInput={event => value.value = event.currentTarget.value }, and under the hood would have the same characteristics (and be easily overridable the way you'd expect sugar over that to be).

MicahZoltu commented 1 year ago

@dcporter What is your thinking on how to override it? Would you make it so if a signal is provided then you get a "two-way binding" and if a non-signal is provided then you get the normal one-way binding? A problem with this is that I think you will lose one of the big benefits of signals which is that you don't re-render the container component if you pass a signal. Maybe there could be some other way to override the "two way" binding behavior, or otherwise make it opt-in?

dcporter commented 1 year ago

I was picturing if you pass the signal it'd wire up a default onInput that worked like the above boilerplate; you'd opt out by passing in your own onInput.

rschristian commented 1 year ago

It's just sugar over the boilerplate-y value={value} onInput={event => value.value = event.currentTarget.value }

If, as you describe, a default onInput is attached for you, that's two-way data binding. That's what's done in frameworks that support it (though usually with a custom directive; v-model in Vue and bind: in Svelte, for example).

and under the hood would have the same characteristics (and be easily overridable the way you'd expect sugar over that to be).

Yeah, this is two-way data binding. Maybe taking a peek at Vue's docs could help? It too has the same underlying characteristics and is overridable (well, most have found it's better to opt-in, rather than override).

Best way of going about this (if you wanted to) is likely using a custom JSX transform that will support a custom directive. Wouldn't recommend transforming <input value={signalValue}> into <input value={signalValue} onInput={...}>, that's a recipe for headaches.

dcporter commented 1 year ago

I understand, I was saying it's a very tightly-focused, localized, sensible case of two-way bindings, not the scary headache you're describing. I've been bit plenty by vast two-way binding webs, my contention is that you need the vastness and the webness to get the headaches. This would be harmlessly local sugar.

Wouldn't recommend transforming into , that's a recipe for headaches.

Interesting, could you elaborate?

rschristian commented 1 year ago

It's stop being syntactic sugar if it can be mistaken for anything else. It's typically referred to as "magic" if it's something done behind the scenes, rather than "sugar", which is a shorter, but more explicit method of expressing something.

It's a recipe for headaches as the average (p)react dev reading that will assume that input isn't meant to update the state value -- this isn't terribly common, but it is something that's done every now and then. By creating a onInput handler behind the scenes, the state value will update without the dev really knowing why.

So that's why I'd recommend against that transform; be explicit! If you want to bind a value, create a directive that'll make it obvious to any reader. Other frameworks have already figured this out (v-model, bind:), you don't want to surprise any dev with unexpected behavior.

MicahZoltu commented 1 year ago

When you say "create a directive" do you mean proposing a change along the lines of (just an illustrative example) this?

<input twoWaySignal={mySignal}/>

The idea here being that it is very explicit, rather than something you can accidentally stumble into unintentionally?

rschristian commented 1 year ago

Well that's a prop, not a directive, but that's along the lines of what I'd suggest/caution one towards. Principle of least surprise applies here.

rschristian commented 1 year ago

Marvin built this demo the other day, showing off the directives approach with Preact: https://gist.github.com/marvinhagemeister/daada86f82466dff81d48840c8d5f27b

rschristian commented 10 months ago

Going to close this out, Marvin's directives demo is definitely along the lines of what I'd recommend (if you didn't want to copy it directly) and better left out of the core lib, IMO.