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

Always emit any for get accessors #36279

Closed
wants to merge 2 commits into from
Closed

Conversation

DanielRosenwasser
Copy link
Member

Fixes at least half of #36216 by always emitting any for get accessors.

Note that set accessors stil emit without parameter types, and from what I can tell TypeScript 3.6 is okay with that under noImplicitAny.

@DanielRosenwasser
Copy link
Member Author

@typescript-bot cherry-pick this to release-3.7

@typescript-bot
Copy link
Collaborator

Hey @DanielRosenwasser, I couldn't open a PR with the cherry-pick. (You can check the log here). You may need to squash and pick this PR into release-3.7 manually.

@rbuckton
Copy link
Member

Don't we normally elide the type annotation for private declarations?

@weswigham
Copy link
Member

Eeeyes? I think so. Unsure. @sandersn ?

@DanielRosenwasser
Copy link
Member Author

Correct, in #36216 it was reported that 3.6.x incorrectly thinks that an error under noImplicitAny.

@DanielRosenwasser
Copy link
Member Author

For the record, we stopped erroring on that in thanks to a PR from @ajaff in #33896.

@sandersn
Copy link
Member

@rbuckton is right, we elide the type annotation for private declarations. The correct thing is not to error on then, so I don't think we should take this PR.

Copy link
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

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

We should not error instead of emitting an any type.

@DanielRosenwasser
Copy link
Member Author

DanielRosenwasser commented Jan 22, 2020

I'm potentially okay with that - can you check out #36280 then?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants