Double initializing a `readonly` field should not be allowed

[dgp1130]

🔎 Search Terms

readonly double twice initializer

🕗 Version & Regression Information

• This is the behavior in every version I tried, and I reviewed the FAQ for entries about initializers?

⏯ Playground Link

https://www.typescriptlang.org/play/?#code/MYGwhgzhAEBiD29oG8BQ0PQE4FMwBN4A7EAT2gCMwsAuaCAFywEsiBzaAXmgHJXmGzMCGYAvHFmgBGHgG5U6TMGKMsAV2AN4WABQBKFIsyYGAC2YQAdFUnc+RAUJHjJAJjlHoAX1Q-Uyogh4EBxLEHg2HSIcAHc4RH1raj1ZIA

💻 Code

class Foo {
    readonly bar: string = 'initializer 1';

    constructor() {
        this.bar = 'initializer 2';
    }
}

console.log(new Foo().bar);

🙁 Actual behavior

This compiles successfully despite overwriting a readonly field. This breaks expectations around readonly. If Foo happens to be a very large class and I only look at readonly bar: string = 'initializer 1'; in isolation, I would expect any other line which reads bar to see initializer 1. However this is not a safe assumption because I need to check in the constructor to ensure it doesn't overwrite my readonly field.

🙂 Expected behavior

I expected a compiler error. TS should only allow initializing readonly fields in a constructor, not overwriting a field which has already been initialized.

Additional information about the issue

I get that we need readonly fields to be assignable in the constructor, but it feels like a bug to me that this is allowed when the field has already been initialized prior to the constructor. readonly should require a field initializer xor a constructor initializer. Having both is invalid IMHO.

You can make the same argument that double initializing in the constructor should be invalid too:

class Foo {
  readonly bar: string;

  constructor() {
    this.bar = 'initializer 1';
    this.bar = 'initializer 2'; // Should maybe error? Doesn't today.
  }
}

But I understand that control flow analysis gets a lot more complicated in the constructor WRT readonly and might be considered a different feature with different priority.

[DanielRosenwasser]

Is there a reason why a double-assignment came up for you? Immutable-after-creation is a very common pattern for a lot of people.

class Executable {
  readonly exePath: string;

  constructor(exeName: string) {
    this.exePath = path.join(EXE_DIR, exeName);
    if (isWindows()) {
      this.exePath += ".exe";
    }
  }
}

I'm actually kind of surprised, but Java has the behavior you're describing for final members, whereas C# is okay with multiple assignments for readonly members.

[dgp1130]

Is there a reason why a double-assignment came up for you?

I genuinely made this mistake. The full context is that I had a class which was converting a record to a Map in the constructor.

class Foo {
  private readonly myMap = new Map<string, string>();

  constructor(data: Record<string, string>) {
    this.myMap = new Map(Object.entries(data));
  }
}

I think in my brain, not paying that much attention, the new Map<string, string>() field initializer was defining the type, not assigning a value. Later I wanted to update the Map to a ReadonlyMap, so I did:

private readonly myMap = new ReadonlyMap<string, string>();

But ReadonlyMap is a type, not a value. At that point I realized I was double-initializing and creating an empty map just to throw it away and replace it with another map. What I actually wanted was:

class Foo {
  private readonly myMap: ReadonlyMap<string, string>;

  constructor(data: Record<string, string>) {
    this.myMap = new Map(Object.entries(data));
  }
}

I only noticed I was doing anything wrong when I wanted to switch to ReadonlyMap, but my mistake was when I added the field initializer in the first place. If TS had warned me about that, it would prevented the pointless empty Map creation.

Your Executable example does make some sense, as much as I personally find this behavior unintuitive (see the readonly bits of my constructors rant) and is partially why I called out the control flow in constructors as a separate problem. I think I'm more ok with that behavior than I am with mixing field and constructor initialization for readonly properties.

Combining constructor initialization with field initializer is noticeably more confusing IMHO. Would you consider this to be valid?

class Executable {
  readonly exePath: string = 'path/to/my/binary';

  constructor() {
    if (isWindows()) {
      this.exePath += '.exe';
    }
  }
}

If so, then you can probably close this issue as WAI.

Though IMHO you have to evaluate the field initializer and constructor independently, as they could be in a large class very far away from each other.

If I see:

readonly exePath: string;

Then I know exePath must be assigned in the constructor and is immutable from that point onward. That immediately draws my attention to the constructor as being related to exePath initialization, where I would find multiple assignments if they were needed.

However if I see:

readonly exePath: string = 'path/to/my/binary';

I think many people might (at least I would) misinterpret that to mean that exePath is immutably assigned to path/to/my/binary and never even consider that the constructor might mutate it. Maybe that's just an incorrect understanding of readonly, but I think it is at least a reasonable expectation for a developer to have.

Personally, I argue utilizing this behavior is bad style, as the field initializer is misleading in what it does. I would recommend against mutating an initialized field in a constructor at all and if you want to do that, you should initialize the field from scratch in the constructor.

class Executable {
  // Requiring an uninitialized field draws my attention to the constructor.
  readonly exePath: string;

  constructor() {
    this.exePath = 'path/to/my/binary';
    if (isWindows()) {
      this.exePath += '.exe';
    }
  }
}

It's possible that's more opinionated than the TS team wants to be and this should maybe be handled in a lint check instead, but it at least feels like an opportunity for an improvement.

[MartinJohns]

and is immutable from that point onward.

You will be shocked to learn that readonly does not mean immutable. It's only read-only via that interface, but read-only versions are implicitly compatible with their mutable counterparts.

const src: { readonly value: string } = { value: "hello" };
const copy: { value: string } = src;
copy.value = "world";
src.value; // Has no longer its initial value.

You can't represent immutable types in TypeScripts type-system as of today.

[RyanCavanaugh]

As Daniel noted, this has conflicting precedents in other OO languages, and we haven't gotten any other feedback like this yet after having had readonly class fields for quite a while. Leaving open to see if other people are hitting this -- if not, it seems unlikely we'd want to break code that is doing this on purpose.