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

Declaration emit should retain (instead of elide) unresolved computed names #58428

Conversation

weswigham
Copy link
Member

Historically, if a [name]: whatever didn't resolve, it was elided in a declaration file, even when you wrote it explicitly in a type. This PR changes that behavior to instead copy the name into the output even when it doesn't resolve. Since the input has a checker error in this case, our declaration output here is flexible, and this makes us align with expectation under isolatedDeclarations.

Fixes #58423

@typescript-bot typescript-bot added Author: Team For Milestone Bug PRs that fix a bug with a specific milestone labels May 3, 2024
@weswigham weswigham marked this pull request as ready for review May 4, 2024 01:00
@jakebailey
Copy link
Member

This seems fine but I'm still not clear how this totally fixes the linked issue; shouldn't there be an error somewhere about this?

@weswigham
Copy link
Member Author

shouldn't there be an error somewhere about this?

There is... if the file is typechecked. I believe this isn't an isolated declarations error since it's the expectation that usually people won't write nonfunctional/conflicting computed names, and it'll just copy the input nodes to the output, which this PR adjusts our node builder and declaration emitter to do.

@jakebailey
Copy link
Member

Ah. Right, we don't include check errors in this output (unless we happen on them)

Copy link
Member

@jakebailey jakebailey left a comment

Choose a reason for hiding this comment

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

But we probably need to warn about that somewhere... But it's not like transpileModule is much different. And a syntactic transpiler will probably just copy too.

@weswigham weswigham merged commit 0d3c481 into microsoft:main May 7, 2024
28 checks passed
@weswigham weswigham deleted the transpile-declaration-preserve-computed-name-in-type branch May 7, 2024 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Milestone Bug PRs that fix a bug with a specific milestone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[transpileDeclaration API][5.5] Type containing enum values is incorrectly emitted
3 participants