Skip to content

Readonly everything by default #42357

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

Open
5 tasks done
safareli opened this issue Jan 15, 2021 · 30 comments
Open
5 tasks done

Readonly everything by default #42357

safareli opened this issue Jan 15, 2021 · 30 comments
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript

Comments

@safareli
Copy link

safareli commented Jan 15, 2021

Suggestion

πŸ” Search Terms

βœ… 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

Right now default is that all record/array/tuple properties are mutable, and if you want any of them to be readonly/immutable you should add readonly flag or use Readonly<...>. My suggestion is add a flag (or something like that) which will "flip" this - it will turn on "assume everything is read only" in TS project(or module) and add a keyword mutable when you want to mark something as mutable.

πŸ“ƒ Motivating Example

When using are not mutating data that much and most of the types are assumed to be immutable while very little is mutable you might accidentally mutate something or when trying to understand portion of code, which mostly uses immutable values but some are mutable, you have one option to use readonly/Readonly.. but that code becomes quite noisy. With this flag you can turn on "readonlyByDefault" flag and everthing will be assumed to be readonly and you could mark mutable fields/values with mutable keyword. This way you would know exactly what's mutable easily and not mutate stuff accidentally.

πŸ’» Use Cases

Probably 99% of react-redux projects do not mutate objects/state or use libraries for immutable structures. Also some teams where folks are using immutable values (and other functional programing practices) would benefit a lot.


I proposed this initially here and then I noticed it had 25 πŸ‘ and suggestion to open separate proposal , which I did here.

@MartinJohns
Copy link
Contributor

Please don't confuse readonly and immutable. TypeScript does not provide any mechanism to represent immutable data structures. For this reason I wouldn't add a keyword "mutable", because that kinda implies the opposite is immutable.

@safareli
Copy link
Author

πŸ‘ Yeah, for example writable could be the name of the keyword. I don't have strong opinion on names whatever would be acceptable by maintainers/community is good for me.

@ghost
Copy link

ghost commented Jan 15, 2021

So, with the flag enabled, this would error:

const x: [ number, number ] = [ 3, 9 ];

because the rhs is "readonly," but the variable doesn't accept a readonly tuple.

The only way to solve that would be via casting, which would beat the purpose, would it not?

const x: [ number, number ] = [ 3, 9 ] as [ number, number ];

@MartinJohns
Copy link
Contributor

MartinJohns commented Jan 15, 2021

@00ff0000red From the proposal:

and add a keyword mutable when you want to mark something as mutable.

Instead of mutable being the default with the option to opt to readonly, the suggestion is to do it the other way around.

// Currently
const x1: [number, number] = [3, 9]; // Mutable
const x2: readonly [number, number] = [3, 9]; // Readonly

// Suggested
const x3: [number, number] = [3, 9]; // Readonly
const x4: mutable [number, number] = [3, 9]; // Mutable

@ghost
Copy link

ghost commented Jan 15, 2021

Oh, okay, I see. In that case, it wouldn't be nearly as bad, except maybe that all of the lib typings would need to be updated, e.g. all TypedArrays are inherently mutable.

Wait, then what would it deduct as?

const x5 = [ 3, 9 ]; // x: Readonly [ n, n ] ?

If by default, literals are deducted as read-only, then my first claim still applies, and we will still get errors like Readonly [ 3, 9 ] is not assignable to Mutable [ number, number ], everywhere.

@RyanCavanaugh RyanCavanaugh added Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript labels Jan 15, 2021
@artursvonda
Copy link

I think this would really useful feature since we'd prefer our code to be "immutable"/readonly by default and write exceptions for mutability. Currently it's very verbose.
I had basically exact same proposal in my head for this issue since it can be adopted gradually both – by each codebase and community. Utility type MutableArray and MutableInterface (or whatever) could be introduced in libraries that uses the new modifier on versions that support it and falls back to current behaviour on older versions.

@boris-kolar
Copy link

boris-kolar commented Jan 19, 2021

I love the idea, because I find myself writing readonly all the time. Immutable by default would also be very useful for compilers that compile TypeScript to wasm/native code (e.g. AssemblyScript, TypeScript Static). Also, allowing readonly for classes, interfaces and object types would be nice.

readonly interface Foo {
    foo: number // equivalent to: readonly foo: number
}

readonly class FooClass {
    static foo: number = 42 // equivalent to: static readonly foo: number = 42
    foo: number = 0 // equivalent to: readonly foo: number = 0
}

type Bar = readonly {
    bar: string // equivalent to: readonly bar: string
    baz: { z: number } // equivalent to: readonly baz: { readonly z: number }
}

In addition, I think methods should be readonly by default:

interface Foo {
    foo(): void
}
let foo: Foo = { foo: () => { } }
foo.foo = () => { throw 0 } // foo should be readonly by default

@ghost
Copy link

ghost commented Jan 19, 2021

Like @boris-kolar said, methods, prototypes, and classes should generally be immutable, by default. I'd also say that it wouldn't be too unreasonable to say that the entire global object should be readonly by default too.

I've written out readonly interfaces, and it's just painful writing readonly on every single line.

@lautarodragan
Copy link

Is this a dup of #32758?

@ghost
Copy link

ghost commented Jan 21, 2021

@lautarodragan Technicially, no, as this asks for "readonly" everything, whereas yours asks for "immutable" everything.
Otherwise, it seems so.

@isaac-weisberg
Copy link

isaac-weisberg commented Apr 26, 2021

Yeah, it makes no sense that I have to write readonly for every single interface member considering that this is my primary tool for encapsulation and access control

I can create a readonly member on a class that implements an interface with a member of the same name and type that would satisfy the interface requirement, but suddenly, it doesn't translate - readonly on the class, but if you hold onto the object by its interface, it's suddenly mutable...

@benwiley4000
Copy link

benwiley4000 commented Jul 29, 2021

Oh, okay, I see. In that case, it wouldn't be nearly as bad, except maybe that all of the lib typings would need to be updated, e.g. all TypedArrays are inherently mutable.

I'm sure whatever implementation this takes will need to basically apply this rule only to the project and not to external modules. Any 3rd party module typings would still be "writable by default" (unless the typings file somehow declares readonly-by-default, which could/should be an option).

I can see it potentially becoming annoying to have to declare any piece of non-primitive data passed to a third party library "writable," but I imagine this annoyance would lesson over time as more libraries adopt the readonly-by-default declaration.

In the meantime there could be two workarounds to avoid excessive writable definitions: A) defensive copying (to writable) before passing (when you really don't trust the library too much), and B) as writable casting while passing, which would be unsafe but acceptable in cases where you know the library doesn't mutate the data.

@MaxGraey
Copy link

MaxGraey commented Jul 30, 2021

currently:

const tup1 = [1, 2]                             // [number, number]
const tup2: readonly [number, number] = [1, 2]  // readonly [number, number]
const tup3 = [1, 2] as const                    // readonly [number, number]

new:

const tup1 = [1, 2]                    // readonly [number, number]
const tup2: [number, number] = [1, 2]  // [number, number]
const tup3 = [1, 2] as mut             // [number, number]
const tup4 = tup3 as const             // readonly [number, number]

@imcotton
Copy link
Contributor

imcotton commented Aug 7, 2021

giphy

https://github.com/tc39/proposal-record-tuple

@anilanar
Copy link
Contributor

anilanar commented Nov 8, 2021

This proposal, in its current form, doesn't address what happens to checking library code. For those who don't know, tsconfig.json flag applies to everything, including library code. What happens when a library function taking a mutable array as arg turns into a readonly array type-wise, but it actually mutates the array internally (because library source code is not type-checked, they are not included in node_modules)?

So I'm not sure how this proposal can work in a practical way without having a way to apply compiler flags (like jsx pragma) to subset of files.

@qpwo
Copy link

qpwo commented Dec 23, 2021

Re:

This proposal, in its current form, doesn't address what happens to checking library code. …
#42357 (comment)

Also considering this one: #42357 (comment)

Example I'll reference:

// myProjectFile.ts:
import { sorted } from 'somelibrary'
function smallestBiggest(supposedToBeReadonlyNums: number[]): [number, number] {
    const sortedArr = sorted(supposedToBeReadonlyNums) // line 4
    return [sortedArr[0], sortedArr.at(-1)]
}

// somelibrary/sorted.ts:
function sorted(arr: number[]): number[] {
    arr.sort((x, y) => x - y) // mutation!
    return arr
}

Not knowing anything about typescript's internals, (but as a heavy user) here's a few options – mix of things the compiler could do or the user could do – not sure if they're all feasible:

  1. Allow compiler flags to subset of files as you stated. Line 4 then throws no error? Or you put as writeable before the close paren? Not sure I understand @anilanar 's comment.
  2. Have a declare global block in your project somewhere stating which functions are writeable and which are readonly. This block could be moved to the lib typings with a PR eventually, and copy-pasted around meanwhile.
  3. Um sorry for asking this but how impossible is automatic detection of mutating operations ? … Like, the compiler can already detect violations of readonly so can't it automatically infer in all library code when a function violates it and the whole question is moot? This currently doesn't 100% work with untyped JS – or work at all?
  4. Again my knowledge is lacking here but would there be an easy way to disable the "Readonly everything by default" setting for a single file? Then you could wrap all the library functions you need without too much clutter. Similar to option 2.
  5. Objects/arrays originating from library code would be writeable but data from user factories would be readonly, right? Does this already avoid a lot of the annoying cases of casting stuff to/from readonly all the time? (Really uncertain about this one but throwing it in for the sake of variety.)

@TheUnlocked
Copy link

Rather than have readonly by default (which seems dangerous for interacting with libraries), I think it would make more sense for the flag to just disallow unannotated array/object types (and maybe a separate flag for whether literals should be writable/readonly, though that might not be worth it).

@anilanar
Copy link
Contributor

anilanar commented Nov 29, 2022

@qpwo

Let me give an example:

// node_modules/sort-library/index.d.ts
/**
 * Sorts an array in place
 */
export function sort(arr: number[]): void;

Today

// src/index.ts

import { sort } from 'sort-library';

const mutableArray: number[] = [1, 2, 3];
const immutableArray: readonly number[] =  [1, 2, 3];

// no error
sort(mutableArray);

// The type 'readonly number[]' is 'readonly' and cannot be assigned to the mutable type 'number[]'.
sort(immutableArray);

Everything readonly by default

// src/index.ts

import { sort } from 'sort-library';

const mutableArray: mutable number[] = [1, 2, 3];
const immutableArray: number[] =  [1, 2, 3];

// no error
sort(mutableArray);

// no error... WHAT???
sort(immutableArray);

The reason the last sort doesn't emit a type error is because a defaultReadonly: true option in tsconfig.json would apply to everything, including library code in node_modules. So all types in all declaration files in node_modules would become readonly by default. So export function sort(arr: number[]): void; becomes export function sort(arr: readonly number[]): void; which is a lie because that function mutates the array in its javascript implementation file.

@MartinJohns
Copy link
Contributor

@anilanar Please don't confuse read-only with immutable.

@anilanar
Copy link
Contributor

@MartinJohns Perhaps this issue and its dual #32758 require some enlightenment about that. Does true immutability exist in TS? If it doesn't, don't we refer to readonly fields and ReadonlyArrays when we say immutable in this context?

@anilanar
Copy link
Contributor

Even the type error itself mentions mutability: The type 'readonly number[]' is 'readonly' and cannot be assigned to the mutable type 'number[]'.

@MartinJohns
Copy link
Contributor

@anilanar Immutability can't be represented in TypeScripts type system as of today. Read-only only means it's read only via that interface. A good example is that mutable objects can be implicitly assigned to the read-only versions. The object is read only, but it's not immutable, it can still be mutated just fine. (side note: the read-only version can be passed implicitly to the mutable version as well.)

@parischap
Copy link

Hi! Has any progress been made on that topic? I am also very interested in a readonly bu default typescript flag in tsconfig.json that would apply only to the files covered by this tsconfig.json.

@Tungetyt
Copy link

Please let it happen.

@rhaksw
Copy link

rhaksw commented Nov 6, 2024

@MartinJohns @anilanar to move this forward, is correct that @safareli would need to modify the proposal to account for the impact on library code, as @anilanar mentioned here in Jan 2021, and here in Nov 2021?

CC @kachkaev for input on why the current proposal is undesirable.

@anilanar
Copy link
Contributor

anilanar commented Nov 6, 2024

@rhaksw This is up to the TS core team to come up with a proposal as only they have the ultimate long-term vision, considering this feature would require a radical approach by either introducing a new tsconfig field, or a file-scoped pragma or similar.

Right now "readonly" doesn't have too much of an adoption neither in libraries nor in application code because:

  1. Library authors avoid readonly. They assume users don't use readonly even for readonly arrays/records. They think, if they start using readonly, the library will become more difficult to use.
  2. The same logic applies to application coders. They don't use readonly because using it makes libraries harder to use.

So this is a vicious cycle that is difficult to get out of. However there's still hope if we make readonly less verbose, e.g. by making it the default as opt-in.

@rhaksw
Copy link

rhaksw commented Nov 6, 2024

Thank you for that explanation, @anilanar. Speaking as an application developer, I do not use "readonly" (either the flag, Readonly<...>, or const ... as const, not because it makes libraries harder to use, but rather because it requires me to alter all of my variable declarations. If a library required me to mutate its objects in my own code, then I simply would not use it.

This proposal interested me because it would not require me to alter all variable declarations. I understand that if I adopted the proposed "readonlyByDefault", then that may limit the libraries available to me. But then, unless I am misunderstanding something here, I would also consider those to be poorly written libraries that I shouldn't be using anyway.

@rhaksw
Copy link

rhaksw commented Nov 23, 2024

Thinking about this again, maybe an ESLint plugin such as eslint-plugin-functional would be more practical than trying to change TypeScript.

@ardokirsipuu
Copy link

ardokirsipuu commented Dec 19, 2024

Thinking about this again, maybe an ESLint plugin such as eslint-plugin-functional would be more practical than trying to change TypeScript.

Although targeting the same problem, it seems to be a very different solution. One gets rid of the need for having boilerplate while the other enforces me to write this boilerplate. πŸ™ˆ

EDIT: Or perhaps instead of the rules enforcing to make everything read-only you were thinking of the rules not allowing mutation? There are a lot of different rules in that ESLint pugin.

@rhaksw
Copy link

rhaksw commented Dec 19, 2024

Yes, I meant the rule in your second link, functional/immutable-data. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests