[rxjs] Establish interop of rxjs-6 and kbn-observable #21722
Conversation
rhoboat
added
review
Team:Core
💚 Build Succeeded |
| @@ -1,4 +1,4 @@ | |||
| import symbolObservable from 'symbol-observable'; | |||
There was a problem hiding this comment.
Can symbol-observable be removed from the dependencies, then?
| @@ -1,4 +1,4 @@ | |||
| import symbolObservable from 'symbol-observable'; | |||
| import { observable as Symbol_observable } from 'rxjs/internal/symbol/observable'; | |||
There was a problem hiding this comment.
Why change the variable name away from camelCase? We don't generally use variables with underscores in them.
This applies to the other file as well.
There was a problem hiding this comment.
Ah, true. I got carried away using the same naming as rxjs.
|
I regenerated lock files because removing symbol-observable with |
|
@epixa Ready for another review after CI finishes. I might have to revert lock file changes. |
|
Yeah, I'm going to revert the lockfile changes. Something else is causing symbol-observable to remain, it's probably valid. |
💚 Build Succeeded |
|
Since we finally came up with the decision to abandon |
💚 Build Succeeded |
* [rxjs] Establish interop of rxjs-6 and kbn-observable * Remove symbol-observable
* [rxjs] Establish interop of rxjs-6 and kbn-observable * Remove symbol-observable
Change the underlying symbol of kbn-observable to be what rxjs-6 uses.
Blocks #20272 (WIP)
Tests only pass with the changes made here. We could write more if you think necessary for confidence. Just kept it as simple as possible.
Question: this and this needs a change too, but not sure.