Skip to content

Remove redundant code from getResolvedSignatureWorker #52721

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

Closed

Conversation

Andarist
Copy link
Contributor

while investigating what extra tests I should add as part of #52679 i noticed that some pieces here might not be needed (at least according to the existing test suite)

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Feb 10, 2023
@typescript-bot
Copy link
Collaborator

This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise.

@Andarist Andarist force-pushed the cleanup/getResolvedSignatureWorker branch from 91adbd2 to 226985f Compare February 10, 2023 23:34
@andrewbranch
Copy link
Member

@typescript-bot user test
@typescript-bot test top100

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 10, 2023

Heya @andrewbranch, I've started to run the diff-based top-repos suite on this PR at 226985f. You can monitor the build here.

Update: The results are in!

Copy link
Member

@andrewbranch andrewbranch left a comment

Choose a reason for hiding this comment

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

getParseTreeNode returns undefined when the input node is transformed. This is here to prevent crashes when API consumers try to do something they shouldn’t do. I think we want to keep it.

@Andarist
Copy link
Contributor Author

In such a case, there is no point in this PR. More meaningful changes in this code are happening here anyway

@Andarist Andarist closed this Feb 11, 2023
@typescript-bot
Copy link
Collaborator

@andrewbranch Here are the results of running the top-repos suite comparing main and refs/pull/52721/merge:

Everything looks good!

@Andarist
Copy link
Contributor Author

@andrewbranch I got curious about this case - is there a way to write a unit~ test that would cover this scenario? 🤔

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.

3 participants