-
-
Notifications
You must be signed in to change notification settings - Fork 32
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 browser support #122
Fix browser support #122
Conversation
65e9334
to
95e5f30
Compare
Fixed the PR. Ready for review. |
@@ -43,6 +43,7 @@ | |||
"concat" | |||
], | |||
"dependencies": { | |||
"@sec-ant/readable-stream": "^0.3.2", |
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 dependency is quite small: ~250 lines, no dependencies.
I looked into each implementation, and its behavior is almost identical to both the Node.js implementation, and the 400KB official polyfill for web streams.
// Use this library as a ponyfill instead of a polyfill. | ||
// I.e. avoid modifying global variables. | ||
// We can remove this once https://github.com/Sec-ant/readable-stream/issues/2 is fixed | ||
if (prototype[Symbol.asyncIterator] === undefined && prototype.values === undefined) { |
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.
This condition is always false
on Node, Firefox and Chrome >124. On those platforms, this file is a noop.
@@ -0,0 +1,13 @@ | |||
export const ponyfill = {}; |
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.
@sindresorhus Didn't you coin that term? Happy to spread it! 🦄
await import('@sec-ant/readable-stream'); | ||
ponyfill.asyncIterator = prototype[Symbol.asyncIterator]; | ||
delete prototype[Symbol.asyncIterator]; | ||
delete prototype.values; |
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.
Not polyfilling ensures we don't get issues for users that use the "official" web streams polyfill. Also, this avoids any global side effects.
The author of the polyfill is working on turning it into a ponyfill. When that happens, we can remove this file entirely.
Fixes #116.