Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to mark array methods as readonly #36498

Closed
bradzacher opened this issue Jan 29, 2020 · 3 comments
Closed

Unable to mark array methods as readonly #36498

bradzacher opened this issue Jan 29, 2020 · 3 comments
Labels
Duplicate An existing issue was already created

Comments

@bradzacher
Copy link
Contributor

bradzacher commented Jan 29, 2020

TypeScript Version: 3.7.5

Search Terms: array, methods, readonly

Code

class Foo {
    x(): void { }
    notReadonly = 1;
}

function foo1(arg: Readonly<Foo>) {
    arg.x = () => { };   // error ✅
    arg.notReadonly = 2; // error ✅
}

function foo2(arg: readonly string[]) {
    arg.length = 1;          // error ✅
    arg.forEach = () => { }; // no error ❌ - #22315
}
function foo3(arg: Readonly<ReadonlyArray<string[]>>) {
    arg.length = 1;          // error ✅
    arg.forEach = () => { }; // no error ❌
}

interface MyArray<T> extends Array<T> {
}

function foo4(arg: MyArray<string>) {
    arg.length = 1;          // no error ✅
    arg.forEach = () => { }; // no error ✅
}
function foo5(arg: Readonly<MyArray<string>>) {
    arg.length = 1;          // error ✅
    arg.forEach = () => { }; // error ✅
}

Expected behavior:
ReadonlyArray<T> and readonly T[] methods should be immutable by default, or should be able to be marked as immutable via Readonly<T>.

Actual behavior:
The typechecker has special handling under the hood which detects an array inside of Readonly<T> converts it to the ReadonlyArray type.

I.e.

Readonly<string[]> === readonly string[]               ✅
Readonly<readonly string[]> === readonly string[]      ❌
Readonly<Array<string>> === readonly string[]          ✅
Readonly<ReadonlyArray<string>> === readonly string[]  ❌

More Info:

  • Methods are not readonly-by-default, and there's no direct way to declare a readonly method (Suggestion: readonly method #22315).
    • Classes/objects can, however, have their methods marked as readonly via Readonly<Class>.
  • If you extend the array type (interface MyArray<T> extends Array<T> {}), then it works as expected, because the typechecker no longer thinks the type is an array type (checker.isArrayType(type) === false).
  • I ran into this whilst building a rule which asserts that all function arguments are readonly (Rule proposal: Enforce read-only typescript-eslint/typescript-eslint#514).

Playground Link:
https://www.typescriptlang.org/play/?ssl=1&ssc=1&pln=30&pc=2#code/MYGwhgzhAEBiD29oG8BQ0PQB4AoCUAXNAG7wCWAJitAL7qYB28ALgEoCmYF8DIAntAC80AIwBuVHVQAzAK4NgzMj2jTEInGABOAcyIcuPfgB4E8AHx4U9DNp0A6LEOj4h56jTGYA9N+jstLXgtaEBQchtoO3smNk5uXgFhACYvX39A4LDJVBl5RWUGVUQkzV0iLTijAQhmLTIGHQBtAF0rNExI3XsQdgbmAAtncQ6R6DSAoJDwjqi1LQBRMGBB4VdBd2RaVL8mdMnoQBlybLkFJRU1eABmUr1oA3iTe6qAQUCwPmMauoaW80trGZdHp9FaiLyjHx+CaZaaYWbBRbLZxrDZbMY7JDQkJHKT1ZgBaRLdjQACyfFeWnexgAKu52Fh8QwKDAKVTadYpCd8udEAAWG5EMmsj5feo6f7tOFA3o6AZDcEQ9HQXZYrKAhxzRGglEebbKzEZKbHPJnQoXACsArulQSxiFbxFtTFfzaESiwNloOGiqVqthti6mqW2qs611vsNWRoQA

Related Issues:
#22315

@RyanCavanaugh
Copy link
Member

Duplicate #35313

@bradzacher
Copy link
Contributor Author

I don't think it's a duplicate, from my understanding of that issue.

This is specifically about being able to mark methods on the array type as readonly via Readonly<T>, so that you cannot reassign the methods.

#35313 is about making Readonly<T> remove methods from object types.

I don't want to remove anything - I just want to be add the readonly modifier to methods via Readonly<T>, which works fine for everything except array types.

@RyanCavanaugh RyanCavanaugh added the Duplicate An existing issue was already created label Jan 29, 2020
@typescript-bot
Copy link
Collaborator

This issue has been marked as a 'Duplicate' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

3 participants