fix(Symbol.iterator): correctly handle case where Symbol constructor itself is not defined #3394
Conversation
Generated by 🚫 dangerJS |
---9.6KB-green.svg?style=flat-square){kind=icon}
---1.5KB-green.svg?style=flat-square){kind=icon}
| by the library. However it will provide clues to users on older | ||
| browsers why things like `from(iterable)` doesn't work. */ | ||
| if (!Symbol || !Symbol.iterator) { | ||
| console.warn('RxJS: Symbol.observable does not exist'); |
There was a problem hiding this comment.
s/observable/iterator too. I missed this before 🙂
src/internal/symbol/iterator.ts
Outdated
| } | ||
|
|
||
| /** The native Symbol.iterator instance or a string */ | ||
| export const iterator = Symbol && Symbol.iterator || '@@iterator'; | ||
| export const iterator = getSymbolIterator(root); |
There was a problem hiding this comment.
We actually want to get rid of root at some point. It's caused more issues than it's fixed.
There was a problem hiding this comment.
do you have alternatives for testing?
…itself is not defined
| @@ -1,14 +1,17 @@ | |||
| export function getSymbolIterator(): symbol { | |||
| /* NOTE: Warning users that they don't have a Symbol.iterator | |||
There was a problem hiding this comment.
So we should probably just remove the console.warn (and related comment) and document it in the migration docs. Because it's going to prompt users to add polyfills they don't need, or even to come file issues that they're getting warnings they don't understand.
benlesh
dismissed
their stale review
I can dismiss my own review if I want to! :D
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Follow up to #3389. This also makes the type a real
symbolinstead ofany(return type ofgetSymbolIteratordoes it)#3387 also needs to be adjusted as well.