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

fix(types): declare buffer() deprecated #1345

Merged
merged 1 commit into from Oct 23, 2021

Conversation

dnalborczyk
Copy link
Contributor

@dnalborczyk dnalborczyk commented Oct 21, 2021

What is the purpose of this pull request?

  • Documentation update
  • Bug fix
  • New feature
  • Other, please explain: adds a type deprecation to the already existing runtime deprecation.

What changes did you make? (provide an overview)

Which issue (if any) does this pull request address?

Is there anything you'd like reviewers to know?

LinusU
LinusU approved these changes Oct 22, 2021
Copy link
Member

@LinusU LinusU left a comment

Neat 👍

@dnalborczyk
Copy link
Contributor Author

@dnalborczyk dnalborczyk commented Oct 22, 2021

@LinusU the deprecation message is taken 1:1 from:

Body.prototype.buffer = deprecate(Body.prototype.buffer, 'Please use \'response.arrayBuffer()\' instead of \'response.buffer()\'', 'node-fetch#buffer');

I didn't want to deviate from the above, but I personally think it should rather be something like:

'Body.prototype.buffer()' is deprecated. Please use 'Body.prototype.arrayBuffer()' instead.

as response is the name of the instance, which not everyone would name it response necessarily.

wording similar to:
https://github.com/nodejs/node/blob/d0a898681f8d5a5fcd53fa2ab8e0a3da807791be/lib/internal/bootstrap/node.js#L295

I can do follow-up PR, or push the changes on top of this one.

@jimmywarting jimmywarting merged commit 965b323 into node-fetch:main Oct 23, 2021
1 check passed
@dnalborczyk dnalborczyk deleted the types-deprecate branch Oct 26, 2021
@renovate renovate bot mentioned this pull request Jan 27, 2022
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants