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

Repo: Readonly vs Immutable Update #5699

Closed
RebeccaStevens opened this issue Sep 30, 2022 · 8 comments
Closed

Repo: Readonly vs Immutable Update #5699

RebeccaStevens opened this issue Sep 30, 2022 · 8 comments
Labels
awaiting response Issues waiting for a reply from the OP or another party documentation Documentation ("docs") that needs adding/updating

Comments

@RebeccaStevens
Copy link
Contributor

RebeccaStevens commented Sep 30, 2022

Suggestion

Hey @JoshuaKGoldberg @bradzacher @marekdedic @qpwo @rodrigofariow

I thought I'd update you with regards to previous discussions around the prefer-readonly-parameter-types rule and the updates I've made to eslint-plugin-functional.

A while ago I created issue #4536 talking about Readonly vs Immutable definitions and how these definitions should be applied. I've recently released a new library called is-immutable-type (based of isTypeReadonly utility function) which uses the definitions discussed in that issue and also addresses some of the issue that have been opened here (such as #4185).

I've created a new rule prefer-immutable-parameter-types (edit: now prefer-immutable-types) to replace the original prefer-readonly-parameter-types rule as part of eslint-plugin-functional@5.0.0 (which is currently in beta). And in case you're curious, there's also a second rule that also uses that new library type-declaration-immutability. eslint-plugin-functional v5 should be released in the next few days 😄 (edit: hopefully soon 😞).

I've also just proposed to type-fest about renaming the ReadonlyDeep type to Immutable (issue) as it follows the "immutable" definition. (Waiting on this PR to properly support Immutable Arrays.)

Let me know if you have any feedback 😄.

@RebeccaStevens RebeccaStevens added repo maintenance things to do with maintenance of the repo, and not with code/docs triage Waiting for maintainers to take a look labels Sep 30, 2022
@qpwo
Copy link

qpwo commented Sep 30, 2022

Awesome I'll try it soon as I can

@marekdedic
Copy link
Contributor

Hi, thanks for the proposal. Are you aware of sindresorhus/type-fest#467 ? That will probably impact your work as well and I am not sure it's actually solvable without changes to TS itself...

@marekdedic
Copy link
Contributor

Also, incorporating #4436 would be great, if you didn't do so already - in typescript-eslint it is waiting for the next major as some breaking changes are needed :(

@JoshuaKGoldberg
Copy link
Member

Fantastic, thanks for the update @RebeccaStevens! Is there anything specifically you think should changed in typescript-eslint?

I wonder if we should have an FAQ entry describing the latest-and-greatest in enforcing readonly and/or immutable types. Like our own little https://github.com/dustinspecker/awesome-eslint...

@JoshuaKGoldberg JoshuaKGoldberg added documentation Documentation ("docs") that needs adding/updating awaiting response Issues waiting for a reply from the OP or another party and removed triage Waiting for maintainers to take a look repo maintenance things to do with maintenance of the repo, and not with code/docs labels Oct 17, 2022
@RebeccaStevens
Copy link
Contributor Author

@JoshuaKGoldberg I don't think there's really anything that needs to be changed here. Maybe is-immutable-type could be used by some rules in the future?

Note: The release of eslint-plugin-functional v5 has been delayed due to what seems to be a bug in tsutils - hopefully it will be patched soon. And @jonaskello wants to do a bit more testing first too.

@RebeccaStevens
Copy link
Contributor Author

@marekdedic

#4436 is indeed included. Instead of an allow list though, it's a set of overrides. See overrides and immutability settings for details on how to use them.

I don't think any type-fest issues will affect things too much as ReadonlyDeep is by no means required to achieve immutability, it's just a convenient helper.

@JoshuaKGoldberg
Copy link
Member

tsutils ... hopefully it will be patched soon

#5552 😬

In this case, I'm going to close this issue as non-actionable to keep it out of our queue. But feel free to keep commenting here a bit, it's useful info. Cheers!

@marekdedic
Copy link
Contributor

@RebeccaStevens

#4436 is indeed included. Instead of an allow list though, it's a set of overrides. See overrides and immutability settings for details on how to use them.

Hi, thanks for the pointer - it's a bit of a missed opportunity that the overrides style will be different in the 2 plugins, but that's probably too late to change...

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
awaiting response Issues waiting for a reply from the OP or another party documentation Documentation ("docs") that needs adding/updating
Projects
None yet
Development

No branches or pull requests

4 participants