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

overriding auto accessor init can be confusing for *composed* decorators #547

Open
trusktr opened this issue Oct 16, 2024 · 2 comments
Open

Comments

@trusktr
Copy link
Contributor

trusktr commented Oct 16, 2024

Note

Keep in mind that this is feedback coming in after using decorators, which was not previously possible until tools like TypeScript and Babel have more recently implemented the Stage 3 spec and we've had time to work with them in the field.

The examples in the README show that the value for an auto accessor decorator is {get, set}, and there is no init.

The examples show that init can only be returned from a decorator, f.e. return {get, set, init}, without a way to patch/override/intercept a previous decorators init.

Thus, there appears to be no way to patch/wrap or override an initializer when using multiple deocrators, however when composing decorators we can override or patch/wrap them.

For example:

function foo({get, set}, context) {
  return {
    get,
    set,
    init(v) { return v * 2 },
  }
}

function bar({get, set}, context) {
  return {
    get,
    set,
    init(v) { return v * 2 },
  }
}

function baz({get, set}, context) {
  // compose the foo decorator
  const {get: fooGet, set: fooSet, init: fooInit} = foo({get, set}, context)

  return {
    get,
    set,

    // HERE =================
    // do we do this?
    init(v) { return fooInit(v) * 2 },
    // or this?
    // init(v) { return v * 2 },
  }
}

class Test {
  @bar @foo accessor a = 1
  @baz accessor b = 1
}

const o = new Test()

console.log(o.a)
console.log(o.b)

As you can see, when writing multiple decorators like @bar @foo, there is no way to patch/override. However, when composing, we can patch/override (and even forget) the composed init.

Wouldn't it be better for consistency if the value for an autoaccessor included the init, for example like this,

function foo({get, set, init}, context) {
  return {
    get,
    set,
    init(v) { return (init ? init(v) : v) * 2 },
  }
}

where init is undefined if the decorator did not provide init?

This would mean that no matter how decorators are connected together, whether strung individually, or wrapped together, the decision is always the same: wrap or don't wrap the previous decorator's get, set, and init functions.

Plus the current setup makes init handling inconsistent with get and set too. It raises the question: why do we get to choose how to wrap or override get and set no matter the compositional pattern, but not init?

For reference: TS playground and Babel repl.

@trusktr trusktr changed the title overriding auto accessor can be confusing for *composed* decorators overriding auto accessor init can be confusing for *composed* decorators Oct 16, 2024
@pzuraq
Copy link
Collaborator

pzuraq commented Oct 16, 2024

This was considered as part of the original proposal, but was shot down over performance concerns. The concern was that the initial byte code generated by a class field initializer would be modified in some way. In general, we couldn't find any use cases that required wrapping or capturing of the initializer, vs just modifying the initialized value, so this was the accepted behavior in the end.

@trusktr
Copy link
Contributor Author

trusktr commented Oct 16, 2024

Oh ok. I'm guessing this means the byte code would need to be generated after decorator application instead of before (as far as for the initializer function) (the byte code for the shape of the initial value could change either way). If so, Is there a benefit of the byte code shape being generated before decorators run instead of after? I'm thinking that its good for AoT, but isn't that just fighting against the nature of JavaScript?

I also had a small typo: init(v) { return fooInit(v) * 2 } -> init(v) { return fooInit.call(this, v) * 2 }

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