Skip to content

Conversation

@ImRodry
Copy link
Contributor

@ImRodry ImRodry commented Nov 3, 2022

Description

What is changing?

This PR simply moves all deprecated function overloads to the bottom of their signatures so that the overloads that should be used come up first and are preferred by TS

Is there new documentation needed for these changes?

No, they were already documented in the PR that introduced the deprecations

What is the motivation for this change?

When using TS's built-in types such as ReturnType or others, TS will usually grab the first overload of the function. Along with that, if you ever had an error in some of your function's arguments, the overload would often fallback to the callback one since, in the presence of an error, TS fins the first overload that matches the amount of parameters passed, causing your function's return type to be void. Also by doing this it will make it easier to remove all of these overloads when the time comes for that, and it will also make the diff look nicer

https://jira.mongodb.org/browse/NODE-4798

Double check the following

  • Ran npm run check:lint script
    Got this error when running it but I believe that's not my fault
    image
  • Self-review completed using the steps outlined here
  • PR title follows the correct format: <type>(NODE-xxxx)<!>: <description>
  • Changes are covered by tests
  • New TODOs have a related JIRA ticket

@ImRodry
Copy link
Contributor Author

ImRodry commented Nov 6, 2022

Dunno why CI is failing and I can't see why either

@durran durran added the Primary Review In Review with primary reviewer, not yet ready for team's eyes label Nov 8, 2022
Copy link
Member

@durran durran left a comment

Choose a reason for hiding this comment

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

Verified the deprecated overloads do now show up as the last suggestions in VS Code. Example:

Screenshot 2022-11-09 at 15 56 19

Failing tests unrelated.

@durran durran added Team Review Needs review from team and removed Primary Review In Review with primary reviewer, not yet ready for team's eyes labels Nov 9, 2022
@dariakp dariakp changed the title types(NODE-4798): move deprecated overloads to the bottom docs(NODE-4798): move deprecated overloads to the bottom Nov 9, 2022
@durran durran merged commit b70cc7c into mongodb:main Nov 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Team Review Needs review from team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants