tc39/proposal-decorators

Intercept class field get/set (without accessor on prototype)

Closed this issue · 13 comments

It would be nice to be able to hook into the get/set of field storage.

The context.access.get/set functions cannot be patched to allow intercepting set/get of a field. I think it would be nice to be able to do that if we provide get/set functions somehow.

Note, this is distinct from accessor properties, and this issue is not the same as

Ideas:

  1. Maybe if there's a way to provide get/set functions for fields, it will convert the value descriptor to a get/set descriptor directly on the instance.
  2. alternatively maybe this does not change the descriptor to a get/set descriptor and instead is a new type of feature that is accessible only via decorators and the descriptor remains as a value descriptor. This might be more work to spec, while idea 1 is may be easier because get/set descriptors already exist.

Would this still uphold the requirement that class shape be statically analyzable up front? Just like accessor properties, this would not expose the prototype and would not allow a decorator author to arbitrarily modify the shape of the class until after instantiation, similar to field initializers.

Possible API shape

What could it look like without breaking stage 3 design?

Currently:

type ClassFieldDecorator = (value: undefined, context: {
  kind: "field";
  name: string | symbol;
  access: { get(): unknown, set(value: unknown): void };
  static: boolean;
  private: boolean;
  addInitializer(initializer: () => void): void;
}) => (initialValue: unknown) => unknown | void;

New shape:

type ClassFieldDecorator = (value: undefined, context: {
  kind: "field";
  name: string | symbol;
  access: {
    get(): unknown,
    set(value: unknown): void,

    // This part is new
    setter?(value: unkown): void,
    getter?(): unknown
  };
  static: boolean;
  private: boolean;
  addInitializer(initializer: () => void): void;
}) => (initialValue: unknown) => unknown | void;
  • This would allow a decorator to define a getter and setter.
  • If the getter or setter already exist in context.access, it means a previous decorator already provided it, and the current decorator can wrap it.
  • the current decorator can use context.access.get/set (if a previous decorator did not provide a getter/setter) to set the value in storage.
    • alternatively, getter/setter may always be undefined, and deco author must always use get/set to set storage which will pipe through previous decorator's getter/setter if any.

Example:

class Foo {
  @doubleAndLog foo = 1
}

function doubleAndLog(_, context) {
  context.access.setter = (value) => {
    console.log('doubled value:', value * 2)
    context.access.set(value * 2)
  }

  return (value) => {
    console.log('initial value:', value)
  }
}

const f = new Foo()
f.foo = 3
console.log('value:', f.foo)

Depending on the rules, the initial value would either

  1. Pass through the setter
  2. Or not

The output would either be (option 1):

initial value: 2
doubled value: 6
value: 6

Or (option 2):

initial value: 1
doubled value: 6
value: 6

This would also serve as an alternative to accessor without using a prototype getter/setter, and would allow reactivity without help from a class decorator:

class Foo {
  @ref foo = 123 // similar to Vue ref()
}

const f = new Foo

watchEffect(() => { // similar to Vue watchEffect
  console.log(f.foo) // this logs any time f.foo changes
})

(Implementation left to imagination)

2. alternatively maybe this does not change the descriptor to a get/set descriptor and instead is a new type of feature that is accessible only via decorators and the descriptor remains as a value descriptor

That is definitely a non-starter. "data" properties with behavior is the realm of exotic objects. There definitely should be no more ways than Proxy to enable JS programs to have such exotic behavior.

  1. Maybe if there's a way to provide get/set functions for fields, it will convert the value descriptor to a get/set descriptor directly on the instance.

I'm confused, how is this different from an auto-accessor besides the implicit nature of the transformation?

This has been discussed previously and has been ultimately determined to be a major performance hazard. See this article for details about how important this is, prior to various class field optimizations both public and private class fields were a major regression from simple named properties: https://v8.dev/blog/faster-class-features

Beyond that, getters and setters on instances are not optimizable, so it would ultimately result in worse performance across the board for these types of fields.

I'm closing this issue since it has been addressed previously.

There definitely should be no more ways than Proxy to enable JS programs to have such exotic behavior.

What's the reasoning for that?

We can already get and set the storage for a field, which is new behavior we've introduced. It isn't far-fetched to then be able to hook into that.

Proxy is by far more inconvenient, with various issues and foot guns, and extremely cumbersome to implement properly; most likely a Proxy would waste many hours of a developer's time compared to the feature ask besides potentially introducing multiple unexpected bugs down the road.

I would rather advocate for features that make development as easy as possible, and having to resort to Proxy is simply the opposite of that direction.

I'm confused, how is this different from an auto-accessor besides the implicit nature of the transformation?

Auto-accessors are on the class prototype (like getters/setters). These fields (with hooks for get/set) are on the instance. That's a main difference, and also aligns with the [[Define]] semantic of foo = 123.

Beyond that, getters and setters on instances are not optimizable, so it would ultimately result in worse performance across the board for these types of fields.

Of course function calls add overhead. This argument is not substantive because we haven't determined how exactly the feature would work, and if that overhead trumps the gain in ergonomics.

Yes there's a perf difference, as with everything, but does it really matter? If it does, I would like to understand exactly how critical it is.

The ergonomic win from this is substantial:

Take this existing code:

class Foo {
  foo = 123
}

Being able to do the following,

import {signal} from 'library-for-solid-js'

class Foo {
  @signal foo = 123
}

is much better than having to change the code to this,

import {signal} from 'library-for-solid-js'

class Foo {
  @signal accessor foo = 123
}

because the change in semantic of the property (from instance field to prototype property) could easily cause runtime errors and require unwanted refactoring, and that refactoring could even be impossible if the code is from 3rd parties.

On top of that, I've extensively used instance getters/setters in my daily 3D WebGL centered work for years (my profession), and I have had no performance issues due to instance getters/setters. The performance implication getters/setters have is small in comparison to the vast majority of problems that actually do cause performance issues. I'm saying this confidently from my years experience using and making reactivity on instances.

If someone has a rare slow field due to a decorator (easily detectable in Performance tab) they can drop that decorator and do something else.

This feature would be simply awesome for ergonomics and for most use cases the micro-performance differences will not matter.

(Measure our apps, be aware of what takes time, use or don't use libraries including decorator libraries based on measurements, and please allow good DX when the performance difference is not significant.)

Nice site btw, love the pixel art!

What's the reasoning for that?

These fields (with hooks for get/set) are on the instance.

I'm now confused as to what the request is.

When looking at the property descriptor of a non-exotic object, if it's a data property, the code can make assumption as to what accessing the property will do (aka it will not execute user code). Accessors are explicit ways of saying accessing the property will result in code execution.

The auto-accessor feature went the route of using accessors exactly because it was not introducing new exotic behavior.

Now if the request is to somehow install the accessors on instances instead of prototypes, since there is no exotic behavior there, I'm totally fine with it, but others may have objections. I honestly still don't understand what the concern is with installing the accessors on the prototype.

I'm now confused as to what the request is.

Let me try to shorten it. We have a class like this:

class Flower {
  color = 'cornflowerblue'
}

where color is a class field whose descriptor is on the instance (because class fields [[Define]]).

Then for example I want a decorator that can log the field's value whenever it changes,

class Flower {
  @log color = 'cornflowerblue'
}

new Flower().color = "violet" // logs "violet"

I want the field to still be a field, on the instance (i.e. I don't want the property to live on the prototype, I don't want to add accessor keyword to my class because I do not want to move the property descriptor to the class prototype).

The ask is to be able to provide getter/setter functions for fields to be able to intercept when the field storage is written or read.

I don't want to add accessor to my class definition because this drastically changes the shape of the class in a dangerous way, potentially breaking code and requiring unwanted refactors, all just for a log.

I want the field to remain a field (on the instance), so that code is much more likely to work as-is after adding the decorator (the @log decorator should have no effect on any part of the program other than logging the value and otherwise passing the value through).

Just to summarize: converting an existing field to an accessor can be very unwanted, and can be too difficult, making @log nearly impossible to use in some cases.

When looking at the property descriptor of a non-exotic object, if it's a data property, the code can make assumption as to what accessing the property will do (aka it will not execute user code). Accessors are explicit ways of saying accessing the property will result in code execution.

I understand this. So, in your opinion, if this was allowed, should the descriptor be modified to have a get and set? I would be ok with that, because having the descriptor in the same place will greatly reduce code ordering problems that could easily be caused by switching to accessor. In this case, each successive decorator would receive the latest get and set functions of the descriptor, and have a chance to replace or wrap them.

I honestly still don't understand what the concern is with installing the accessors on the prototype.

Hopefully the above clarified why they could be very inconvenient. But if an example is needed, I'd be happy to come up with one, let me know.

I want the field to still be a field, on the instance (i.e. I don't want the property to live on the prototype, I don't want to add accessor keyword to my class because I do not want to move the property descriptor to the class prototype).

can you elaborate on why?

I made an example. This "program" (it is a simple one, but for sake of example) relies on own keys:

class Foo {
    foo = 123

    constructor() {
        alert(Object.keys(this))
    }
}

new Foo()

live example (hit Run)

Now if you change it to use accessor, the behavior of the program changes (unwanted):

class Foo {
    accessor foo = 123

    constructor() {
        alert(Object.keys(this))
    }
}

new Foo()

live example

If we wanted to add a @log decorator and needed to also add accessor to use it, the change in program behavior could translate into a bug in the application.

Can you help me understand why anyone would want to do this, though? not just use alert, which has been strongly discouraged for decades, but to care about the own properties of an object instance - caring about own properties is for "dictionary-like" objects, and instances aren't dictionary-like.

The class that we want to add @log to might pre-exist. Maybe we didn't write it. alert(Object.keys(this)) is just an example placeholder for arbitrary logic of any size.

Adding @log accessor to a class could simply break it. That's the point. Doesn't matter why. You could tell me not to use alert all day, or to avoid Object.keys(), etc, for whatever reason you may have, but it would make no difference to the main point.

Simply put, code re-ordering can break existing programs.

I don't want to be required to refactor someone else's code just to add @log to a field, for example.

If you're adding a decorator to the class, you own it - who wrote it is irrelevant, and you're responsible for taking into account the choices made in that class, including whether they're good choices or not. I agree that blindly adding a decorator can break code - this is often the case - but that's part of the responsibility of owning code. It'd be the same thing with blindly making any changes.

So, in your opinion, if this was allowed, should the descriptor be modified to have a get and set?

Yes, the field would have to become an accessor property with get and set. However I don't think such implicit modification would be appropriate, for the same reason the accessor modifier was introduced, and as such an other explicit syntactic opt-in would be necessary. I also doubt the motivation to avoid putting the accessor on the prototype would be sufficient for the committee.

If you're adding a decorator to the class, you own it - who wrote it is irrelevant

Not when this happens:

import {someIndispensableFeature} from 'blackbox'

class MyClass {
  foo = 123
  constructor() {
    someIndispensableFeature(this)
  }
}

where blackbox is from a 3rd party and someIndispensableFeature is an inherently important part of our class that we can't easily replace.