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

Immutable-By-Default Flags #32758

Open
4 of 5 tasks
lautarodragan opened this issue Aug 8, 2019 · 20 comments
Open
4 of 5 tasks

Immutable-By-Default Flags #32758

lautarodragan opened this issue Aug 8, 2019 · 20 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

@lautarodragan
Copy link

lautarodragan commented Aug 8, 2019

Search Terms

readonly, default, immutable

Suggestion

Adding flags under the strict umbrella to default to immutability in different cases, and a new mutable keyword.

Use Cases

I'm creating this issue as a parent issue to track a couple of issues that already exist for specific cases:

Off of the top of my head, these flags would have some direct advantages:

  • They clearly state the intention of the whole project in regards to mutability
  • It's very common for new members of teams to forget to add the readonly keyword or use T[] rather than ReadonlyArray<T> accidentally. Defaulting to immutability would help prevent these accidents, and a mutable keyword would be easy to find by tslint or even a simple grep, to automatically request review in PRs or leave an automated comment this PR introduces mutation.

Examples

Examples should probably live in the children GitHub issues, but I'm copying here my comment for a quick example:

interface T {
  n: number // immutable
  mutable s: string // mutable
}

const o: T = {
  n: 42,
  s: 'hello world',
}

o.n = 43 // error
o.s = '👋🌎' // ok

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, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.

Backwards Compatibility

@AnyhowStep made an interesting comment on this issue.

Basically this feature wouldn't be any problem for applications, but it could be problematic for libraries, as the emitted .d.ts may be imported from a library/application that isn't using this flag or is using an older TS version.

Possible solutions:

Records and Tuples

The Record and Tuple proposal has reached stage 2, so it may be arriving to TS soon-ish. It seems closely related to this issue, but I don't think it fully addresses it.

readonly interface

The possibility of using the readonly keyword for an entire interface has been suggested in A cheaper, easier to implement middle ground could be #21152.

readonly interface State {
    prop1: string;
    prop2: string;
    ...
    prop22: string;
}

This would probably be a much cheaper and less disruptive feature to implement, and could be a great starting point.

@AnyhowStep
Copy link
Contributor

Why not a lint rule where readonly is always required?

And if they want stuff to be mutable, add eslint-disable-next-line.

Not much to say about methods, though.

@lautarodragan
Copy link
Author

Why not a lint rule where readonly is always required?

And if they want stuff to be mutable, add eslint-disable-next-line.

Not much to say about methods, though.

Three reasons:

  • The way one writes affects the way one thinks. It's harder for new team members to get into the immutability mindset if everything's mutable by default, specially if they are relatively new to TypeScript.
  • One less thing to pay attention to, less cognitive overhead.
  • Just did a quick search, there are 179 occurrences of the word readonly in one of the projects I work on daily, and it's a relatively small codebase. And it's just one project, we have a few.

@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 Aug 9, 2019
@kevinbarabash
Copy link

kevinbarabash commented Dec 27, 2019

I've started using prefer-readonly-type with a small project (9K SLOC) and ended up having to add a 346 readonlys throughout the code and 18 // eslint-disable-next-line functional/prefer-readonly-type comments. I'd much rather only have to add 18 mutables instead. The code would be much cleaner and easier to maintain.

One of the reasons for the high readonly count is that the prefer-readonly-type doesn't support wrapping objects with Readonly<>.

@AnyhowStep
Copy link
Contributor

I use readonly as much as possible, myself. I have 1019 usages of readonly in a hobby project.
But I still think this is better as a lint rule (at the moment).

It seems like people don't agree with me, so I figured I'd elaborate.


If library A uses this flag, the emit would still have to be compatible with downstream libraries, whether they use this flag or not.

So, now, library A's .d.ts emit has to look like this,

export type Immutable = { readonly x : number };
export type Mutable = { mutable x : number };

If it was this instead,

export type Immutable = { readonly x : number };
export type Mutable = { x : number };

Then downstream libraries that use the flag will think Mutable is immutable.


If it was this instead,

export type Immutable = { x : number };
export type Mutable = { mutable x : number };

Then downstream libraries that do not use the flag will think Immutable is mutable.


Of course, with the introduction of this mutable keyword, you now have a breaking change. One that might not be justifiable because of how much impact it has.

Projects on TS-without-mutable-keyword suddenly can't use projects compiled by TS-with-mutable-keyword. And not everyone upgrades their project's version of TS as quickly as the releases come.


I can't think of a way to introduce this flag without breaking .d.ts files, somehow.
The ideal implementation would,

  • Be compatible with older versions of TS (No new keywords/property modifiers)
  • Be compatible with projects that turn off this flag
  • Be compatible with projects that turn on this flag

You could maybe have some kind of TS-specific pre-processor directive in the emit that says, "Treat the following type as mutable". Then, your emit would be,

export type Immutable = { readonly x : number };
/* magic-pre-processor-directive-that-says-following-type-is-mutable */
export type Mutable = { x : number };
  • Older versions of TS correctly see Mutable as being mutable.

    No behaviour has changed. It looks like regular emit.

  • Newer versions of TS with the flag correctly see Mutable as being mutable.

    With the flag switched on, it should think that Mutable is immutable, since readonly is the default property modifier. But the magical comment should force TS to treat Mutable as having mutable properties by default.

  • Newer versions of TS without the flag correctly see Mutable as being mutable.

    Same as older versions of TS.

However, this requires that magical pre-processor directive. I'm not sure if that's a thing the TS team would enjoy having.

@lautarodragan
Copy link
Author

Thanks @AnyhowStep for that thorough analysis! Initially I didn't realize this would be a breaking change. I've added your comment to the issue's description, along with a possible solution.

@AnyhowStep
Copy link
Contributor

AnyhowStep commented Jan 24, 2020

Oh, wait. downlevel-dts is a thing.

https://github.com/sandersn/downlevel-dts

Even if new emit is incompatible, downlevel-dts can be used.

@SRachamim
Copy link

SRachamim commented Sep 4, 2020

Making readonly the default will make TypeScript a lot more powerful. In our codebases we use prefer-readonly-type to automatically add readonly to everything, and we don't have any eslint-ignore. Still, it's annoying to see and read all those Readonly.

If TypeScript wants to be here to stay it must go through this soon. I'm shocked to see 2020 code that uses mutations. I thought that already behind us.

@ghost
Copy link

ghost commented Jan 21, 2021

Immutability is a complete lack of mutation, to have it as the default would prevent usage of let and var.
I rarely use let, and forgot that var exists, but what do you think others would think of the situation, and what do you think of it?

var and let would have to be treated as reserved keywords.

@pleunv
Copy link

pleunv commented Jan 21, 2021

If TypeScript wants to be here to stay it must go through this soon. I'm shocked to see 2020 code that uses mutations. I thought that already behind us.

There's nothing wrong with mutations, and in certain situations (hot paths, etc) they even make more sense than trying desperately to keep things fast and immutable, but having the ability to make them opt-in rather than opt-out would definitely be handy.

Immutability is a complete lack of mutation, to have it as the default would prevent usage of let and var.
I rarely use let, and forgot that var exists, but what do you think others would think of the situation, and what do you think of it?

Immutability isn't necessarily linked to variable assignments, is it? I think most use-cases people run into are related to object (im)mutability, with the introduction and confusion around const definitely not helping there.

@ghost
Copy link

ghost commented Jan 21, 2021

@pleunv that would be "readonly," or frozen, something that separates value from variable. Immutability is both, non-reassigning variables, and using data structures that are not mutated or cannot be mutated.

@lautarodragan
Copy link
Author

@00ff0000red we were talking about readonly here really. I don't think anyone had const in mind, since we're not talking about completely prohibiting anything — just making readonly the default wherever that keyword is currently needed.

@btoo
Copy link

btoo commented Mar 25, 2021

how about adding a mutable modifier as the immutable-by-default flag's readonly counterpart:

/**
 * - without `--strict`/`--noUncheckedIndexedAccess`:
 *      `number[]` (i.e. `readonly number[]` in normal mode)
 * - with `--strict`/`--noUncheckedIndexedAccess`:
 *      `[1, 2]` (i.e. `readonly [1, 2]` in normal mode)
 */
const immutableByDefaultArray = [1, 2];
// error `Type '<typeof immutableByDefaultArray>' is immutable`
immutableByDefaultArray.sort();
// error `Type '<typeof immutableByDefaultArray>' is immutable`
immutableByDefaultArray[3] = 4;

/**
 * - without `--strict`/`--noUncheckedIndexedAccess`:
 *      `mutable number[]` (i.e. `number[]` in normal mode)
 * - with `--strict`/`--noUncheckedIndexedAccess`:
 *      `mutable [1, 2]` (i.e. `[1, 2]` in normal mode)
 */
const immutableByDefaultMutableArray = mutable [1, 2];
// ok
immutableByDefaultMutableArray.sort();
// ok
// or error `Tuple type '[1, 2]' of length '2' has no element at index '3'.` with `--strict`/`--noUncheckedIndexedAccess`
immutableByDefaultMutableArray[3] = 4;
// ok
// or error `Object is possibly 'undefined'.` with `--strict`/`--noUncheckedIndexedAccess`
immutableByDefaultMutableArray[3].toString();

@lautarodragan
Copy link
Author

@btoo I did mention the possibility of using a new keyword mutable for interface fields, but I had not thought of arrays at the time.

Love your idea!

I think if we were to tackle this, your proposal may fall under #42357 too.

@lautarodragan
Copy link
Author

A cheaper, easier to implement middle ground could be #21152

@benwiley4000
Copy link

@lautarodragan A cheaper, easier to implement middle ground could be #21152

I'm personally less into the idea of a read-only interface (as a solution to this problem). We already have Readonly<{}> for type aliases but I tend not to use it for exported/shared types because it's annoying on the user side to be forced to build the object in one way or another.

The way I think about it, when I'm declaring the type, I shouldn't care about immutability. On the user side, with readonly (or mutable) I'm able to declare my intentions for how this specific implementing data of the shared type will be used.

In fact it would be pretty annoying if lots of shared type authors started wrapping types as readonly but we didn't have a mutable keyword to go with it. That would mean it's pretty hard to build this type across multiple lines.

With a Readonly type alias we can use:

type Mutable<T> = T extends Readonly<infer U> ? U : never;

With readonly interfaces we could hope that could work but I'd imagine the semantics would be different.

@rope-hmg
Copy link

If this were to be the default I would end up having to ask for the opposite of my current feature request #35313

I like the idea of having immutable by default since that's also how Rust and many other modern and functional languages do things. One of the current issues with marking things readonly is trying to use Readonly<T> on classes. It doesn't actually do anything for you because you can still call the methods that mutate the object. We have ReadonlyArray, ReadonlyMap and ReadonlySet to address this for those types, but nothing general.

I'd appreciate some feedback on my feature request which tries to address the above. Or, if this is something that's adopted then I'd like some feedback on the opposite version of my idea.

@mmkal
Copy link

mmkal commented Apr 10, 2022

If d.ts compatibility is a concern, could the mutable keyword be disallowed in declarations? So for a source file foo.ts:

export interface Foo {
  bar: string
  mutable baz: number
}

the generated foo.d.ts could be:

export interface Foo {
  readonly bar: string
  baz: number
}

the only downside would be that manually written declarations wouldn’t be able to use this feature (yet), but that doesn’t seem like a big deal to me.

@lautarodragan
Copy link
Author

lautarodragan commented Feb 20, 2023

I think all of these immutable-by-default or readonly interface (either shallow or deep, ideally deep) are made redundant / unnecessary if Records and Tuples reaches Stage 3.

@adrian-gierakowski
Copy link

Records and Tuples reaches Stage 3.

Some people think that this proposal is bad for immutability in TS

@lautarodragan
Copy link
Author

@adrian-gierakowski maybe so. But many of us are already following that pattern, and would love first-class support of it.

Regardless, if that proposal winds up being rejected by TC39, we can always continue this discussion and propose ts-specific improvements to immutability.

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