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

Prefer ReadOnly<T> over readonly keyword #58

Open
elldritch opened this issue Dec 10, 2017 · 5 comments
Open

Prefer ReadOnly<T> over readonly keyword #58

elldritch opened this issue Dec 10, 2017 · 5 comments
Labels
transferred to eslint port This issue has been transferred to github.com/jonaskello/eslint-plugin-ts-immutable

Comments

@elldritch
Copy link

Using Readonly<T> is much more readable than marking every property as readonly.

@jonaskello
Copy link
Owner

So if I understand you are proposing to use:

interface FooMutable {
  bar: string
}
type Foo = ReadOnly<FooMutable>

Or perhaps:

type Foo = ReadOnly<{
  bar: string
}>

The second option would not give an interface but instead a type I think. Is there a shorter way to write it and still use an interface?

@elldritch
Copy link
Author

I actually prefer types over interfaces in my code, so I'm not very familiar with how this would work on interfaces. I'm not sure if there's a way to create mapped interfaces (a la mapped types).

Maybe it would make sense to use this rule only for types?

@jonaskello
Copy link
Owner

Yes, I think making a new rule that only apply for types would make the most sense. Something like readonly-type. AFAIR ReadOnly<T> creates a mapped type which adds the readonly modifier on all members.

So the rule would check for type alias declarations (TypeAliasDeclaration) that declares an alias for type literals (TypeLiteral).

So it would warn for this case:

type Foo = {
  bar: string
}

But ignore this case:

type Foo = number;

Is there more cases we need to check for?

@gabejohnson
Copy link

type Identity<T> = T;

export interface Bar extends Identity<{foo: string}> {}

should also be check, no?

@geon
Copy link
Contributor

geon commented Mar 1, 2019

The second option would not give an interface but instead a type I think. Is there a shorter way to write it and still use an interface?

interface Foo extends Readonly<{
    myProperty: string;
    myOtherProperty: number;
}> {};

I think this looks pretty readable. At least when there are a lot of properties. Compare to how it currently looks:

interface Foo {
    readonly myProperty: string;
    readonly myOtherProperty: number;
};

@RebeccaStevens RebeccaStevens added the transferred to eslint port This issue has been transferred to github.com/jonaskello/eslint-plugin-ts-immutable label Jul 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
transferred to eslint port This issue has been transferred to github.com/jonaskello/eslint-plugin-ts-immutable
Projects
None yet
Development

No branches or pull requests

5 participants