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

[Optional feature] Mutable references like in Rust with "mut" #56409

Closed
6 tasks done
coolCucumber-cat opened this issue Nov 15, 2023 · 16 comments
Closed
6 tasks done

[Optional feature] Mutable references like in Rust with "mut" #56409

coolCucumber-cat opened this issue Nov 15, 2023 · 16 comments
Labels
Suggestion An idea for TypeScript Too Complex An issue which adding support for may be too complex for the value it adds

Comments

@coolCucumber-cat
Copy link

πŸ” Search Terms

mutable, mut, mutable references

βœ… Viability Checklist

⭐ Suggestion

It's very common to accidentally mutate an object and not be aware of it, or purposefully mutate something but it's hard to keep track of it. Rust solves this problem with the mut keyword, like let mut x = something or fn x(y: &mut Something) {}. This would be an opt-in feature and the implementation may differ from the Rust one, but the general idea is that if you want to mutate a variable, you have to use the mut keyword and if you want to mutate a function parameter, both the function parameter and function argument when you call the function must use the mut keyword. If you want to mutate this in a function, you must also use the mut keyword using this as a parameter (since this is a reserved word, it already let's you do this, for example (this: Something) => void.

πŸ“ƒ Motivating Example

The ReadonlyArray type in TypeScript is quite flawed because it requires a separate type for arrays that are readonly. It relies on the fact that the devs made all readonly arrays be of the ReadonlyArray type and that they also knew if a function mutated the array or not. There is simply no way for TypeScript to know that an array with a fixed length shouldn't have the push method. How could we tell it? With the mut keyword specifying whether a method mutates itself. For example, the type definition of push would change from this push(...items: T[]): number to this push(mut this, ...items: T[]): number. Now TypeScript knows this only works on mutable arrays and not immutable ones. So this would work const mut array = [1, 2, 3]; array.push(4), but this would fail const array = [1, 2, 3]; array.push(4), because the push method requires this to be mutable, but it isn't.

πŸ’» Use Cases

  1. What do you want to use this for? Everyone that opts in. It will also help people that aren't using it, because TypeScript and big libaries will have it enabled, so anyone using those type definitions will see that it is mutable, it just won't do anything (similar to how TypeScript benefits people that aren't using it, it just isn't enforced)
  2. What shortcomings exist with current approaches? You just have to know what mutates what and you have 0 safety.
  3. What workarounds are you using in the meantime? None, because there aren't any.
@RyanCavanaugh
Copy link
Member

This isn't something we'd be keen to adopt. The level of infectiousness here is very high (same as const in C++), where basically everyone in the stack needs to agree to have these annotations, but as demonstrated so far, it's even somewhat rare to bother writing ReadonlyArray<string> in lieu of string[] when a function doesn't mutate the array.

@coolCucumber-cat
Copy link
Author

This isn't something we'd be keen to adopt. The level of infectiousness here is very high (same as const in C++), where basically everyone in the stack needs to agree to have these annotations, but as demonstrated so far, it's even somewhat rare to bother writing ReadonlyArray<string> in lieu of string[] when a function doesn't mutate the array.

I don't think you understand. This only affects one codebase and if you are using a library that doesn't use it, you can still use it, but if a library does you it, you don't have to. TypeScript as a whole is considered "infectious" in that sense, yet we still use it. I don't know what you mean by the ReadonlyArray bit, because everyone just writes as const, because without immutability being enforced everywhere, it's entirely pointless.

@RyanCavanaugh
Copy link
Member

I know about the idea of optional features, yes. That doesn't mean they appear for free, or have no other concerns, or don't represent ongoing cost to maintain and reason about as they interact with other new features.

@RyanCavanaugh RyanCavanaugh closed this as not planned Won't fix, can't repro, duplicate, stale Nov 15, 2023
@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript Too Complex An issue which adding support for may be too complex for the value it adds labels Nov 15, 2023
@coolCucumber-cat
Copy link
Author

What do you even mean? I never said it was free. What did you mean about ReadonlyArray because it makes no sense

@coolCucumber-cat
Copy link
Author

How is this "too complex"? You just can't mutate something if it's not marked as mut. It wouldn't just help in the ReadonlyArray aspect, you can use it for anything like that.

@fatcerberus
Copy link

As proposed, I think this is actually more dangerous than the status quo.

Scenario:

  1. Someone writes a library and chooses not to use mut. It gets typechecked using the current behavior where everything is implicitly mutable.
  2. Your project uses this library, but you have enabled mut enforcement and use it throughout your project.
  3. You call a function from the aforementioned library, which, because it lacks mut annotations, TS assumes all its calls are non-mutating.
  4. Result: Non-mut object gets mutated in the mode that’s explicitly designed to make that impossible

@coolCucumber-cat
Copy link
Author

@fatcerberus No, it would see you didn't use this feature and just mark everything as mut.

@coolCucumber-cat
Copy link
Author

@fatcerberus Since you didn't use the feature, that means it is like how it is now, which means it is mutable. You'll have to specify mut in loads of places possibly, but that's actually good considering it's the worst case scenario.

@fatcerberus
Copy link

I don't think you understand - compiler options are not per-file, they are per-project, and extend even to imported modules. TS would have absolutely no way of knowing that the library author didn't enable the option to enforce this, it would just see that the mut annotations are missing. So enabling the feature would automatically break every library written before it was introduced.

@coolCucumber-cat
Copy link
Author

@fatcerberus And tsconfig.json is also per project and not per file so I'm not sure you understand.

@fatcerberus
Copy link

fatcerberus commented Nov 15, 2023

Yes, and your tsconfig applies even to the libraries you import. Which may mutate stuff without being marked mut. That was my point.

@coolCucumber-cat
Copy link
Author

@fatcerberus If that were true, so many things would be broken. It only applies your options to your project, including when you use other libraries, but not the internals of the library. If it only checked for mut blindly, it would be a problem, but you can just check that before.

For example, if a library doesn't have explicit return types but you require explicit return types, it's not an issue, same with implicit overrides.

@fatcerberus
Copy link

fatcerberus commented Nov 15, 2023

Let me spell this out.

Let's suppose you enabled strictMutability for your project, then...

/* myStuff.ts - in your project */

// written in the future, when strict mutability is available
import { mutateArray } from 'theirStuff'
const x: string[] = [ "foo", "bar" ]
// x.push("qux")  // type error, `this` is not mutable
mutateArray(x)  // not an error!


/* theirStuff.d.ts - buried somewhere in node_modules */

// generated by current TS, before the `mut` keyword existed
declare function mutateArray(x: string[]): void

Since you have strict mutability enabled and all TS sees of mutateArray is its type signature, TS thinks it's non-mutating and allows the call. There's no way around this because TS can't know whether that .d.ts came from a project with strict mutability enabled or not.

@coolCucumber-cat
Copy link
Author

@fatcerberus Yes the declaration is using the current way, but since we don't know, that means it's not enabled.

Or we could have the opposite, a readonly keyword. In a project that uses mut, it would set anything that isn't explicitly readonly as such and then if nothing is specificied, that means it can't be using mut and is therefore mut.

@coolCucumber-cat
Copy link
Author

coolCucumber-cat commented Nov 23, 2023

@RyanCavanaugh how would you solve this problem here?

type X = { a: number }
type UnReadonly<T extends Record<string, any>> = {
	-readonly [K in keyof T]: T[K]
}
declare const j: Readonly<X>
function i(e: UnReadonly<X>) {
	e.a = 1
}
i(j)

Clearly this shouldn't be allowed. It doesn't even work if when you manually specify it, because it doesn't see a difference between must being writable and not being readonly.

@MartinJohns
Copy link
Contributor

Currently types with readonly properties are implicitly compatible with writable properties. The open issue for this is #13347.

Currently there is no way to represent immutability in the type system. Read-only does not mean immutable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Suggestion An idea for TypeScript Too Complex An issue which adding support for may be too complex for the value it adds
Projects
None yet
Development

No branches or pull requests

4 participants