Consider keeping parserServices.hasFullTypeInformation for another major version?

[JoshuaKGoldberg]

Overview

Coming over from microsoft/TypeScript#54693 (comment): in v6 we removed parserServices.hasFullTypeInformation. Consumers can now check if parserServices.program exists. But, there are a few existing external consumers that rely on it:

• Sourcegraph search for parserServices.hasFullTypeInformation: 23 results
• Sourcegraph search for parserServices?.hasFullTypeInformation: 4 results

If we do nothing and keep it removed, the referenced plugins/etc. may not work out-of-the-box with v6. I'm happy to send PRs over to help them upgrade, but that might be kind of a pain. And we don't know what internal code will break.

Proposal: how about we add it back in as a /** @deprecated */ property, then remove it in v7? It's not much to add/change on our end. Thoughts @typescript-eslint/triage-team?

[JamesHenry]

Sounds like a good plan to me!

[bradzacher]

Our parser services weren't ever a documented feature - the only way we supported and documented using them was via our utility functions. It had types, but that's because TS doesn't have any way to define an opaque type.

Some of those are incorrect usages of the API and should instead have used our recommended and documented getParserServices function. Eg there's a lot of cases like if (! hasFullTypeInfo) { silently don't even create a rule } which is an anti-pattern.

I personally don't think we should keep it around. It doesn't represent any information that exists any more (it will always be true, or it won't exist)

[Josh-Cena]

I don't think we should push it back further, either. The migration effort sounds low and breaking changes are meant to be breaking changes (adding @deprecated is not). If you want to, you can start adding @deprecated now before v6 ships.

[JamesHenry]

Consider me convinced!

[bradzacher]

Just to clarify why I think we should rip the band-aid.

I can definitely understand the argument that we should keep it and give people time to migrate.
However, I don't think that you'd see many consumers actually migrate - I'd hazard a guess that a lot of consumers would just ignore it and not make any changes at all, even if it was marked as @deprecated.

Instead the status quo would remain until such time that they were forced to migrate.

If, for example, there was a reason for them to migrate - for example we were releasing some new API/feature that required them to not use hasFullTypeInfo, then people would naturally migrate to use the new feature - but purely based on the reasoning of "hey don't use this cos it's now the same as program != null" - there's no drive/motivation to make the transition.

One could argue that leaving it as @deprecated at least gives time for people to build backwards and forwards compatible checks before we remove it - but I don't think that people will really do that. Which is why I think we should just rip the band-aid and save us a TODO for the next major.

[JoshuaKGoldberg]

👍 I'll go ahead and file issues on downstream repos found by the OP's Sourcegraph search for parserServices.hasFullTypeInformation. It's a pretty straightforward fix so I feel comfortable contributing to them if needed.

Edit: oh, and I'll write a blog post that mentions this change, as it wasn't documented in the beta. #7153

[JoshuaKGoldberg]

Actually, wait - the backwards compatibility story here isn't great. Right now parserServices.program exists even if parserOptions.project wasn't configured in the ESLint config. So for the community configs, switching from checking parserServices.hasFullTypeInformation to !!parserServices.program isn't backwards compatible for v5.

This is actually quite relevant for some community configs. For example, eslint-plugin-sdl behaves differently based on whether there is type information (e.g. see https://github.com/microsoft/eslint-plugin-sdl/blob/31d6d6802499c22f948e780ac573610d248b78a2/lib/ast-utils.js#L40 and other functions in that file).

I agree that if the migration effort were low then this would be a definite "break it and let them know". But since the configs will probably want to support v5 for a little while, and it's very little effort on our end to keep it, I suspect we should actually push back to v7 after all. Thoughts?

[Josh-Cena]

Is it possible for configs to detect the TS-ESLint version and branch logic?

I get the compatibility story of "config use the legacy API to support both v5 and v6, and then use the modern API in the next major to support both v6 and v7"—that also makes sense.

[bradzacher]

I guess the thing is we've always given the advice that you shouldn't do progressive enhancement of lint rules depending on if type information might be there.

Thats the same hard-and-fast rule that we have taken with every single one of our lint rules.

The reason we take this line is because config is hard and if you just silently enhance a rule with type information but the user misconfigures it how can you alert the user to the misconfiguration?

Either use type info or not - never "maybe use it if it's there".

Also as mentioned - this wasn't ever a documented usecase - so should we be so cognisant of breaking an undocumented, not recommended way of using our API?

I get your concern - for sure! But there's sadly no backwards compatible way to remove this. At some point we need to rip the bandaid and downstream consumers will need to figure out how to write their own backwards compatible code.

[JoshuaKGoldberg]

we've always given the advice that you shouldn't do progressive enhancement of lint rules depending on if type information might be there ... this wasn't ever a documented usecase - so should we be so cognisant of breaking an undocumented, not recommended way of using our API?

On the flip side, I don't think the public docs website ever explicitly mentions the advice against progressive enhancement. Filing an issue: #7158

At any rate, 👍 I can close this out again and just post an informative message linking to this thread in the downstream repos. Thanks!