Skip to content

Assume that type node annotations resolving to error types can be reused #60195

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

Merged
merged 4 commits into from
Oct 11, 2024

Conversation

Andarist
Copy link
Contributor

fixes #60192
similar to #60129

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Oct 10, 2024
Comment on lines +6244 to +6248
// allow "reusing" type nodes that resolve to error types
// those can't truly be reused but it prevents cascading errors in isolatedDeclarations
// for source with errors there is no guarantee to emit correct code anyway
return true;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be handled elsewhere but this place is an easy and effective fix.

Note that in a sense this was already happening - just in different situations. Consider this:

export const foo3 = (type: Unresolved): void => {};

In here both the annotationType and type are error types, they just happen to be the same aliased error types. So they are assumed to be the same on the basis of typeFromTypeNode === type in typeNodeIsEquivalentToType.

The reported case is the same yet different. Both types are also error types there but the type is a regular non-aliased error type whereas the annotationType is an aliased error type. Based on that typeNodeIsEquivalentToType doesn't assume that they are the same and thus typeFromParameter goes into inferTypeOfDeclaration which leads to reporting the error.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a condition in which this will cause us to try and print something unprintable?

Or will it not matter because it'll be an error anyway?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I propose this fix mainly on the assumption of "will it not matter because it'll be an error anyway". It seems to me that if the error is there anyway then any emit is basically a fair game. It should reuse the annotation node - so at the very least it should just be broken in the same way.

@jakebailey jakebailey merged commit c003609 into microsoft:main Oct 11, 2024
32 checks passed
@sandersn sandersn removed this from PR Backlog Apr 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[5.7-beta][isolatedDeclarations][regression] Missing explicit type annotation false positive
4 participants