-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Update rxjsObservableConfig for rxjs 6 #660
base: master
Are you sure you want to change the base?
Conversation
@@ -93,20 +109,3 @@ test('complete props stream before unmounting', () => { | |||
wrapper.unmount() | |||
expect(counter).toBe(0) | |||
}) | |||
|
|||
test('completed props stream should throw an exception', () => { |
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.
It seems like rxjs 6 doesn't throw this error anymore. I need dig deeper into this test.
What are u think to split library on 2, ie recompose and recompose-rx(-observable, -blabla) and move all the streaming parts out of the recompose library. Also I think we can move on https://github.com/facebook/react/tree/master/packages/create-subscription to support async in the future |
It would be a good idea to separate them. But I'm thinking about keeping the es-observable parts remain in Recompose and move only those 3rd-party-config files into other libraries. The reason for me to separate them is to have better version controlling. And yes, I think we need to introduce create-subscription to handle streaming events. |
RxJS 6 has been out for awhile now. Is this PR good to merge? Re. the suggestion of moving 3rd-party-config files into other libraries, perhaps keep that idea to a separate PR, so we can unblock this one? |
For those people waiting for this PR, there is a simple configuration possible to get recompose working with RxJS@^6.0.0: import { from } from 'rxjs';
setObservableConfig({
fromESObservable: from,
}); |
Need some discussion before merging:
There were
rxjs4ObservableConfig.js
andrxjsObservableConfig.js
(for rxjs 5) before. This PR upgraderxjsObservableConfig.js
directly to adopt rxjs 6, but it's a breaking change. We might want to create separate files for both rxjs 5 and rxjs 6.Rxjs used to polyfill
Symbol.observable
, but it has been removed since feat(Symbol.observable): is no longer polyfilled ReactiveX/rxjs#3387. We might need to add a notice about polyfillSymbol.observable
to the doc.