feat(Symbol.observable): is no longer polyfilled

[benlesh]

BREAKING CHANGE: RxJS will no longer be polyfilling Symbol.observable. That should be done by an actual polyfill library. This is to prevent duplication of code, and also to prevent having modules with side-effects in rxjs.

[benlesh]

cc @jasonaden @IgorMinar

[rxjs-bot]

	Messages
📖

CJS: 1302.1KB, global: 692.3KB (gzipped: 113.3KB), min: 133.7KB (gzipped: 29.3KB)

Generated by 🚫 dangerJS

[coveralls]

Coverage decreased (-0.004%) to 97.447% when pulling 433d774 on benlesh:no-more-symbol-observable-polyfill into 2507518 on ReactiveX:master.

[jayphelps]

This will need to do typeof Symbol because otherwise it still will do Symbol is not defined. See #3394 for reference example

[jayphelps]

Same as above, prolly easiest to wrap in a function like done in #3394 so only need to check once. Also will allow you to easily make sure the type is of symbol instead of any as it is in this case.

[benlesh]

Wrapping it in a function will only make it easier to test. The module body itself should only be called once, no?

[jayphelps]

@benlesh Partially, though it also makes it easier to only need to do the typeof Symbol check once.

[jayphelps]

btw I think these symbols are no longer publicly exported except for in the UMD build. Which one is intended?

I'm not entirely sure if we should be exporting any of them. If we did, seems like Symbol.observable would be the only one we have any business doing so but even then if someone wants to use one of them without native support (no JS env has native support yet, so right now that means any time they want to use it), they should include a polyfill IMO and then rxjs will end up using it transparently anyway so no need to access variable RxJS creates to reference it.

I can see a possible argument about allowing libraries that use RxJS to add support for Symbol.observable without including the polyfill automatically (which would mutate Symbol, forcing it on their users). My gut says that in those cases they too should either follow our lead with "@@observable" string as a fallback, or just choose not to implement support for Symbol.observable when it's not defined. In fact, that's probably what we should be doing too? I'm not sure when the "@@observable" approach is useful?

[benlesh]

I don't disagree, @jayphelps, I think we should only be implementing/executing some logic if Symbol.observable is defined... this was just the quickest incremental step.

[IgorMinar]

looks good now!

Dismissed after offline discussion.

[lock]

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.