-
Notifications
You must be signed in to change notification settings - Fork 84
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Request: pass in observable directly instead of factory #8
Comments
Good suggestion. In a react component, if I want declare a Observable and using it: function App () {
const counter$ = new BehaviorSubject(0)
const value = useObservable(counter$)
return <div>{counter}</div>
} the You can pass the But if The other additional advantage is, I can expose this API: function useObservable<T, U>(sourceFactory: (props$: Observable<U>) => Observable<T>, initialState: T, inputs: U): T users can easily compose the |
I wouldn't expect anyone to create the Subject in component. But if they did I don't think you would switchMap or exhaustMap. Just unsub the old observable and sub to the new one. That said, I think it would be ok just not to support that like you do now with the factory. Document that its only sub-ed during mount and thats it. I don't think the case is much different with or without the factory function. In my case, I have services that provide observables. I'm using your useObservable as a replacement for my old withObservables HOC. You can see my use case in that repo @cgross/withObservables. And sure you'd have to provide an overloaded API like: function useObservable<T>(source:Observable<T>,initialState:T):T; |
The observable maybe change, then you will need to do And your use of useObservable is wrong. You should use it in this way: useObservable((inputObs) => inputObs.pipe(mergeMap(([obs]) => obs)),{}, [myObservable]) |
Why ever would I do that? What does that gain vs: |
@cgross |
@TotooriaHyperion That's horrible advice and I hope for the sake of anyone who works with you that you don't over-complicated all of your code like that. |
If you read the source code, you will find that only the first output from the factory is used. If you use a singleton observable, you can use useObservable(obs) // currently not supported
or
useObservable(() => obs) If you are using an observable that created dynamicly, you should use: function useChangingObservable<T>(obs: Observable<T>, initial?: T): T {
return useObservable(input => input.pipe(switchMap(([_obs]) => _obs)), initial, [obs])
} What if the useObservable(obs) are supported at the very beginning? It will confuse people why only the first observable works. React is not well-fit with rxjs, so we can't help but to do things ugly. |
@cgross I also have issues like this and I think it's becasue the api is a bit confusing. The mixing (or splitting) of event$, inputs$ and state$ is restraining the flexibility and making the api very hard to use. So after #60 when it finally became unusable to me. I took a step back, put some serious thoughts on the relationship between React hooks and RxJS. Ended up re-designing the api and made observable-hooks which I think fixes some of the issues including this one and #73 #51 . Althought ended up making my own wheel, I still thank this library for bring true inspiration on hooks and observables. |
@crimx, thanks for the works! However, wasn't the library still looks a bit too complicated?! I mean, do we really need to recreate state management over and over again?! Putting everything into observable doesn't Reacty. React already have nice solution based on I think we really need to separate the concerns here, there barely have a chance that a component will update itself without any user interaction to the UI. Those interactions are dispatched as DOMEvents, which is equivalent to actions basically. So what we need is the ability to chaining and orchestrating these events and In other words, I don't think passing observable around or as props is a good pattern, it potentially breaks the uni-directional flow. Anyone can complete the observable silently, making it hard to debug. |
You can use the bit you need which is not complicated at all. It's like RxJS operators. There are tons but normally you just use a few of those.
Not my intention.
Observables are just funciton calls which can barely impact performance in normal cases. But I agree with you on not abusing Observables.
I think
@leoyli I think you misunderstood my design. props are just normal props. |
Here is an example on your case import {
useObservableState,
useObservableCallback,
pluckCurrentTargetValue
} from 'observable-hooks'
const [onChange, textChange$] = useObservableCallback(pluckCurrentTargetValue)
const text = useObservableState(textChange$, '') Or just import {
useObservableState,
pluckCurrentTargetValue
} from 'observable-hooks'
const [text, onChange] = useObservableState(pluckCurrentTargetValue, '') With deps import {
useObservable,
useObservableState,
pluckFirst
} from 'observable-hooks'
// props.prefix is normal string type
const prefix$ = useObservable(pluckFirst, [props.prefix])
const [text, onChange] = useObservableState(event$ => event$.pipe(
withLatestFrom(prefix$),
map(([e, prefix]) => prefix + e.currentTarget.value)
)) |
@crimx, perhaps. But as long as I see And the As you may find in my suggested implementation, I heavily use And I'd to learn more about why you see |
Just simple helper functions to avoid garbage collection for common use cases. Totally optional.
You got your point, I repect that, and observable-hoos can also fit the I think my goal for the library is more about consistency. If I already use Observables, I want to think in the Observable way. And messing with |
@crimx, right. I think your solutions covers the entire use case if we just want to use React + RxJS and make everything observable, which is really nice. And surely But as many experiments have been don yearly. Generally, most people will agree React doesn't fit too well with RxJS, especially when we start to optimize our app to cut down unnecessary rendering with But thank you for paying time for these interesting discussion. I'm surely would like to use your library for my next side-project. |
One can, but one don't have to. What I care is reusing stateful logic which is what hooks are for. With Observables now async logic can be nicely resued. This can just be a simple typeahead component. Redux is still necessary for complex state management. But with hooks we don't have to dump everything into the global state. |
Thanks! I fear the owner of this repo is going to punch me if I keep on babbling. |
@crimx feel free to make discuss happen here. |
@Brooooooklyn what about add another api called |
Great library. I would be great if I could just pass an observable into the hook directly:
useObservable(myObservable);
instead of
useObservable(() => myObservable);
The text was updated successfully, but these errors were encountered: