-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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
stream: edit variables for readableStreamPipeTo #55769
Conversation
e122989
to
5fafd62
Compare
const preventCancel = options?.preventCancel; | ||
const preventClose = options?.preventClose; | ||
const signal = options?.signal; | ||
const { preventAbort, preventCancel, preventClose, signal } = options; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't options
also be null
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when I looked at the places where it was crossed, I saw that it was protected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The line right above (validateObject) allows a null value. We should account for that.
What if the user calls this function:
pipeThrough(..., null)
Than this'll throw a destructoring error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks I made an update that prevents it from giving an error, but the old code was better in terms of readability
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #55769 +/- ##
==========================================
- Coverage 88.41% 88.40% -0.01%
==========================================
Files 654 654
Lines 187752 187751 -1
Branches 36125 36120 -5
==========================================
- Hits 165993 165977 -16
- Misses 15000 15003 +3
- Partials 6759 6771 +12
|
5fafd62
to
9ff094c
Compare
the old version is more readable and beautiful so I'm turning this off, thanks for the support |
I tried to make it better using destructuring