Add req res data warn #1333
Add req res data warn #1333
Conversation
|
Thinking maybe if we could use https://nodejs.org/dist/latest-v16.x/docs/api/util.html#util_util_deprecate_fn_msg_code It is a one time warning function... |
|
I feel like that's more for deprecated functions specifically, due to the node flags for it. Plus, I can't really see how exactly the deprecation function would be used here, unless No-op functions are used, but that kinda defeats the purpose of the deprecation wrapper. |
|
I thought so too... But it's mere elegant in a way also. + We are already using it |
|
I tried using the deprecate wrapper just now, and it's kinda hard to test for, should I remove the warn tests, as they aren't that critical? I don't think there is a good way of checking for |
|
we don't have a test for the other warning message about https://www.geeksforgeeks.org/node-js-process-warning-event/ |
|
Changed the tests and the warnings to use the deprecate API. |
| clone: {enumerable: true}, | ||
| data: {get: deprecate(() => {}, | ||
| '.data is not a valid Response property, use .json(), .text(), .arrayBuffer(), or .body instead', | ||
| 'https://github.com/node-fetch/node-fetch/issues/1000 (response)')} |
There was a problem hiding this comment.
overall i like this PR but wonder if not this getter fn should not be in the body mixin instead.
It's possible to use the Request too new Request(url, {body}).arrayBuffer()
all doe i don't thing anyone will make that misstake... seems almost unnecessary, nobody would do: new Request().data or assigning a value onto it...
|
made some small changes to this in #1421 |
What is the purpose of this pull request?
What changes did you make? (provide an overview)
Added a one-time warning for when .data is used in any way, as a way to help porting from other libraries for requests (such as
axios).Which issue (if any) does this pull request address?
Closes #1000