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

Enforce TS's readonly more strictly #3126

Closed
zoddicus opened this issue Nov 1, 2023 · 5 comments
Closed

Enforce TS's readonly more strictly #3126

zoddicus opened this issue Nov 1, 2023 · 5 comments
Labels
enhancement New feature or request

Comments

@zoddicus
Copy link
Contributor

zoddicus commented Nov 1, 2023

The current configuration that we have for linting, etc does not always strictly enforce not assigning to readonly params. I have a WIP branch where I know I am improperly assigning to a readonly.

I do not get errors about this in my IDE, npm run fix or when running the unittests, but npm run check and running the CTS via dawn.node it errors on this.

There is probably somewhere in the configs that this is being suppressed or not turned on.

I need to take some time to figure out how make sure this properly enabled and fix up any violates that have snuck in.

@zoddicus zoddicus added the enhancement New feature or request label Nov 1, 2023
@zoddicus zoddicus self-assigned this Nov 1, 2023
@kainino0x
Copy link
Collaborator

Via this comment
https://gist.github.com/dilame/32709f16e3f8d4d64b596f5b19d812e1?permalink_comment_id=4708061#gistcomment-4708061
I just found this reference for a "strictest" mode for tsconfig:
https://github.com/tsconfig/bases/blob/main/bases/strictest.json
We're already using most of these (except a few I intentionally disabled), I don't suspect any of the remaining ones can make readonly stricter.

It's more likely we can find new linter plugins to enable. There are some suggestions on the TypeScript bug thread, e.g.:
microsoft/TypeScript#18770 (comment)

@zoddicus
Copy link
Contributor Author

zoddicus commented May 6, 2024

I don't think I have run into this specific issue in a while, so I am going to close this.

@zoddicus zoddicus closed this as completed May 6, 2024
@kainino0x
Copy link
Collaborator

kainino0x commented May 6, 2024

Oh, I forgot we had this bug filed. I'd actually like to keep this open, as this hole could still let some bugs bypass the safety checks in #3097. (And it sounds like we might be able to add lint checks for it.)

@kainino0x
Copy link
Collaborator

Linking from https://crbug.com/331393770, will assign myself if I manage to get to this.

@kainino0x
Copy link
Collaborator

Looked into this a bit. While some of the rules in https://github.com/eslint-functional/eslint-plugin-functional might be helpful, there aren't any that would actually enforce readonly the way we need.

The most useful rules in there are around preferring readonly types, but we only would want to apply that to globals and I don't think there's a way to do this.

I think this is infeasible for now unfortunately.

@kainino0x kainino0x closed this as not planned Won't fix, can't repro, duplicate, stale Jul 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants