tc39/proposal-decorators

Field decorator initializer should support configurable field

Closed this issue · 8 comments

Consider this example:

function readonly(value: any, context: ClassFieldDecoratorContext) {
  return function (initialValue) {
    Object.defineProperty(this, context.name, {
      writable: false,
      value: initialValue,
    });

    return initialValue;
  };
}

class Person {
  @readonly
  firstName = "Santi";
}

const p = new Person();

This fails with the following error:

image

It happens because of this writable: false. The initializer in case it returns undefined, it should not try to set the field again. So the readonly decorator will work fine without an extra decorator in the class.

Sorry if this is duplicate but I did not find it anywhere.

Thanks.

Are you sure it doesn’t happen because of the implied configurable false?

If configurable: true is provided, the returned value will replace the property entirely making readonly decorator to fail its mission:

function readonly(value: any, context: ClassFieldDecoratorContext) {
  return function (initialValue) {
    Object.defineProperty(this, context.name, {
      writable: false,
      value: initialValue,
      configurable: true,
    });

    return initialValue;
  };
}

class Person {
  @readonly
  firstName = "Santi";
}

const p = new Person();
p.firstName = "Some other value";
console.log(p);

image

Since classes use Define semantics, not Set, writable is irrelevant there - i think you’d have to add an initializer that runs later and marks it as nonwritable, instead of returning a function

I'm returning a function to be able to access the this instance. Do you mean something like context.addInitializer()? That would be nice to have but it does not exists.

ah, right, this use case is explicitly mentioned in #437

Interesting https://github.com/tc39/proposal-decorators/pull/437/files#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5R370 however how can this be solved? I think if returning undefined from that function initializer should not re-assign the value to the instance; that would make the Object.defineProperty work perfectly fine.

I suppose you could use a class decorator that wraps the constructor and reconfigures the field at the end - otherwise, currently you can’t.

Yeah a class decorator definitely can be of help. Also this #469 (comment) has a lot of sense for why has not been implemented this way. @ljharb thank you so much.