Skip to content
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

Moving builder and condition into a "use" hook (maybe useBlocBuilder) for react-bloc #39

Closed
theweiweiway opened this issue Jan 18, 2021 · 5 comments
Labels
enhancement New feature or request
Projects

Comments

@theweiweiway
Copy link

theweiweiway commented Jan 18, 2021

Hi Felix!

Thanks for your port of the bloc library to React! I have a feature suggestion due to the limitations I'm facing with <BlocBuilder ... />

Is your feature request related to a problem? Please describe.
<BlocBuilder .../> is great, but it has some limitations:

  • every component that needs access to the bloc <BlocBuilder bloc={<how_do_i_easily_get_this_bloc>} ... /> in the tree must be prop-drilled down, OR passed via useContext. The problem with useContext is that it doesn't update when bloc state changes (not sure why it behaves like this). For example:
 const authBloc = useContext(AuthBlocContext);
 console.log(authBloc.state)

 // somewhere else in app
 authBloc.add(AuthEvent.logIn)

this does not trigger the console.log to show the new state. However, if I use a <BlocBuilder bloc={<bloc_from_useContext>}/> component and pass the authBloc in from the useContext, I get the most updated state. But using authBloc.state directly from the context won't give me the newest state. This has implications, for example:

  • not easy to access the bloc state outside of the render body. For example, it is not easy to access the state in a useEffect hook (in Flutter, we can just use context to access the most updated version of the Bloc in initState)
  • Finally, another drawback of <BlocBuilder .../> is that we are relying on a renderProp to use the state which is not a huge problem, but it is undesirable

Describe the solution you'd like
If we had a useBlocBuilder hook instead of a <BlocBuilder ... /> component like so:

const [bloc, state]= useBlocBuilder(AuthBloc, {
  condition: (previous, current) => { 
    // some logic here
  }
})

useEffect(() => {
  // easily access bloc state here with `state`
}, []}

return (
  <div>{
    // do stuff with state 
    // call events with bloc.add
  }</div>
)

On top of this, we could also have a useBlocListener hook

Not only is this much more react-ish, it's more concise and easier to use (in my opinion). AND, it still preserves the "spirit" of bloc library by using BlocBuilders, having conditions, etc.

@theweiweiway theweiweiway changed the title Moving builder and condition into a "use" hook (maybe useBloc) for react-bloc Moving builder and condition into a "use" hook (maybe useBlocBuilder) for react-bloc Jan 18, 2021
@felangel felangel added the enhancement New feature or request label Jan 18, 2021
@felangel
Copy link
Owner

Hi @theweiweiway 👋
Thanks for opening an issue!

I agree that your proposal is more "react-like" and would be a lot more familiar to the existing react community while still preserving the core concepts/behavior of the bloc library.

Unfortunately, I'm focusing on migrating the dart bloc ecosystem to null safety right now but if you're willing to give this a shot, I'm always happy to review the code 👍

If you're busy/prefer not to work on the implementation, no worries! I'll try to get to this in the coming weeks 😄

@theweiweiway
Copy link
Author

Hey Felix,

This might be over my head, but I'll look through the code and try to give it a shot!

@felangel
Copy link
Owner

Hey Felix,

This might be over my head, but I'll look through the code and try to give it a shot!

No worries at all! If you have any questions don't hesitate to ask and again if you prefer to wait that's totally fine as well 👍

@theweiweiway
Copy link
Author

Hey @felangel, I've got something that kind of looks like a useBlocBuilder hook - but I think I'm missing a few things which is preventing it from working properly. I've been modelling this hook off of your <BlocBuilder /> component - I have a few questions though.

Here's your <BlocBuilder /> component. I've added comments in to explain what I think the code is doing. Can you let me know if my understanding is correct? For now, I'm omitting previous state and condition and only focusing on the core streaming functionality. I've also added some questions in the code

  private subscribe(): void {
    this.subscription = this.bloc.listen((state: S) => {
      let rebuild: boolean =
        this.condition !== null ? this.condition.call(this, this.previousState, state) : true

      if (rebuild) {
        // this setState updates the state that is being emitted.
        // this also causes componentDidUpdate to fire 
        // which will update ensure we are listening to
        // the most updated version of the bloc
        this.setState({ blocState: state })
      }
      this.previousState = state
    })
  }

  private unsubscribe(): void {
    this.subscription.unsubscribe()
  }

  componentDidUpdate(prevProps: BlocBuilderProps<B, S>): void {
    // every time there is a `blocState` change, we need to update
    // this.bloc so that it has the most recent version of the bloc.
    // The most recent version of the bloc is always `props.bloc`! (is 
    // this assumption actually correct?)
    if (prevProps.bloc !== this.props.bloc) {
      this.unsubscribe()
      // update this.bloc with most updated version of the bloc
      this.bloc = this.props.bloc
      this.previousState = this.bloc.state

      // not 100% sure why we are setStating here. Didn't the
      // setState in the subscribe function already bring us to most
      // updated state? 
      this.setState({ blocState: this.bloc.state })

      // now we resubscribe, which will listen to the most
      // updated version of the bloc
      this.subscribe()
    }
  }

OK - Thanks Felix!

@theweiweiway
Copy link
Author

Closing due to PR #40

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
bloc.js
  
Awaiting triage
Development

No branches or pull requests

2 participants