Angular Signals' 'this' context usage prevents passing the .set() function

[chrisrocco]

Which @angular/* package(s) are the source of the bug?

core

Is this a regression?

No

Description

The Signals library makes clever usage of the this context to avoid creating extra functions when creating new Signals.

signalFn.set = signalSetFn;   // 👈 Depends on 'this'
// vs
signalFn.set = signalSetFn.bind(node);
// or (fastest?)
signalFn.set = signalSetFn(node);  // 👈 Returns a bound closure

The problem is it's easy to lose the this context by using the function as a value.

const current = signal(0);
dataStream.subscribe(current.set);  // 👈 Wrong 'this' context leads to obscure errors

This surfaces a minor trade off between performance and correctness.

Microbenchmarks suggest optimal performance would be creating a closure like so -

const signalSetFn = (node) => (newValue) => ...;
signalFn.set = signalSetFn(node);  // 👈 In the factory function

^ https://measurethat.net/Benchmarks/Show/1328/0/bind-vs-closure-declaration

That's what I'd propose.

Alternatively, we should at least add a descriptive error message since this will come up a lot.

Please provide a link to a minimal reproduction of the bug

No response

Please provide the exception or error you saw

No response

Please provide the environment you discovered this bug in (run ng version)

No response

Anything else?

No response

[pkozlowski-opensource]

That's what I'd propose.

That's sounds reasonable to me. @chrisrocco would you like to crate a PR or do we want me to send a fix?

[chrisrocco]

Yep, I can send the PR.

[alxhub]

Fixed by #51986!

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}