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

Scope.write() without explicit type argument is not compile time safe #8

Open
hcbpassos opened this issue Feb 9, 2021 · 5 comments
Labels
enhancement New feature or request

Comments

@hcbpassos
Copy link

hcbpassos commented Feb 9, 2021

When there's no explicit type argument on Scope.write(), it is compile time valid to pass a state that can't be attributed to ref. Take this example:

final countRef = StateRef(0);

class CounterLogic with Logic {
  ...
  void increment() {
    write(countRef, 'banana'); // compiler: yup, go ahead.
  }
}

This happens because, implicitly, T becomes the least upper bound type of both arguments, which is Object. Such conclusion implies that, more precisely, the problem is not about explicitly defining a type argument, since write<Object>(countRef, 'banana') is also compile time valid. The actual point is the way the compiler fails to spot this particular incorrect construction.

The only way I see to solve this up is by not declaring both state and ref as parameters of the same method. I'd consider something like the following:

abstract class Scope {
  StateRefHandler<T> onStateRef<T>(StateRef<T> ref);
  ...
}

abstract class StateRefHandler<T> {
  void write<T>(T state, [Object action]);
  ...
}

There's a clear complexity increase on both library implementation and use, besides being a breaking change, but I believe it's worth it.

An alternative is discouraging calling Scope.write() without explicit type arguments. Perhaps a summary of this discussion could be placed somewhere at the documentation.

@letsar
Copy link
Owner

letsar commented Feb 12, 2021

Nicely spotted ! Thanks. Yes that's inconvenient, I need to find a syntax which is compile safe without adding a lot of complexity 🤔

@letsar
Copy link
Owner

letsar commented Mar 28, 2021

Hi, I will go with a solution highly inspired by you:

We will have this class responsible for mutating a state:

/// An objet which can mutate a state.
class StateMutator<T> {
  const StateMutator._(this.scope, this.ref);

  /// The scope where the actual state is stored.
  final Scope scope;

  /// The reference to the state.
  final StateRef<T> ref;

  /// {@macro binder.scope.write}
  void write(T state, [Object? action]) {
    scope.write(ref, state, action);
  }

  /// Updates the value of the state referenced by [ref] with a function which
  /// provides the current state.
  ///
  /// An optional [action] can be send to track which method did the update.
  void update(Updater<T> updater, [Object? action]) {
    scope.write(ref, updater(scope.read(ref, null)), action);
  }
}

And I will add a method in the Logic class to get an instance of a mutator.

mixin Logic {
  /// The scope where this logic is stored.
  @visibleForOverriding
  Scope get scope;

  StateMutator<T> withRef<T>(StateRef<T> ref){
    return StateMutator._(scope, ref);
  }
}

Then we would use it like this:

class MyLogic with Logic {
  void increment(){
    withRef(counterRef).update((x) => x++);
  }
}

I'm not sure of the name for the method used in the logic to get the mutator though.
What do you think?

@hcbpassos
Copy link
Author

hcbpassos commented Mar 28, 2021

Very interesting approach. My considerations:

  • There are methods that mutate the state but are not impacted by the described issue. Those are: clear(), undo(), redo(). They will remain inside Logic, right? If so, I wonder whether StateMutator is a good name. After all, the state may be mutated elsewhere as well.
  • This doesn't seem like an absolute solution, since it is still possible to use Scope directly:
final countRef = StateRef(0);

class CounterLogic with Logic {
  ...
  void increment() {
    withRef(countRef).write('banana'); // compiler: you're not fooling me you nasty boi.
    scope.write(countRef, 'banana'); // also compiler: yup, go ahead.
  }
}

About the name of the method to get the mutator, I think withRef() is fine. Initially, I though about onRef() because onRef().write() feels closer to "write on ref". However, now I think "write with ref" makes more sense because you're actually using the ref to write on state.

@letsar
Copy link
Owner

letsar commented Mar 28, 2021

Yes, you're right, Mutator is not a good name. Have you a better idea?

About the write method on scope, yes you're also right. If only we had an internal annotation, or something like that. I will think about something better, thanks!

@hcbpassos
Copy link
Author

hcbpassos commented Mar 28, 2021

Ahh, hard to find a good name for that. I think I'll let you think of something better and, if after that the mutator still exists, then we think of a better name :)

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
None yet
Development

No branches or pull requests

2 participants