proposal-signals / signal-utils

MIT License
89 stars 10 forks source link

Allow signal options in the @signal decorator #42

Open justinfagnani opened 7 months ago

justinfagnani commented 7 months ago

@signal is a wrapper for new Signal.State() but it does not expose the options argument.

It should be possible to do:

class MyClass {
  @signal({equals: deepEquals})
  accessor someImmutableThing;
}

There would be two options for the signal() signature:

  1. @signal accessor foo for non-options usage and @signal({...}) accessor foo for options usage.
  2. @signal() accessor foo for non-options usage and @signal({...}) accessor foo for options usage.

(2) is possible by duck-typing the arguments.

I personally prefer style (2) for consistency, but there's also consistency to consider with the other decorators. I guess @cached could accept options as well, since it's basically a wrapper for new Signal.Computed().

NullVoxPopuli commented 7 months ago

Option 1 is my preference as well with decorators, prevents usneeded function execution in the simple (usually more common) case.

@cached could accept options as well

Sounds like a good idea!

justinfagnani commented 7 months ago

Oops... I meant to say that I prefer option (2)! I found that a lot of decorators take options, so always having the () after the decorator provides visual consistency. It also makes it easier to add options in the future even if a decorator doesn't take them now.

NullVoxPopuli commented 7 months ago

There is no reason both can't be supported simultaneously, as it turns out. Invocation without options is just {} for options.

But i do think this should be avoided for faster module evaluation