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

previousState updating #671

Closed
wants to merge 2 commits into from
Closed

previousState updating #671

wants to merge 2 commits into from

Conversation

Ramesh-X
Copy link

Status

READY

Breaking Changes

NO

Description

Currently BlocBuilder is changing its previousState no matter the Widget was built or not. But this PR will update previousState only if the Widget was rebuilt.

This might be helpful when number of streams do changes to the State in bloc, but you need to rebuild Widget only if the all the changes are correctly done.

Todos

  • Tests
  • Documentation
  • Examples

Impact to Remaining Code Base

This PR will affect:

  • BlocBuilder build logic is changed

previousState updates only if the condition returns true.
@felangel
Copy link
Owner

@Ramesh-X thanks for opening a pull request. Can you please provide some more context? Is there a concrete use-case that you have?

@Ramesh-X
Copy link
Author

Consider the following case where I have two actions called F1Action and F2Action that will be dispatched asynchronously. The BlocBuilder will be rebuilt on every state change only if the both variables have changed.

  class A {
    int f1;
    int f2;
    A(this.f1, this.f2);
  }

  @override
  Stream<A> mapEventToState(RootPageAction action) async* {
    if (action is F1Action) {
      yield A(action.val, state.f2);
    } else if (action is F2Action) {
      yield A(state.f1, action.val);
    }
  }

  return BlocBuilder<MyBloc, A>(
    condition: (pre, current) => pre.f1 != current.f1 && pre.f2 != current.f2,
    builder: ...
  );

With the existing code, the BlocBuilder will never be rebuilt, because in every state change either f1 or f2 will remain constant. Therefore the condition will not be satisfied every time.

But with my PR, I'm keeping the previous state which the BlocBuilder was rebuilt and compare the current state with that.

Despite of the example, I think it is better to do it this way anyhow. Because a Widget should always be rebuilt only if the state it was previously built has changed (and satisfy the condition). It should not consider the state changes that was not contributed to any rebuild. I hope you can understand the point that I'm making..

You could tell that I can satisfy my requirements by changing && to ||. Of course it will do the thing I want but with a few unnecessary rebuilds.

@felangel
Copy link
Owner

@Ramesh-X thanks for the clarification 👍. For the PR to be able to be merged you’ll need to fix unit tests. Also, a similar change should also be made to BlocListener. Are you able to make those changes or would you like me to? Thanks again!

@felangel
Copy link
Owner

@Ramesh-X I've opened a PR with the improvements to both BlocListener and BlocBuilder at #678

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants