You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
No type error on length-mutating methods with readonly length #56685
functionwithFrozenLength<Aextendsunknown[]>(ar:A){constresult=Array.from(ar)Object.defineProperty(result,"length",{writable:false})returnresultasA&{readonlylength: A['length']}}constmyArray=withFrozenLength([1,2,3])myArray.push(4)// No type error. Runtime error.
🙁 Actual behavior
Typescript accepts the above code. The code crashes at runtime with a runtime TypeError like:
Uncaught TypeError: Cannot assign to read only property 'length' of object '[object Array]'
🙂 Expected behavior
The push method should be rejected by the type checker.
Additional information about the issue
Partial overlap with a similar issue involving tuple types #40316
Based on what? Your type clearly states it has such a method. The existence of a read-only length property does not change the rest of the type.
The push method assigns the length property. If I were to do the effective equivalent, myArray[myArray.length++]=4, it would be rightly rejected as mutating a readonly property.
Relatedly, making a readonly Array type makes TypeScript think it doesn’t have a push method at all, which seems like it will lead to incorrect flow analysis.
But that's an implementation detail of your method. The type system is completely unaware of this. Besides that, using a type assertion as you tell the compiler "trust me, this is the type", and when your implementation does not match this type you're eventually going to have a bad time.
Relatedly, making a readonly Array type makes TypeScript think it doesn’t have a push method at all
Writing readonly Array<T> will make it a ReadonlyArray<T>, which does not include any mutating methods. That's the correct type to use when you want to provide a read-only array.
This method is intentionally generic; it does not require that its this value be an Array. Therefore it can be transferred to other kinds of objects for use as a method.
It’s not merely an implementation detail - it’s a feature of the push method.
TypeScript doesn't know what push does beyond the types of its parameters and return type - it's just another method call, like any other. The compiler doesn't look at the implementation of functions when you call them, it only checks whether the argument types are compatible. It's no different to this:
constfoo: {readonlyfoo: string}={foo: "bar"};fooey(foo);// no errorconsole.log(foo.foo);// prints "qux"functionfooey(foo: {foo: string}){foo.foo="qux";}
readonly is not as strong a guarantee as you think it is. It just means you can't mutate the property locally.
Writing readonly Array<T> will make it a ReadonlyArray<T>, which does not include any mutating methods. That's the correct type to use when you want to provide a read-only array.
The absence of a method is unfortunately different from saying that calling the method is illegal. For instance, it can be implicitly re-introduced by narrowing:
The lack of those properties does prevent accidentally treating a ReadonlyArray as an Array, which is good. But it doesn't do much for preventing accidental mutation by generic methods.
TypeScript doesn't know what push does beyond the types of its parameters and return type - it's just another method call, like any other.
The type of push can also depend on the type of this, just as map does.
readonly is not as strong a guarantee as you think it is. It just means you can't mutate the property locally.
I'm realizing this, and I think it might be a limitation of the TypeScript type system that you can't meaningfully encode a readonly property in a way that it can't be implicitly forgotten. My attempt below by declaring getters and setters does not achieve the desired result:
// @target: ES2022typeT=symbol;functionMake<T>(): T{thrownewError("unimplementable")}// Array versus readonly ArrayMake<ReadonlyArray<T>>()satisfiesArray<T>;Make<Array<T>>()satisfiesReadonlyArray<T>;// ReadonlyArray < Array// property versus readonly propertyMake<{readonlya: T}>()satisfies{a:T}Make<{a: T}>()satisfies{readonlya:T}// property and readonly property are freely interconvertible// property implementationtypeGetX={getx(): Tsetx(value: never)}typeSetX={setx(value: T)getx(): unknown}typeGetSetX={x: T}Make<GetSetX>()satisfiesGetX;Make<GetX>()satisfiesGetSetX;// GetX and GetSetX are freely interconvertible,Make<GetX>()satisfiesSetX;Make<SetX>()satisfiesGetX;// GetX < SetXMake<GetSetX>()satisfiesSetX;Make<SetX>()satisfiesGetSetX;// GetSetX < SetX
You can't represent "illegal methods" in TypeScripts type-system.
I would think you could by making the function signature require an argument of type never. Or by having some failing extends constraint.
And you can't represent immutability in TypeScripts type-system.
Yeah. I don’t understand why my above implementation of GetX decays into GetSetX given that function types should be contravariant in their argument types.
With that argument everything is pointless, because any lets you do all kind of shenanigans.
Emphasis on “implicitly”. Yes, any is the “hold my beer” of TypeScript 🥲.
———-
At any rate, I think this issue is a design limitation (at least unless/until #13347 is resolved)
#13347 wouldn't change anything, because, again, your type says it has a push method, so it's possible to call the push method. Having a read-only length property does not magically make that method disappear.
It's valid to call functions expecting an argument of never when you have a value typed never. This is useful e.g. for a panic function.
Yes. Such a function is callable only by constructing a value of type never (or any). That is, only by using unsound parts of the type system.
#13347 wouldn't change anything, because, again, your type says it has a push method, so it's possible to call the push method. Having a read-only length property does not magically make that method disappear.
It wouldn’t fix the issue directly, but it would allow either declaring a function push({length:number}, /*…*/) so that calling it with a {readonly length:number} would require first explicitly narrowing away the readonly modifier.
To make a push method safe, you’d do something similar with a constraint on the type of this.
readonly doesn't mean immutable. A readonly property is one that you don't have write access to. It doesn't mean that the property value itself cannot change. There's nothing wrong, inconsistent, or unsound with an object having a readonly property that changes as a side effect of some other operation.
readonly doesn't mean immutable. A readonly property is one that you don't have write access to. It doesn't mean that the property value itself cannot change. There's nothing wrong, inconsistent, or unsound with an object having a readonly property that changes as a side effect of some other operation.
That’s true. A readonly property may change as the side effect of another operation. That is expressly not what happens here.
The Array.prototype.push function has no special access to the private underlying length. Per spec, it goes through the public length setter. That’s why making the property unwritable with Object.defineProperty results in a runtime TypeError.
I fail to see how an operation which calls the setter could possibly be in semantic agreement with something declared readonly. Design limitation, sure. But also semantically wrong.
Activity
MartinJohns commentedon Dec 6, 2023
Based on what? Your type clearly states it has such a method. The existence of a read-only
lengthproperty does not change the rest of the type.rotu commentedon Dec 6, 2023
The
pushmethod assigns thelengthproperty. If I were to do the effective equivalent,myArray[myArray.length++]=4, it would be rightly rejected as mutating a readonly property.Relatedly, making a
readonly Arraytype makes TypeScript think it doesn’t have apushmethod at all, which seems like it will lead to incorrect flow analysis.MartinJohns commentedon Dec 6, 2023
But that's an implementation detail of your method. The type system is completely unaware of this. Besides that, using a type assertion
asyou tell the compiler "trust me, this is the type", and when your implementation does not match this type you're eventually going to have a bad time.Writing
readonly Array<T>will make it aReadonlyArray<T>, which does not include any mutating methods. That's the correct type to use when you want to provide a read-only array.I don't see anything wrong with this behavior.
rotu commentedon Dec 6, 2023
The ECMAScript spec has a note:
It’s not merely an implementation detail - it’s a feature of the
pushmethod.fatcerberus commentedon Dec 6, 2023
TypeScript doesn't know what
pushdoes beyond the types of its parameters and return type - it's just another method call, like any other. The compiler doesn't look at the implementation of functions when you call them, it only checks whether the argument types are compatible. It's no different to this:readonlyis not as strong a guarantee as you think it is. It just means you can't mutate the property locally.rotu commentedon Dec 6, 2023
The absence of a method is unfortunately different from saying that calling the method is illegal. For instance, it can be implicitly re-introduced by narrowing:
The lack of those properties does prevent accidentally treating a
ReadonlyArrayas anArray, which is good. But it doesn't do much for preventing accidental mutation by generic methods.The type of
pushcan also depend on the type ofthis, just asmapdoes.I'm realizing this, and I think it might be a limitation of the TypeScript type system that you can't meaningfully encode a
readonlyproperty in a way that it can't be implicitly forgotten. My attempt below by declaring getters and setters does not achieve the desired result:MartinJohns commentedon Dec 6, 2023
You can't represent "illegal methods" in TypeScripts type-system. And you can't represent immutability in TypeScripts type-system.
With that argument everything is pointless, because
anylets you do all kind of shenanigans.fatcerberus commentedon Dec 6, 2023
Indeed; I was implicitly including
thisas a parameter when I said “parameters and return type”rotu commentedon Dec 8, 2023
I would think you could by making the function signature require an argument of type
never. Or by having some failingextendsconstraint.Yeah. I don’t understand why my above implementation of
GetXdecays intoGetSetXgiven that function types should be contravariant in their argument types.Emphasis on “implicitly”. Yes,
anyis the “hold my beer” of TypeScript 🥲.———-
At any rate, I think this issue is a design limitation (at least unless/until #13347 is resolved)
MartinJohns commentedon Dec 8, 2023
It's valid to call functions expecting an argument of
neverwhen you have a value typednever. This is useful e.g. for a panic function.#13347 wouldn't change anything, because, again, your type says it has a
pushmethod, so it's possible to call thepushmethod. Having a read-onlylengthproperty does not magically make that method disappear.rotu commentedon Dec 8, 2023
Yes. Such a function is callable only by constructing a value of type
never(orany). That is, only by using unsound parts of the type system.It wouldn’t fix the issue directly, but it would allow either declaring a function
push({length:number}, /*…*/)so that calling it with a{readonly length:number}would require first explicitly narrowing away thereadonlymodifier.To make a push method safe, you’d do something similar with a constraint on the type of
this.RyanCavanaugh commentedon Dec 8, 2023
readonlydoesn't mean immutable. Areadonlyproperty is one that you don't have write access to. It doesn't mean that the property value itself cannot change. There's nothing wrong, inconsistent, or unsound with an object having areadonlyproperty that changes as a side effect of some other operation.rotu commentedon Dec 9, 2023
That’s true. A
readonlyproperty may change as the side effect of another operation. That is expressly not what happens here.The
Array.prototype.pushfunction has no special access to the private underlying length. Per spec, it goes through the publiclengthsetter. That’s why making the property unwritable withObject.definePropertyresults in a runtimeTypeError.I fail to see how an operation which calls the setter could possibly be in semantic agreement with something declared
readonly. Design limitation, sure. But also semantically wrong.typescript-bot commentedon Dec 12, 2023
This issue has been marked as "Working as Intended" and has seen no recent activity. It has been automatically closed for house-keeping purposes.