tc39 / proposal-signals

A proposal to add signals to JavaScript.
MIT License
3.37k stars 58 forks source link

Classes are useless here #232

Closed Hadaward closed 3 months ago

Hadaward commented 3 months ago

In my point of view, we are creating an instance of a class without the slightest need, if we are going to use just one function then it is better to follow the same pattern as other global objects like Object.fromEntries, why not Signal.someMethod()?

E.g:

const counter = Signal.state(0);
// added deps array so the code knows what state to watch for changes
const isEven = Signal.computed(counter => counter.get() & 1 === 0, [counter]);
const parity = Signal.computed(isEven => isEvent.get() ? "even" : "odd", [isEven]);

// If you're going to copy react, do it right, there's no point in having state if it's not going to have effect.
// Using an external library just to have effect would be strange.
Signal.effect(parity => element.innerText = parity.get(), [parity]);
NullVoxPopuli commented 3 months ago

Please read:

We should probably add to the FAQ about 'why classes', but the short of it is (I'm on mobile, sorry), they are more memory efficient - requiring less allocations than functions that manage internal state and expose an object of its public api.

See also: https://github.com/tc39/proposal-signals?tab=readme-ov-file#memory-management

This isn't to say we can't add shorthand functions later, like a PR, proposals to language should be minimal and focused, so there is the least amount of bike shedding at one time.

At the moment, what you ask for is implementable in user space. We have https://github.com/proposal-signals/signal-utils/pulls For exploring user land implementations of things. 💪

Any pr to improve the faq or document over all would be much appreciated 🎉

Hadaward commented 3 months ago

But we are creating an instance of Signal for each method call, so what is the purpose of the class itself? We are not storing anything in the class itself. In the homepage example, you created several Signal instances, I don't know how this can be more efficient but you are just doing micro-optimizations, right? I don't see this as a problem. Another point, if you are going to use classes, then it makes more sense for these methods to be aligned with a single instance that you created, creating multiple instances is only worse.

const signal = new Signal();

const counter = signal.state(0);
const isEven = signal.computed(() => (counter.get() & 1) == 0);
const parity = signal.computed(() => isEven.get() ? "even" : "odd");

signal.effect(() => element.innerText = parity.get());

// Simulate external updates to counter...
setInterval(() => counter.set(counter.get() + 1), 1000);

also remember that in JS the naming convention is camelCase, using Signal.State is weird as State is a function not a class.

Hadaward commented 3 months ago

Other developers on a javascript discord community said that having to explicitly declare a variable as computable is clunky, many libraries and frameworks allow you to do this without having to use some method to do so.

NullVoxPopuli commented 3 months ago

A goal is for existing frameworks and libraries to wrap these signals, so folks would continue using the APIS they're used to... no need to change -- This proposal is not intending to replace any signals APIs that exists today, but enable cross-compatibility between all the reactive ecosystems that use these signals as their foundation.

NullVoxPopuli commented 3 months ago

so what is the purpose of the class itself? We are not storing anything in the class itself

this is false as state is stored <3 (where would get() otherwise get the value from?)

if you're curious about how the internals could work -- please have a browse of this example implementation: https://github.com/proposal-signals/signal-polyfill in particular the State class here: https://github.com/proposal-signals/signal-polyfill/blob/main/src/wrapper.ts#L40