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

Classes are useless here #232

Closed
Hadaward opened this issue Jul 5, 2024 · 5 comments
Closed

Classes are useless here #232

Hadaward opened this issue Jul 5, 2024 · 5 comments

Comments

@Hadaward
Copy link

Hadaward commented Jul 5, 2024

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
Copy link
Collaborator

NullVoxPopuli commented Jul 5, 2024

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
Copy link
Author

Hadaward commented Jul 5, 2024

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
Copy link
Author

Hadaward commented Jul 5, 2024

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
Copy link
Collaborator

NullVoxPopuli commented Jul 5, 2024

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
Copy link
Collaborator

NullVoxPopuli commented Jul 5, 2024

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

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

2 participants