microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.5k stars 12.43k forks source link

Readonly everything by default #42357

Open safareli opened 3 years ago

safareli commented 3 years ago

Suggestion

🔍 Search Terms

My suggestion meets these guidelines:

⭐ 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 commented 3 years ago

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 commented 3 years ago

👍 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 commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

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.

artursvonda commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

Is this a dup of https://github.com/microsoft/TypeScript/issues/32758?

ghost commented 3 years ago

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

isaac-weisberg commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

giphy

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

anilanar commented 2 years ago

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 commented 2 years ago

Re:

This proposal, in its current form, doesn't address what happens to checking library code. … https://github.com/microsoft/TypeScript/issues/42357#issuecomment-963151858

Also considering this one: https://github.com/microsoft/TypeScript/issues/42357#issuecomment-889403488

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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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

anilanar commented 1 year ago

@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 commented 1 year ago

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

MartinJohns commented 1 year ago

@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.)

soplwang commented 1 year ago

"Readonly everything by default" is just like Rust does and compliant with "Principle of least privilege" that's awesome.

Every language that not readonly by default should consider unsafe and should obsoleted.

--

The reason the last sort doesn't emit a type error is because a defaultReadonly: true option in tsconfig.json would apply to everything...

@anilanar may be more precise defaultReadonly: "./src" option address your concern?

soplwang commented 1 year ago

"Readonly everything by default" is just like Rust does and compliant with "Principle of least privilege" that's awesome.

Every language that not readonly by default should consider unsafe and should obsoleted.

The reason the last sort doesn't emit a type error is because a defaultReadonly: true option in tsconfig.json would apply to everything...

Seems "use safety" directive-like solution is more appropriate for future JS standard and TS.

Reasons:

  1. Use flags like defaultReadonly: true in tsconfig.json made TS code be incompatible with JS;
  2. Flags is not language feature and should not affect JS;
  3. We all experienced with "use strict" directive to make incompatible semantics for new JS and coexist with old JS.
parischap commented 1 year ago

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 commented 1 year ago

Please let it happen.