-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Comments
Why not a lint rule where And if they want stuff to be mutable, add eslint-disable-next-line. Not much to say about methods, though. |
Three reasons:
|
I've started using prefer-readonly-type with a small project (9K SLOC) and ended up having to add a 346 One of the reasons for the high |
I use 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 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 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 Of course, with the introduction of this 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
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 };
However, this requires that magical pre-processor directive. I'm not sure if that's a thing the TS team would enjoy having. |
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. |
Oh, wait. https://github.com/sandersn/downlevel-dts Even if new emit is incompatible, downlevel-dts can be used. |
Making If |
Immutability is a complete lack of mutation, to have it as the default would prevent usage of
|
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 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 |
@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. |
@00ff0000red we were talking about |
how about adding a /**
* - 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(); |
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 The way I think about it, when I'm declaring the type, I shouldn't care about immutability. On the user side, with In fact it would be pretty annoying if lots of shared type authors started wrapping types as readonly but we didn't have a With a 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. |
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 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. |
If export interface Foo {
bar: string
mutable baz: number
} the generated 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. |
I think all of these immutable-by-default or |
Some people think that this proposal is bad for immutability in TS |
@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. |
Search Terms
readonly, default, immutable
Suggestion
Adding flags under the
strict
umbrella to default to immutability in different cases, and a newmutable
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:
readonly
keyword or useT[]
rather thanReadonlyArray<T>
accidentally. Defaulting to immutability would help prevent these accidents, and amutable
keyword would be easy to find bytslint
or even a simplegrep
, to automatically request review in PRs or leave an automated commentthis PR introduces mutation
.Examples
Examples should probably live in the children GitHub issues, but I'm copying here my comment for a quick example:
Checklist
My suggestion meets these guidelines:
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:
d.ts
withreadonly
, nevermutable
(possibly behind another flag?)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 entireinterface
has been suggested in A cheaper, easier to implement middle ground could be #21152.This would probably be a much cheaper and less disruptive feature to implement, and could be a great starting point.
The text was updated successfully, but these errors were encountered: