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

Should getter be separated from setter in Signals? #94

Open
Spades-S opened this issue Apr 1, 2024 · 7 comments
Open

Should getter be separated from setter in Signals? #94

Spades-S opened this issue Apr 1, 2024 · 7 comments

Comments

@Spades-S
Copy link

Spades-S commented Apr 1, 2024

getter setter separation may be a good base for one-way data flow

@littledan
Copy link
Member

Hilariously, the README currently says

(See implications below in the "Capability separation" section.)

But that section doesn't exist! Anyway the basic point is: yes, I agree, we should encourage one-way dataflow, but this works out very cleanly as something to implement on top of the Signal.State class, and doesn't need to be the shape of its built-in API. This relates to our expectation that most frameworks (especially those implementing this capability separation) will wrap Signals for their own ergonomics (e.g., to make calling it be the way to get the value).

The reason for having them be the same object is a sort of micro-optimization: this avoids allocating two (or really three) objects. Because many frameworks will wrap them for different ergonomics, there will already be a separate allocation in practice that can implement this capability separation. No need to do that twice.

@Spades-S
Copy link
Author

Spades-S commented Apr 1, 2024

Application developer may get diverse signal APIs from UI frameworks, e.g. .value style in Preact, [getValue, setValue] style in Solidjs. Is it possible to remove the wrapper in frameworks? Framework developers and application developers use the same API, follow the same specs.

@NullVoxPopuli
Copy link
Collaborator

You may be interested in this issue:

It's currently on the library/framework author to to ergonomicify APIs.
There will still probably be room for an abstraction project, as frameworks tend to have (and want) varying semantics around effects, rendering, destruction/cleanup, timing of all these, etc (Starbeam is one such project, but is an exploratory project for now)

@Spades-S
Copy link
Author

Spades-S commented Apr 2, 2024

@NullVoxPopuli Thanks for sharing

@WebReflection
Copy link

Imho the current proposal and polyfill works like a charm already and if a library wants to expose a read-only signal it has already the primitive to do so: Signal.Computed:

import { Signal } from 'this-proposal';

const pvt = new Signal.State(0);

export default new Signal.Computed(() => pvt.get());

// from this module author / ownership
setTimeout(() => {
  pvt.set(pvt.get() + 1);
}, 1000);

This way anyone can effect on that computed as read-only signal as the API is equivalent around the .get() behavior and I don't find it particularly complicated or convoluted.

In alternative one can always wrap a signal and expose the get only or the set one, for whatever use case.

class GetValue {
  #signal;
  constructor(signal) {
    this.#signal = signal;
  }
  get() {
    return this.#signal.get();
  }
}

class SetValue {
  #signal;
  constructor(signal) {
    this.#signal = signal;
  }
  set(value) {
    this.#signal.set(value);
  }
}

const getInteral = new Signal.State(0);
const setInternal = new Signal.State(0);

export const getOnly = new GetValue(getInteral);
export const setOnly = new SetValue(setInternal);

effect(() => {
  setInternal.get();
  // get notified any time that value changes
});

@d8corp
Copy link

d8corp commented May 4, 2024

I like when a getter and setter combined in one entity.

In React I saw a warning like: "You provided a value prop to a form field without an onChange handler..." that is close to the question.

An example, which pushes to like the combining:

import innet from 'innet'
import dom from '@innet/dom'
import { State } from 'watch-state'

import { Input } from '@cantinc/ui'

const label = new State('test')

innet(
  <>
    <Input value={label} />
    <Input label={label} />
  </>,
  dom,
)

The first Input controls a label of the second one, so simple.

@ljharb
Copy link
Member

ljharb commented May 4, 2024

Related: #124

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants