Interface with readonly property is assignable to interface with mutable property

[danielearwicker]

TypeScript Version: 2.1.4

Code

interface MutableValue<T> {
    value: T;
}

interface ImmutableValue<T> {
    readonly value: T;
}

let i: ImmutableValue<string> = { value: "hi" };
i.value = "Excellent, I can't change it"; // compile-time error

let m: MutableValue<string> = i;
m.value = "Oh dear, I can change it";

Expected behavior:
The assignment of i to m would fail, to stop us accidentally allowing value to be modified.

Actual behavior:
The assignment is allowed.

The current behaviour was a deliberate choice so this is a breaking change (or strict flag) feature request rather than a bug report!

The Handbook has this snippet:

let a: number[] = [1, 2, 3, 4];
let ro: ReadonlyArray<number> = a;
ro[0] = 12; // error!
ro.push(5); // error!
ro.length = 100; // error!
a = ro; // error!

It notes that a = ro being an error is helpful. But this happens because ReadonlyArray has no push method, making it incompatible with Array.

My example above seems "morally equivalent" to modelling the input/output flow of values with separate methods:

interface MutableValue<T> {
    getValue(): T;
    setValue(v: T): void;
}

interface ImmutableValue<T> {
    getValue(): T;
}

declare let i: ImmutableValue<string>;
i.setValue("Excellent, I can't change it"); // compile-time error

let m: MutableValue<string> = i;
m.setValue("Oh dear, I can change it");

And sure enough, this stops the assignment of i to m.

Would be great if mutable and readonly properties had the same relationship as if they were modelled by separate get/set methods (which of course they might actually be, via property getter/setters).

[danielearwicker]

Judging from #6614 there was an idea to add a mutable modifier. If I'm correct in assuming that this would mean:

interface MutableValue<T> {
    mutable value: T;
}

interface ImmutableValue<T> {
    readonly value: T;
}

and that would stop i being assigned to m because I've explicitly said that value must be mutable, then this whole issue can be boiled down to "Please add the mutable modifier"! 👍

[aluanhaddad]

@danielearwicker I think you are channeling @Aleksey-Bykov and his issue readonly modifiers are a joke

[danielearwicker]

Sometimes I think I'm way too polite!

[DavidKDeutsch]

I don't see an obvious "vote for this" button, so count this as my +1. Just ran into a nasty bug of mine where I was incorrectly modifying a member of a class instance, so I changed the type using ReadOnly, hoping I would get a compile error to quickly point my lazy self to there I was assigning it to a "normal" T, but no luck. As far as I am concerned, there is no difference between allowing an assignment of a type ReadOnly to a T and allowing an assignment of an Object to, say, an Array.

[Pauan]

@DavidKDeutsch In the upper-right corner of every comment, there is a +:grinning: button. If you click that, you can choose the :+1: reaction. This counts as a plus vote. You can do this on any comment, but often it is done on the first comment.

[dead-claudia]

Here's a repro demonstrating the issue online.

[dead-claudia]

The correct behavior in that repro would have the fifth line (let roAsRw: RW = ro) be an error.

[leoasis]

Has adding this behavior behind a configuration flag been already discussed internally or externally? If so, what has been the outcome of that? I think adding this behavior as a feature flag would encourage people to try this and work with library definitions to add readonly modifiers as needed. Then at some point in the future in a major version of Typescript this could be set by default.

[aluanhaddad]

@leoasis #13002

[Ailrun]

@RyanCavanaugh Is there any progress?

[EisenbergEffect]

I'd love to see if there's any progress on this. I was just teaching some engineers about TypeScript today, showing different aspects of interfaces, optional and readonly properties. I went a little off my script and showed something like above....which didn't work how I would have expected.

I'd love to see the readonly constraint respected. It's particularly relevant for some of the work we're doing on the vNext of Aurelia, which is all TypeScript. We also have a lot of immutable scenarios in my day job's codebase where it would be nice to get some help from the compiler...

[DavidSouther]

I think this is more likely to come up when calling a mutating function:

interface Foo {
  id: string;
}

const foo: Readonly<Foo> = {
  id: 'original ID',
}

function mutateFoo(foo: Foo) {
  foo.id = 'new ID';
}

mutateFoo(foo);

As in the prior cases, it's "obvious" that a coercion is happening, here mutateFoo is possibly deep in some library I have no idea about.

If readonly were a narrowing modifier and that were prevented, I could have (in 2.8+) something like:

type Mutable<T> = {
  -readonly [k in typeof T]: T[k]
}

foo = mutateFoo(Mutable<foo>); // Hey look I know foo will be mutated and explicitly opted in!

[iliubinskii]

Returning to the code sample in the first post...
You are welcome to stop your Don Quixote windmill fighting and just use an eslint rule (misc/typescript/no-unsafe-object-assignment) that catches this and one more unsafe pattern:

[RyanCavanaugh]

Hey folks, let's dial it down a notch. Remember the principles of the code of conduct

In no particular order:

• If this feature request were ill-defined or beyond the scope of the language, rest assured I would have closed it already. It isn't.
• Accusations of bad faith engagement should not be tossed around lightly
• That said, if you can't engage with the substance of someone's comment without nitpicking (picking at the use of certain words, purposefully taking code substrings out of context, etc), that is bad faith engagement and I'll be asking you to refrain from participation

I don't see any block-worthy behavior here but multiple people are quite close to crossing that line and I'd obviously prefer to not have to do that. Thanks!

[aboodman]

@RyanCavanaugh - can you please comment on the status of this bug from the project's perspective? I don't see any official comment from the ts team.

[RyanCavanaugh]

It's in the long list of features which are plausible, but aren't currently a priority based on the overall tradeoff of what would be gained here (write safety under aliasing, which is already a soundness hole as relates to the types of the properties themselves) vs what would be required for anyone to be able to turn the option on (generally speaking, "everyone" would have to properly document readonly for it to be usable in practice).

[adrian-gierakowski]

generally speaking, "everyone" would have to properly document readonly for it to be usable in practice

It’s a chicken and egg problem

[cedw032]

This is a pretty massive hole.

What do we have to do to get some movement on this?

Does anyone know why this keyword was added when it provides next to no protection?

EDIT: To be clear, I am asking if someone could point me towards the PR or issue that led to the addition of this feature so I can establish what the intention was, and how it ended up in the state that it's in. I am sure it was done with good intentions, and it seems unfortunate that it is stuck half way.

[MartinJohns]

Relevant comment by RyanCavanaugh: #58236 (comment)

We're considering closing the { readonly x: number } -> { x: number } soundness hole under a flag, [..]

[ahejlsberg]

Now implemented in #58296.