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

@signal decorator improvements #9

Merged
merged 2 commits into from
Sep 25, 2024
Merged

@signal decorator improvements #9

merged 2 commits into from
Sep 25, 2024

Conversation

trusktr
Copy link
Member

@trusktr trusktr commented Sep 25, 2024

BREAKING: There's a breaking change on how to apply @signal to a getter/setter, see below.

  • feat: Place @signal decorator descriptors in the same locations as decorated class members to more closely align the object shape with what a user wrote. For example if a field was decorated with @signal, the signal descriptor will be on the instance, and if an auto or getter/setter accessor was decorated with @signal then the descriptor will be on the prototype
    • BREAKING: Getters/setters decorated with @signal no longer result in own properties on instances, but result in a prototype property. For example, any checks like Object.hasOwn(this, 'prop') for a property named prop defined as @signal get prop() {} will now return false instead of true. Features like Object.assign will no longer work on those properties. Etc. If you need the properties to be own, migrate to using @signal prop instead of a getter/setter, or update the dependent code to do something else.
  • feat: add the ability to finalize @signal fields without a class decorator by defining @signal #finalize after all fields are defined. For example instead of this,
    @reactive
    class MyClass {
      @signal foo = 1
      @signal bar = 2
    }
    we can write this:
    class MyClass {
      @signal foo = 1
      @signal bar = 2
      @signal #finalize
    }
    The latter version has an advantage: fields are signalified by the time any code in constructor runs, making it possible to rely on the signal fields inside constructor. For example the following will not work when using the @reactive class decorator approach:
    @reactive
    class MyClass {
      @signal foo = 1
      constructor() {
        createEffect(() => console.log(this.foo)) // This effect will never re-run (do not rely on signal fields in constructor in this case)
      }
    }
    but the following will work:
    class MyClass {
      @signal foo = 1
      @signal #finalize
      constructor() {
        createEffect(() => console.log(this.foo)) // This effect will re-run (signal fields are reliable in constructor)
      }
    }
    Just make sure that @signal #finalize comes after all class field definitions; any fields that come after @signal #finalize will not be signalified.
    It is up to you which form you prefer. If you do not need the signals within constructor, you might like the aesthetic look of the @reactive class decorator more than an unused #finalize field, or, at an implementation level you might like the purity of the @signal #finalize approach because it does not make a subclass like @reactive does.
  • feat: refactor @signal on getters/setters to work without a class decorator (adding support for #private getters/setters, but now requiring the @signal decorator to be placed on both the getter and the setter otherwise it won't work and an error will be shown in console)
    • BREAKING: getter/setter pairs no longer require the use of the @reactive class decorator, but the @signal decorator must now be placed on both the getter and setter of a property definition:
      // Bad (this example will show a helpful error in console in case you forget to decorate *both* the getter and setter, and the property will not be reactive)
      class MyClass {
        @signal
        get foo() {...}
        set foo(value) {...}
      }
      
      // Good (the property will be reactive)
      class MyClass {
        @signal get foo() {...}
        @signal set foo(value) {...}
      }
      To migrate, add an extra @signal to your getter/setter so that both are decorated. Optionally delete the @reactive decorator from your class if you are using the new @signal #finalize or no fields at all (f.e. only auto accessors).
    • This now supports #private getters/setters:
      class MyClass {
        @signal get #foo() {...}
        @signal set #foo(value) {...}
      }
  • feat: add support for public and #private auto accessors. For example, and as an alternative to @reactive or @signal #finalize approaches, we can now write "auto accessor" fields:
    class MyClass {
      @signal accessor foo = 1
      constructor() {
        createEffect(() => console.log(this.foo)) // This effect will re-run.
      }
    }
    It also supports #private "auto accessor" fields:
    class MyClass {
      @signal accessor #foo = 1
    }

The main benefit of this update is that we now have a way to apply @signal to #private properties, but note that only auto accessors and getters/setters are supported when it comes to making them #private, and fields cannot not be private due to the current version of the decorator spec:

@reactive
class MyClass {
  // This is impossible with the current decorator spec, an error will be thrown to let you know.
  @signal #foo = 1

  // In TypeScript you can at least mark the field "private" for type checking if you prefer to use fields:
  @signal private foo = 1 // (not actually private at runtime)

  // This works.
  @signal accessor #foo = 1

  // This works too.
  @signal get #foo() {...}
  @signal set #foo(v) {...}
}

…d class members to more closely align the object shape with what a user wrote
@trusktr trusktr marked this pull request as draft September 25, 2024 08:55
- add the ability to finalize `@signal` fields without a class decorator by defining a `@signal #finalize` field after all fields are defined
- refactor `@signal` on getters/setters to work without a class decorator (add support for `#private` getters/setters, but now requires the `@signal` decorator to be placed on both the getter and the setter, otherwise it won't work and an error will be shown in console)
- add support for public and `#private` auto accessors
- organize code and tests

organize files and tests
@trusktr trusktr marked this pull request as ready for review September 25, 2024 22:19
@trusktr trusktr merged commit 77183cb into main Sep 25, 2024
2 checks passed
@trusktr trusktr deleted the improve-signal-decorator branch September 25, 2024 22:19
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

Successfully merging this pull request may close these issues.

1 participant