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

docs(NODE-4798): move deprecated overloads to the bottom #3461

Merged
merged 1 commit into from Nov 9, 2022
Merged

docs(NODE-4798): move deprecated overloads to the bottom #3461

merged 1 commit into from Nov 9, 2022

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
3 participants