jayphelps/core-decorators

@nonenumerable makes property readonly in typescript using strict

Opened this issue · 12 comments

Hi,
then I try to use nonenumerable decorator, I cannot write to the variable. I receive this exception: Cannot assign to read only property. Is this expected behavior?

@janjilek can you please provide reproducible example code?

Same issue.

Example is simple:

class Foo {
  @nonenumerable
  property = {
    foo: 1,
    bar: 1,
  }
}

new Foo();

In console:

Uncaught TypeError: Cannot assign to read only property 'property' of object '#'

Typescript Compiler 1.8.10

May be it will be helpfull, generated code is

"use strict";

var _typeof = typeof Symbol === "function" && typeof Symbol.iterator === "symbol" ? function (obj) { return typeof obj; } : function (obj) { return obj && typeof Symbol === "function" && obj.constructor === Symbol ? "symbol" : typeof obj; };

function _classCallCheck(instance, Constructor) { if (!(instance instanceof Constructor)) { throw new TypeError("Cannot call a class as a function"); } }

var __decorate = undefined && undefined.__decorate || function (decorators, target, key, desc) {
    var c = arguments.length,
        r = c < 3 ? target : desc === null ? desc = Object.getOwnPropertyDescriptor(target, key) : desc,
        d;
    if ((typeof Reflect === "undefined" ? "undefined" : _typeof(Reflect)) === "object" && typeof Reflect.decorate === "function") r = Reflect.decorate(decorators, target, key, desc);else for (var i = decorators.length - 1; i >= 0; i--) {
        if (d = decorators[i]) r = (c < 3 ? d(r) : c > 3 ? d(target, key, r) : d(target, key)) || r;
    }return c > 3 && r && Object.defineProperty(target, key, r), r;
};
var __metadata = undefined && undefined.__metadata || function (k, v) {
    if ((typeof Reflect === "undefined" ? "undefined" : _typeof(Reflect)) === "object" && typeof Reflect.metadata === "function") return Reflect.metadata(k, v);
};
var core_decorators_1 = require('core-decorators');

var Foo = function Foo() {
    _classCallCheck(this, Foo);

    this.property = {
        foo: 1,
        bar: 1
    };
};

__decorate([core_decorators_1.nonenumerable, __metadata('design:type', Object)], Foo.prototype, "property", void 0);
exports.Foo = Foo;

AFAICT the issue is that TypeScript is not generating an initial descriptor that all our decorators expect is being provided.

So what happens is, the descriptor ends up just looking just like this:

{ enumerable: false }

Which means the defaults of Object.defineProperty will be used, including writable: false.

This behavior is contrary to the behavior Babel v5 had, as well as babel-plugin-transform-decorators-legacy and again AFAICT contrary to every iteration of the decorators spec.

This appears at first glance to be a deviation from the proposed decorators spec and I'm not sure I can work around this. 😢

This appears to be a known issue for property initializers only, they're choosing not to address at the moment:

NOTE  A Property Descriptor is not provided as an argument to a property decorator due to how property decorators are initialized in TypeScript. This is because there is currently no mechanism to describe an instance property when defining members of a prototype, and no way to observe or modify the initializer for a property. As such, a property decorator can only be used to observe that a property of a specific name has been declared for a class.

http://www.typescriptlang.org/docs/handbook/decorators.html#property-decorators

The solution (right or wrong) that Babel took was providing a default descriptor.

e.g.

(TEMP = Object.getOwnPropertyDescriptor(TARGET, PROPERTY), (TEMP = TEMP ? TEMP.value : undefined), {
    enumerable: true,
    configurable: true,
    writable: true,
    initializer: function(){
        return TEMP;
    }
})

All of this can be attributed to the fact that the decorators spec is very much non-trivial to implement. The spec was and still is rough and ambiguous at this stage, so some transpilers will choose to fill in the gaps with assumptions while others will refuse.

I'm running into this issue too. We have to explicitly set writable = true in the descriptor so we can write to the property while keeping it non enumerable. It would be useful to have a @writable decorator or, even better, an option to set writable=true in the @nonenumerable decorator. (something like @nonenumerable(writable=true))

I proposed a patch where @nonenumerable sets writable=true, unles the user specifies @nonenumerable({ writable: false })

@reinert hmm not sure of the solution you're proposing but if it's easy enough could you submit a PR with tests?

Just got stuck on the same issue 😕 Any progress on this?

TS implementation of decorators different from babel's, so there can't be any progress. This library designed for ES decorators.

By the way, if you need @nonenumerable decorator in typescript, you can pick my version:

export function nonenumerable(target: any, key: string) {
  // first property defined in prototype, that's why we use getters/setters
  // (otherwise assignment in object will override property in prototype)
  Object.defineProperty(target, key, {
    get: function() {
      return undefined;
    },
    set: function(this: any, val) {
      // here we have reference to instance and can set property directly to it
      Object.defineProperty(this, key, {
        value: val,
        writable: true,
        enumerable: false,
      });
    },

    enumerable: false,
  });
}

@thekip Thanks! Way more robust than the one I was going for.

endel commented

@thekip works great! I've extracted your implementation into a standalone module here: https://github.com/endel/nonenumerable