readonly fields should be writable: false

[Nokel81]

Suggestion

When useDefineForClassFields is enabled readonly and static readonly fields should be emitted with writable: false.

🔍 Search Terms

• readonly
• fields
• define

✅ Viability Checklist

My suggestion meets these guidelines:

• This wouldn't be a breaking change in existing TypeScript/JavaScript code
• This wouldn't change the runtime behavior of existing JavaScript code
• This could be implemented without emitting different JS based on the types of the expressions
• This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
• This feature would agree with the rest of TypeScript's Design Goals.

⭐ Suggestion

Emit writable: false when a field is readonly and useDefineForClassFields is enabled

📃 Motivating Example

readonly fields now are runtime readonly too.

[jcalz]

You checked “This could be implemented without emitting different JS based on the types of the expressions”

but isn’t this specifically asking for different JS based on information from the static type system?

[Nokel81]

Maybe I misunderstood what that meant, but to me a field declaration is not an expression. It is a different kind of parser entry. Namely, a declaration.

[jcalz]

Well, I expect this will be declined as it does not allow type erasure.

[Nokel81]

How is it not allowing type erasure? There is already a well defined JS way of doing just this. I was slightly surprised that turning on useDefineForClassFields didn't do this anyway.

[MartinJohns]

Also be aware that readonly properties are implicitly compatible with the mutable counterpart. Your class can implement interfaces where the property is not marked readonly. I don't know well enough what writable actually does to determine whether this has an impact.

[justinfagnani]

Hopefully this is possible at runtime with the new TC39 decorators proposal, and there would be a way for TypeScript to recognize the property descriptor modifications made by the decorator so the readonly keyword isn't necessary.

[Nokel81]

I don't know well enough what writable actually does to determine whether this has an impact.

It is similar to how Object.freeze works. Namely, it is shallow and in normal mode the assignments are ignored. In strict mode a TypeError is thrown.

[Nokel81]

Hopefully this is possible at runtime with the new TC39 decorators proposal.

I don't quite understand this, yes the configuration is done at run time but it really should be done in a constructor. The annoying thing is is that without this proposal, the equivalent code is rather obtuse:

---

With proposal:

class Foo {
  readonly foo = "1";
}

const f = new Foo();

(f as any).foo = "2"; // error

Current situation:

interface Foo {
  readonly foo: string;
}

class Foo {
  constructor() {
    Object.defineProperty(this, "foo", {
      enumerable: true,
      configurable: true,
      writable: false,
      value: "1"
    });
  }
}

const f = new Foo();

(f as any).foo = "2"; // error

The current situation is necessary so that a second Object.defineProperty isn't emitted in the constructor.

[MartinJohns]

(f as any).foo = "2"; // error

But why should that be an error? readonly only applies to the specific TypeScript type. It does not mean immutable, only "can't write via this type". Like I mentioned, the mutable counter-part is implicitly compatible:

class Foo { readonly foo = "1"; }
class MFoo { foo = "1"; }

function changeFoo(x: MFoo) { x.foo = "abc"; }

const foo = new Foo();
changeFoo(foo);

[Nokel81]

But why should that be an error? readonly only applies to the specific TypeScript type. It does not mean immutable, only "can't write via this type". Like I mentioned, the mutable counter-part is implicitly compatible:

Well that is what I am proposing. Namely a change.

And furthermore, I know that you can change the readonly-ness of child classes too. But that is compatible with my proposal because I am not talking about changing the configurable flag.

[jcalz]

How is it not allowing type erasure?

Because currently the readonly modifier is not part of ECMAScript; it's part of TypeScript's static type system. It sounds like you want the type system to affect the emitted JavaScript, which is counter to type erasure.

There is already a well defined JS way of doing just this. I was slightly surprised that turning on useDefineForClassFields didn't do this anyway.

If you're suggesting that the readonly modifier should be considered part of class fields at runtime and not purely part of the static type system, then sure, this isn't a type erasure issue. But then you're asking for a runtime feature involving "non-ECMAScript syntax with JavaScript output". TypeScript won't introduce such new runtime features; such requests would belong in https://github.com/tc39/proposal-class-fields/, such as the (closed) tc39/proposal-class-fields#83.

Or am I missing what we are trying to achieve here?

[RyanCavanaugh]

The only non-type-directed-emit case I can see here would be class fields, and readonly class fields are intentionally writable in the constructor, so this is a no-go.

[RobertSandiford]

I'm guessing it we asked for a "nonwritable" tag that would emit code with { writable: false } this wouldn't be accepted due to being out-of-scope (as not a typings issue)? Just checking.

[RyanCavanaugh]

The only way we'd do that is if nonwritable was an ECMAScript keyword that had the same behavior (IOW, that proposal should go to TC39, not here)

[RobertSandiford]

Sure, thanks.

[c-harding]

It would be possible to emit Object.defineProperties at the end of the constructor, as subclasses aren’t allowed to set readonly properties in their constructor, unless the property is overwritten.

Example with useDefineForClassFields: true

class Super {
    readonly field: number = 1;
    readonly otherField: number = 0;

    constructor() {
        this.field = 2;
    }
}

class Sub extends Super {
    readonly field: number = 3;

    constructor() {
        const value = 4;
        super();
        this.field = value;
    }
}

"use strict";
class Super {
    constructor() {
        // field init
        Object.defineProperty(this, "field", {
            enumerable: true,
            configurable: true,
            writable: true,
            value: 1
        });
        Object.defineProperty(this, "otherField", {
            enumerable: true,
            configurable: true,
            writable: true,
            value: 0
        });

        // constructor
        this.field = 2;

        // after constructor: make all readonly properties non-writable
        Object.defineProperties(this, {
          field: { writable: false },
          otherField: { writable: false },
        });
    }
}
class Sub extends Super {
    constructor() {
        // part of constructor up to super call
        const value = 4;
        super();

        // field init
        Object.defineProperty(this, "field", {
            enumerable: true,
            configurable: true,
            writable: true,
            value: 3
        });

        // part of constructor after super call
        this.field = value;

        // after constructor: make all readonly properties non-writable
        Object.defineProperties(this, { field: { writable: false } });
    }
}

When useDefineForClassFields is false, it would be more complex, but presumably that combination wouldn’t be supported.

Example with useDefineForClassFields: false

class Super {
    readonly field: number = 1;
    readonly otherField: number = 0;

    constructor() {
        this.field = 2;
    }
}

class Sub extends Super {
    readonly field: number = 3;

    constructor() {
        const value = 4;
        super();
        this.field = value;
    }
}

"use strict";
class Super {
    constructor() {
        // field init
        this.field = 1;
        this.otherField = 0;

        // constructor
        this.field = 2;

        // after constructor: make all readonly properties non-writable
        Object.defineProperties(this, {
          field: { writable: false },
          otherField: { writable: false },
        });
    }
}
class Sub extends Super {
    constructor() {
        // part of constructor up to super call
        const value = 4;
        super();

        // after super call: make all properties that override readonly properties writable
        Object.defineProperty(this, { field: { writable: true } });

        // field init
        this.field = 3;

        // part of constructor after super call
        this.field = value;

        // after constructor: make all readonly properties non-writable
        Object.defineProperties(this, { field: { writable: false } });
    }
}

It would be great if this could be a compiler option @RyanCavanaugh

[justinfagnani]

@c-harding that would make this violate this guideline:

This wouldn't change the runtime behavior of existing JavaScript code

I think a decorator would be the TC39-approved way of accomplishing this.

[c-harding]

@justinfagnani How would such a decorator look, such that it is possible to set the property in the constructor but no later?

[c-harding]

@justinfagnani I had a go in this gist here, but it has slightly different semantics: I had to disallow overriding readonly properties, as it wasn’t possible to allow this using a decorator. Additionally, it seems like I would have to annotate both the field and the class (in addition to the readonly type). It feels like far more effort than if the typescript compiler could do this automatically