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

Suggestion for readonly interface #21152

Open
ababik opened this issue Jan 11, 2018 · 13 comments
Open

Suggestion for readonly interface #21152

ababik opened this issue Jan 11, 2018 · 13 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

@ababik
Copy link

ababik commented Jan 11, 2018

Avoid duplicating "readonly" for each property by applying it to the interface definition itself.
This is to reduce boilerplate code defining immutable objects.

Having interface where ALL properties are read only:

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

equals to:

readonly interface State {
    prop1: string;
    prop2: string;
    ...
    prop22: string;
}
@mhegazy mhegazy added Suggestion An idea for TypeScript Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature labels Jan 11, 2018
@acutmore
Copy link
Contributor

@acutmore
Copy link
Contributor

Have realised that my suggestion above has some limitations. Like using this for self-referential types

interface Foo extends Readonly<{
    prop1: string;
    prop2: number;
    prop3: this['prop1']; // 'this' type is available only in a non-static member of a class or interface.
}> { }

@aleclarson
Copy link

const interface might be nice for deep readonly (kinda like { a: { b: 1 } } as const)

@RomainMuller
Copy link

I think there might be value in extending this to cover a C++ like const behavior...

const interface Foo {
  property: string; // Implicitly readonly
  method(): void; // Cannot mutate `this`
}

interface Bar {
  const method(): void; // Guaranteed non-mutating
  mutatingMethod(): void; // Could mutate `this`
}

function baz(obj: const Bar) { // Require "obj" to not be mutated by implementation
  obj.mutatingMethod(); // 💥 illegal call of mutating method on const reference
}

I don't have particularly strong feelings on the const versus readonly term here... const is "nice" to me as it relates nicely to what this means in C++, which (some) people are used to.

@shicks
Copy link
Contributor

shicks commented Dec 14, 2019

There's a lot of discussion about various ways to apply readonly to individual properties, but nothing about readonly objects themselves. I've found that TypeScript's ReadonlyArray type is very trustworthy in preventing accidentally passing an array that shouldn't be mutated somewhere that could possibly mutate it:

const FOO = [1, 2] as const; // "TS has my back - this should never change"
function use(arr: Array<number>) { arr[0]++; }
use(FOO); // error!

But readonly properties on objects just don't have sufficient checking, nor are there any guards against passing objects to contexts that might mutate the object structure itself:

const FOO = {x: 1} as const; // "But is it really const?"
function use(obj: {x: number, y?: number}) { obj.x++; obj.y = 2; }
use(FOO); // "This is fine"

Playground Example

Ideally a readonly {x: number} would not be allowed to be silently upcast to {x: number}, possibly not even to {readonly x: number} since the readonly modifier outside would provide additional guarantees about the readonly-ness of the object structure itself. I don't have the slightest idea how feasible this actually is.

@acutmore
Copy link
Contributor

acutmore commented Dec 16, 2019

@shicks

Ideally a readonly {x: number} would not be allowed to be silently upcast to {x: number},

This is already how readonly works, it isn’t silently upcast.

The reason there is no error is your example is because {x:1} as const produces the type {x:1} rather than {readonly x:1}.
The const modifier is only narrowing number to 1.

EDIT: I am wrong. It is being upcast. Viewing the playground on my phone it wasn’t showing the readonly modifier

@shicks
Copy link
Contributor

shicks commented Dec 16, 2019

This seems like sufficient scope for its own feature request, no?

@lautarodragan
Copy link

lautarodragan commented May 26, 2021

Love this proposal. I suspect it wouldn't be that big of a challenge to implement and the value is immense.

At a project I'm currently working on, as a team we settled on @acutmore's suggestion, but using DeepReadonly:

declare type DeepReadonly<T> = { readonly [K in keyof T]: DeepReadonly<T[K]> };

interface Interface extends DeepReadonly<{
  data: {
    prop1?: string; // this would be mutable if we extended for Readonly instead of DeepReadonly
  };
}> {}

This is still quite a bit of boilerplate, and the empty curly braces at the end (}> { }) aren't very elegant or intuitive.

This would be so more readable:

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

One question does arise: should the readonly modifier for the interface be a deep-readonly, or a shallow one?

Intuitively I think a deep/recursive readonly makes the most sense.

@shicks
Copy link
Contributor

shicks commented May 28, 2021

I guess my point is that we don't actually gain that much by simply distributing readonly to all the fields when the implicit downcasting (I believe I misspoke earlier when I called it upcasting) is on by default. Introducing new syntax provides an opportunity to also introduce stricter defaults that would otherwise be infeasible: if the readonly interface refused to implicitly downcast, I argue it would be much safer and more useful (and would be more likely to pull its own weight) than if it were simply syntactic sugar around something that's already possible.

EDIT: fixed typo

@lautarodragan
Copy link

Ah, I see what you mean @shicks! I hadn't seen your playground example before. That is really bad :/

I agree with you. I think they should be separate things though. I'd intuitively expect readonly to be syntax sugar (pretty much), but it's also a good opportunity to fix a mistake from the past.

@lautarodragan
Copy link

@shicks is there a GH issue to specifically tackle the downcasting?

@shicks
Copy link
Contributor

shicks commented Jul 15, 2021

I'm not aware of one.

@acutmore
Copy link
Contributor

@shicks is there a GH issue to specifically tackle the downcasting?

I'm not aware of one.

#13347

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

7 participants