-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Merge rx-recompose into main project #196
Changes from 6 commits
50d86f5
61eb5fd
534415d
fbb62b6
d9bb6f7
12de7e3
5732a70
c0ccf69
b3c5d4d
95fe144
14ae089
f5f1cd5
e3ff58b
ea8ec6d
1dfa6f3
af51c21
01f0aab
b47d748
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,17 +1,26 @@ | ||
import { Component } from 'react' | ||
import { Observable } from 'rx' | ||
import { createChangeEmitter } from 'change-emitter' | ||
import $$observable from 'symbol-observable' | ||
import { getTransform } from './configureObservable' | ||
|
||
const createComponent = propsToVdom => | ||
class RxComponent extends Component { | ||
const componentFromStream = propsToVdom => | ||
class ComponentFromStream extends Component { | ||
state = {}; | ||
|
||
propsEmitter = createChangeEmitter(); | ||
|
||
// Stream of props | ||
props$ = Observable.create(observer => | ||
this.propsEmitter.listen(props => observer.onNext(props)) | ||
); | ||
props$ = getTransform()({ | ||
subscribe: observer => { | ||
const unsubscribe = this.propsEmitter.listen( | ||
props => observer.next(props) | ||
) | ||
return { unsubscribe } | ||
}, | ||
[$$observable]() { | ||
return this | ||
} | ||
}); | ||
|
||
// Stream of vdom | ||
vdom$ = propsToVdom(this.props$); | ||
|
@@ -23,17 +32,16 @@ const createComponent = propsToVdom => | |
|
||
componentWillMount() { | ||
// Subscribe to child prop changes so we know when to re-render | ||
this.subscription = this.vdom$.subscribe( | ||
vdom => { | ||
this.subscription = this.vdom$.subscribe({ | ||
next: vdom => { | ||
this.didReceiveVdom = true | ||
if (!this.componentHasMounted) { | ||
this.state = { vdom } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why check on hasMounted? (It was a bug in react, but it solved) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Link?
You're right. This is left over from when it used to subscribe inside the constructor. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
One of: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The check is to prevent calling |
||
return | ||
} else { | ||
this.setState({ vdom }) | ||
} | ||
this.setState({ vdom }) | ||
} | ||
) | ||
|
||
}) | ||
this.propsEmitter.emit(this.props) | ||
} | ||
|
||
|
@@ -52,7 +60,7 @@ const createComponent = propsToVdom => | |
|
||
componentWillUnmount() { | ||
// Clean-up subscription before un-mounting | ||
this.subscription.dispose() | ||
this.subscription.unsubscribe() | ||
} | ||
|
||
render() { | ||
|
@@ -61,4 +69,4 @@ const createComponent = propsToVdom => | |
} | ||
} | ||
|
||
export default createComponent | ||
export default componentFromStream |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
// Default transform is identity function | ||
let transform = t => t | ||
|
||
export const getTransform = () => transform | ||
|
||
const configureObservable = newTransform => { | ||
transform = newTransform | ||
} | ||
|
||
export default configureObservable |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
import $$observable from 'symbol-observable' | ||
import { createChangeEmitter } from 'change-emitter' | ||
import { getTransform } from './configureObservable' | ||
|
||
const createEventHandler = () => { | ||
const emitter = createChangeEmitter() | ||
const transform = getTransform() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @acdlite Maybe allow pass to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @typeetfunc There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @chicoxyzzy Thanks for links. Also observer dont has interop point like |
||
const stream = transform({ | ||
subscribe(observer) { | ||
const unsubscribe = emitter.listen(value => observer.next(value)) | ||
return { unsubscribe } | ||
}, | ||
[$$observable]() { | ||
return this | ||
} | ||
}) | ||
return { | ||
handler: emitter.emit, | ||
stream | ||
} | ||
} | ||
|
||
export default createEventHandler |
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.
Bind operator is still at stage 0 so syntax may change in future and there are no guarantees it will be included in spec
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.
Well it's a test, so oh well for now :D
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 reason I'm importing each helper separately is to ensure as much as possible that we're not depending on any RxJS-specific functionality.
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.
yes, I understand that