Nullability support

[AlexBacich]

Hey, do you have in plans nullability support for the library? I can take care about nullability support for Rx_widgets.

[escamoteur]

yep will come, I only have to have time :-)
get_it and functional_listener are already migrated

[mk-dev-1]

Morning everyone,

is this coming anytime soon? This library is currently the only one that is left to update our production code.
I've had a go at it myself, but as there are a number of ways to migrate the package, I don't really want to interfere with the intentions of the maintainers...

Thanks for your support!

[AlexBacich]

@mk-dev-1 , do you have migrated version? I've run into issues while trying to migrate.
Problematic piece - inputStream.materialize().

@escamoteur , hopefully you 'll have time to look into this. As been said, I'll take care of rx_widgets.

[escamoteur]

@josh-ksr have you already done something on this side?

[escamoteur]

@mk-dev-1 could you make a PR, so that we could have a look at it?

[mk-dev-1]

@escamoteur: Please have a look at #49. I am struggling with an issue, but I am sure it can be resolved rather easily ;-)

[AlexBacich]

@escamoteur , would you have time to work on it any time soon? This seems too challenging to me to migrate myself, and this issue is blocking for full project migration to null-safety.

[escamoteur]

Will try to do it over the weekend

[escamoteur]

ok the weekend wasn't long enough. I'm thinking of making the API exactly (almost) the same as that of flutter_command. This would be a breaking change, though nothing that isn't quickly fixable. What are your thoughts?

[AlexBacich]

The whole null-safety is breaking change, so I think now it's the perfect time to introduce it. All logic will be re-tested anyway.

[escamoteur]

just pushed first version 6.0.0-null-safety

https://github.com/fluttercommunity/rx_command/tree/null-safety

please have a close look and tell me what you think!

[mk-dev-1]

Thank you so much for your work, much appreciated!!

I can see that initialLastResult has now become a required field as it is non-nullable. Previously I have not been needing any initial results, therefore I now have to add this for every RxCommand that I set up, which seems a bit verbose. From what I can see it is safe to put in a dummy instance of TResult as long as I don't use emitInitialCommandResult, right?

In any case, maybe there is a way to avoid making initialLastResult mandatory?

[escamoteur]

Totally right. This came because for flutter_command it's mandatory as every ValueNotifier needs to have an initial value
Fixed with
6.0.0-null-safety.1

What would be absolutely awesome if someone could update the readme to the changed API, namely:

• thrownExceptions now emits CommandErrors
• CommandResult now contains the param value
• renaming of canExecute to constraint
  also check all source snippets
  and if I forgot something else :-)

That would be a great help

[mk-dev-1]

Here's another observation / question:

  static RxCommand<TParam, void> createAsyncNoResult<TParam>(
      AsyncAction1<TParam> action,
      {Stream<bool>? restriction,
      bool emitInitialCommandResult = false,
      bool emitsLastValueToNewSubscriptions = false,
      String? debugName}) {

where AsyncAction1 is defined as

typedef AsyncAction1<TParam> = Future Function(TParam? param);

Should it not be this?

typedef AsyncAction1<TParam> = Future Function(TParam param);

After all, action from above is passed to RxCommandAsync's _func, which is defined as

final Future<TResult> Function(TParam)? _func;

... same question for these two:

typedef Action1<TParam> = void Function(TParam? param);
typedef AsyncFunc1<TParam, TResult> = Future<TResult> Function(TParam? param);

Generally, we would not expect a null value to be returned from a command that is set up like RxCommand.createSync<bool, bool>(_func), or am I missing something?

Thanks for your feedback!

Edit: Looking at the API of flutter_command, this should be corrected to mirror the API of flutter_command more closely...

[escamoteur]

you are probably right

[escamoteur]

could you make a PR and try if it works? I'm kind a busy at the moment

[5eeman]

Hi,
Seems like something broken between migration to null safety. Here is code snippet.
var stringCommand = RxCommand.createAsync<String, String>((param) async => 'Here is your string - $param'); stringCommand.listen((value) { print('Value is not empty ${value.isNotEmpty}'); });
In this case value may be nullable for some reason, while params are non null.

[mk-dev-1]

@5eeman This issue has already been identified and is addressed in PR #53

[mk-dev-1]

Everything's looking good as far as I can see.

What we are left with is this "issue" here:

Hi all,

I have put together an initial PR for migrating the package to null-safety. However, I can't seem to find a way to migrate the package in a way that this

late RxCommand<bool, bool> myCommand;

[...]

RxCommand.createSync<bool, bool>(_myFunc)

[...]

bool void _myFunc(bool param) {
return bool;
}

[...]

myCommand();
myCommand.execute();

You would expect myCommand() to give you an error, because TParam is non-nullable, but it doesn't as execute and call are set up with a nullable TParam:

  /// Calls the wrapped handler function with an option input parameter
  void execute([TParam? param]);

  /// This makes RxCommand a callable class, so instead of `myCommand.execute()` you can write `myCommand()`
  void call([TParam? param]) => execute(param);

I've played around with this, but it seems there is no way to make TParam non-nullable and optional at the same time. So we either have to live with this or will have to go for explicitly calling myCommand(null) in cases where TParam is nullable.

I am okay to live with this. Any thoughts?

[escamoteur]

Actually I would call this not a problem but a feature. It took me quite some work to get it work like this so that you don't have to pass null explicitly.
IMHO I prefer that I can still pass the command directly to event handlers in that which is even if this means they might get an exception when I don't pass a parameter for a non nullable.
At least as a design decision that I took for flutter_command

[escamoteur]

Thanks a lot for all your help. Migrating a complex package like rx_command isn't easy without people testing and giving feedback!!!

[mk-dev-1]

Thanks for the feedback. If it is a design decision that is okay, of course. I just thought I'd ask about it ;-)

[escamoteur]

I think we leave the package another week in prerelease mode to see if we get any complaints

[mk-dev-1]

I've successfully tested the null-safety release in our app's beta program without experiencing any issues. I'd suggest to go ahead with releasing...

[escamoteur]

If we don't get any comments till monday I push a new release version

[AlexBacich]

@escamoteur , guess we don't have any comments besides updating docs PR. Please push new version. I'll use it for RxWidgets as well.

[escamoteur]

Just pushed V6.0.0