I'm not sure I understand how this is a valid trade-off as this is completely at-odds with what the rule enforces.

With this option it means that very simple cases can leak through and cause side-effects in your code.

Eg

function foo(arg: Readonly<{prop: {other: string}}>): void {
  arg.prop.other = 'oops';
}

There are utility types that exist and that allow you to make types deeply readonly such as the one published to npm in this utility package - so I'm not sure why it's better to add a large hole to the rule instead of using one of those types.

Hi,
I agree that this makes it fairly easy to circumvent the rule - for example with your code. However, it is still a step forward - I would ideally like to not need such an option, however, this gives a middle ground between nothing and strict deep readonlyness - similarly to how you usually start with strict: false when converting a JS codebase to TS.

Thanks for the link to the ReadonlyDeep<> utility type, I haven't seen that before. However, it seems to me that this is not as versatile as the builtin Readonly<> - example from my codebase:

$( '.some-class' ).each( ( _, container: HTMLElement ) => {
    // Some code that only reads from container
} );

When I make container be of type Readonly<HTMLElement>, everything works just fine. However, when I use ReadonlyDeep<HTMLElement> I get error TS2345: Argument of type '(this: HTMLElement, _: number, container: ReadonlyObjectDeep<HTMLElement>) => void' is not assignable to parameter of type '(this: HTMLElement, index: number, element: HTMLElement) => false | void'. :/

It seems to me that in this case, I have two options - either settle for Readonly<>, or have no readonlyness check at all. I'd rather have the shallow check than none - and that is what this issue proposes.

Another option is to consider using the ignoreInferredTypes option and not providing an explicit type for the variable.

similarly to how you usually start with strict: false when converting a JS codebase to TS

The middle ground here is that you can turn the rule on or off.
You can use eslint overrides to turn the rule on one folder at a time.
You can selectively disable violations using disable comments.

That's how you can start with this rule.

We do not accept options that help people adopt rules because options live forever. They're not temporary like a codebase transition. Which means that people can forget to turn these "transitional" options off.

False positives do suck - but are "okay" because they can be seen, understood, and ignored (disabled via comment).
False negatives, on the other hand, are terrible. They're invisible and insidious. They hide and cannot be found until they cause an issue.

False positives annoy users by being annoying. False negatives irrevocably damage a user's trust in linting.
So when building options we need to be sure they are as targeted as possible.

Your suggestion of a blanket option that just allows single-level readonly is too far-reaching and will very, very easily lead to false negatives (like I have shown).

The above two reasons are enough for me to reject your initial proposal.

Given your example, however, now I can actually understand your problem.
Your issue isn't that "it's too hard to make a type deeply readonly".
Instead your issue is "certain types are hard/impossible to make deeply readonly".

For reference - this is why our templates all ask for example code when filing issues. Code is worth 1000 words and it can get everyone on the same page much faster than just a description - which can avoids a lot of back-and-forth.

Making a type is deeply readonly - as I've indicated. The issue is that there are certain types that just don't play nicely with deep readonly-ness.
The HTML types are a known problem due to the immense complexity and deeply recursive nature of the types.

You don't actually want an option that allows just single-level readonly. You want an option that allows you to ignore certain impossible-to-deep-readonly types.
It's not easy to build such an option, but there are things we can do for the general case of "specify a string and we'll ignore the type". However we can easily build the rule to ignore certain "known-bad" types, like any subtypes of HTMLElement.

I'm more than happy to accept a change which adds an option to ignore types that are a subtype of HTMLElement. That's an acceptable level of targeting which helps deal with a specific pain-point.

The middle ground here is that you can turn the rule on or off.

I'm not sure you and I understand the concept of middle ground the same 🤷

False positives do suck - but are "okay" because they can be seen, understood, and ignored (disabled via comment).
False negatives, on the other hand, are terrible. They're invisible and insidious. They hide and cannot be found until they cause an issue.

I think that is a very good point, at the same time, a rule with very many FPs is a rule that isn't going to be enabled at all. I am currently trying to "fix up" a project to use readonly parameters, however, in something like a third of the cases I have to disable the rule. I would go by "Don't let perfect be the enemy of good" here. I think we have different mental frameworks here - you are thinking weaker rule vs. stronger rule, I am thinking weaker rule vs. no rule.

Your suggestion of a blanket option that just allows single-level readonly is too far-reaching and will very, very easily lead to false negatives (like I have shown).

Is there a way to do this locally? I don't know of any :/ That would be much better though...

You are right that certain types don't play nicely with deep readonlyness. It's not only HTMLElement though - in my fairly simple project, it's HTMLElement, JQuery, React props, React events, React elements. I think given all the libraries with TS support there are, the whitelist would have to be giant... :/

the whitelist would have to be giant.

You need to make a distinction between "types that need deep Readonly instead of single-level Readonly" and "types that are heavy, deep and cyclic and cannot work with deep Readonly"

The former there are many, and the latter there are few.

The former is solved easily by a type like the one that I linked, and the latter cannot be solved by anything.

The latter is all that we need to solve for.

I'm not sure you and I understand the concept of middle ground the same 🤷

Your concept of middle ground is "turn it on weakly across the entire project at once and maybe then gradually improve the types".

My concept of middle ground is "turn it on strictly on a subset of folders and gradually expand that to cover the entire codebase".

Taking your example of migrating a codebase from JS to TS. Would you do a single commit which turns every single file to a .ts file with strict: false, or would you work over many commits to convert specific subtrees a chunk at a time?
The latter approach can be done incrementally and can be distributed across many people. The former has to be done all at once by a single person.

We use scoped/targeted linting a lot at Facebook. We enforce certain folders strictly adhere to linting practices and we treat other folders as weakly linted.
We also do the same thing in this project (check out our ESLint config).

You need to make a distinction between "types that need deep Readonly instead of single-level Readonly" and "types that are heavy, deep and cyclic and cannot work with deep Readonly"

The former there are many, and the latter there are few.

Oh, in the part at the end of my last comment (about the ~ third of types that tuned out to be problematic) I was only talking about those that cannot be solved by ReadonlyDeep<>...

Would you do a single commit which turns every single file to a .ts file with strict: false

Honestly that is exactly what I did :D But I get your point, I am not using TS on big repositories with multiple people actively involved.

But it seems to me that we have reached as far as we could - I'd rather have a weaker rule that doesn't have to be manually turned off in a third of the cases (YMMV of course), you prefer a stricter rule that protects the user better in the cases where it is applicable - I think that is a difference of opinion and I respect that you as a maintainer have the final say in such a case.

Is there any other way to make the rule easier to use in general? You mentioned adding an exception specifically for HTMLElement, however, from my use of the rule it seems the problematic types are much more common. Any way to deal with all of them?

Is there any other way to make the rule easier to use in general? You mentioned adding an exception specifically for HTMLElement...

We can create an allowlist of types that are problematic and expand on them as required.
This is actually pretty trivial to implement.

1. get the Symbol for the type
2. get the declarations for the type
3. ensure that the declaration is within a TS "default library"
   • This check is to ensure that we don't false-negative if someone declares a HTMLElement type locally within their file

For example here is some code we could generalise from an existing rule:

however, from my use of the rule it seems the problematic types are much more common. Any way to deal with all of them?

If you've got some examples of the sorts of types that are problematic, we can discuss a solution for them.
If we're just talking about TS lib types like HTMLElement - then they can just be allowlisted using the above solution.

If there "custom" types or such - then I'd like to gain an understanding of what these types are so we can figure out the best approach for them.

Thanks, I opened that as a new issue to keep the record untangled.