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

converter is not called after parent changes #78

Closed
jeonghoonkim opened this issue Sep 17, 2018 · 7 comments · Fixed by #160
Closed

converter is not called after parent changes #78

jeonghoonkim opened this issue Sep 17, 2018 · 7 comments · Fixed by #160

Comments

@jeonghoonkim
Copy link

jeonghoonkim commented Sep 17, 2018

Hello

I'm not sure this is intended... can you take a look at it?

mm... Here is sample child widget.

class ChildViewModel {
  ChildViewModel({this.data1, this.data2});

  final String data1;
  final String data2;
}

class ChildView extends StatefulWidget {
  final String someId;

  ChildView({
    this.someId
  });

  @override
  State<StatefulWidget> createState() => new ChildViewState();
}

class ChildViewState extends State<ChildView> {
  @override
  Widget build(BuildContext context) {
    print(widget.someId); // #1
    return new StoreConnector<ReduxState, ChildViewModel>(
      converter: (store) {
          print(widget.someId); // #2
          return ChildViewModel(data1: ..., data2: ...);
      },
      builder: (context, viewModel) {
          print(widget.someId); // #3
      }
    );
  }
}

If i change 'someId' value of ChildView from 'aaaa' to 'bbbb', converter is not called.
ChildView(someId: 'aaaa') -> ChildView(someId: 'bbbb')
then above code print like this.

aaaa -> #2 
bbbb -> #1
bbbb -> #3
@arthurgustin
Copy link

@jeonghoonkim Hi, if you test on your phone, you may have the same issue than me, sometimes the app is not built completely and I noticed I had to uninstall the app, delete the build folder from the project and rebuild, and it works

@brianegan
Copy link
Owner

brianegan commented Sep 28, 2018

Hey hey :) Hrm, that's an interesting one... The converter function is only called when Store's stream emits a change, but perhaps I should change that up. Thanks for the report!

@xqwzts
Copy link
Contributor

xqwzts commented Oct 8, 2018

@brianegan I ran into a use case where this would be handy, so put together a quick solution to trigger the stream on each didUpdateWidget call (quick implementation diff here).

Haven't PR'ed as I'm not sure it's the nicest way to do this, setting that rebuildOnUpdate flag will be pretty inefficient (especially if not used in combination with distinct).

@justinsilvestre
Copy link

justinsilvestre commented Feb 11, 2019

One workaround to force an update is to give your widget a Key derived from the values that should trigger the update.

Before

ChildView(someId: 'aaaa');

After

final someId = 'aaaa';
ChildView(someId: someId, key: Key(someId));

@jeonghoonkim
Copy link
Author

@justinsilvestre oh my god... that's right. Why did I not think of that before?.... Thanks!

@khainke
Copy link

khainke commented Aug 20, 2019

Manual key management should not be the solution here, i prepared an example to highlight the problem: https://github.com/khainke/flutter_redux_multiple_counter.git (after an add, when switching between the counters, the index text gets updated immediately but the displayed value will still display the old selected counter value instead of the new selected counter value, until the next other add, then the converter function will get evaluated).

This behaviour looks a bit odd to me, and was pretty unexpected to be honest. Right now im facing some bugs because i was using the converter argument to select individual parts of lists and maps from the store, in an effort to keep the selection maximally local and small. But i probably have to select the lists and maps as a whole in the converter and actually convert them in the build method. Again, it looks odd to me.

It should be documented, that the converter function is supposed to be constant and that a rebuild of a store connector in the same context with a different converter function will not update the value.

The relevant code point: In the didUpdateWidget function of the StoreStreamListener the init function gets called again, when the store reference changes, see here. Why not also do this for changes to the converter function? Or atleast recalculate the latestValue ? Is there a reason for this strange update behaviour on this widget?

@rostyslavroshak
Copy link

rostyslavroshak commented Sep 12, 2019

We have the same issue. Basically we have a list of items and every item has it's own StoreConnector. Something like:

class ForecastStoreConnector extends StatelessWidget {
  const ForecastStoreConnector(this.panel) : assert(panel != null);

  final Panel panel;

  @override
  Widget build(BuildContext context) {
    return StoreConnector<AppState, PanelTileForecastViewModel>(
      converter: (store) => PanelTileForecastViewModel.fromStore(store, panel),
      builder: (context, vm) => PanelTileForecast(vm),
      distinct: true,
    );
  }
}

The issue is that convert is invoked only after some action is send to the store.

The most obvious fix with using keys is not ideal, because it recreate a widget, which causes a performance degradation.

Let's discuss how we can fix it. I am fine with doing that, but need to confirm way.

Option1

Invoke convert during didUpdateWidget, in case distinct is used, it should not be an issue.

Option2

Provide dependentPropertiesExtractor property, in my case:

return StoreConnector<AppState, PanelTileForecastViewModel>(
      dependentPropertiesExtractor: [panel],
      converter: (store) => PanelTileForecastViewModel.fromStore(store, panel),
      builder: (context, vm) => PanelTileForecast(vm),
      distinct: true,
    );

Inside didUpdateWidget check weather list is changed and invoke convert if needed.

P.S. I will be glad to help, you! TY for a great library.

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