search

GregoryConrad / rearch-dart

Re-imagined approach to application design and architecture
https://pub.dev/packages/rearch
MIT License
88 stars 4 forks source link

[DOC CHANGE PROPOSAL] #32

Closed busslina closed 9 months ago

busslina commented 9 months ago

https://github.com/GregoryConrad/rearch-dart/blob/b39cd673c06c932208c8d620db2eadd36c282300/packages/rearch/lib/src/side_effects.dart#L50C34-L50C34

Maybe specify that it keeps the same getter across rebuilds too

GregoryConrad commented 9 months ago

That was intentional: we want use.state itself to return the same state setter across rebuilds so that when an == check is done on the output of a capsule, a new state setter would not cause dependents to unnecessarily rebuild (a pretty niche little optimization). The getter itself does't matter, as it will be invoked to get the value.

However, this issue made me realize a potential problem with the current state getter/setters (so thanks for opening!). Take this failing test I just wrote to confirm my thinking:

  test('stateGetterSetter dependents should rebuild when state updated', () {
    (int Function(), void Function(int)) stateCapsule(CapsuleHandle use) =>
        use.stateGetterSetter(0);
    int currStateCapsule(CapsuleHandle use) => use(stateCapsule).$1();

    final container = useContainer();
    expect(container.read(currStateCapsule), equals(0));
    container.read(stateCapsule).$2(1);
    expect(container.read(currStateCapsule), equals(1)); // --------- fails here, still 0
  });

Because the same getter is returned, the == optimization is a little too optimistic and actually doesn't rebuild the dependent capsule! I don't know whether or not to classify this as a "bug" per se, but this is likely some unintuitive behavior. Will put a PR up to fix that issue and the code comments you referenced in a second.

busslina commented 9 months ago

So, for clarify myself. Do you mean that even if you call rebuild() after checking the new value is not the same that the current value, it will not rebuild because you are doing an equality check between former and new Side Effect return value (getter and setter) which are the same. So no updating?

busslina commented 9 months ago

I don't know whether or not to classify this as a "bug".

I thing we can call it a bug because your intention is to rebuild but it wouldn't ever rebuild

GregoryConrad commented 9 months ago

Yea. Maybe not a bug, but unintuitive behavior which is why I pushed that fix up yesterday.

rebuild() only requests for the associated capsule to be rebuilt. If that capsule’s data doesn’t change after it is rebuilt, none of the dependents/downstream capsules will be modified. Thus, we return a new getter on each build to force the new data to be different from the old data so dependents will rebuild.

GregoryConrad commented 9 months ago

 Yea. Maybe not a bug, but unintuitive behavior which is why I pushed that fix up yesterday.

rebuild() only requests for the associated capsule to be rebuilt. If that capsule’s data doesn’t change after it is rebuilt, none of the dependents/downstream capsules will be modified. Thus, we return a new getter on each build to force the new data to be different from the old data so dependents will rebuild.