Some widgets' nullOk parameters are inconsistently applied.

[shyndman]

The Actions widget has a number of static methods with a nullOk parameter, which can be used to enable or disable the throwing of errors when the caller's requested object was not found.

Turns out, this parameter is only considered when asserts are enabled. This feels a little iffy to me, as the documentation makes no mention of how the contract can change depending on environment.

Request: Improve docs (and maybe the name of the param, since it's sort of a debug only thing), or alternatively normalize behavior between asserts-enabled and disabled builds.

Affected methods:

• Actions.find
• Actions.of
• Actions.handler
• Actions.invoke

Thanks!

@gspencergoog

[gspencergoog]

The name of the parameter is consistent across many different similar static methods, not just Actions, and has the same behavior there (e.g. MediaQuery.of, Focus.of, Localizations.localeOf, etc.). And yes, it only applies to builds with assertions on, but that was always its intent.

I agree that the behavior should be mentioned more explicitly in the docs.

[shyndman]

I took a look at all of those, and it appears they are designed to throw regardless of whether asserts are enabled (although Localizations.localeOf might throw an NPE as opposed to a well-formed FlutterError).

[gspencergoog]

Ahh, I see what you mean. OK, I can see making those throw a FlutterError on a non-assert build.

We seem to actually be quite inconsistent with our methods that take nullOk as a parameter, perhaps we should try and normalize that across the framework.

[shyndman]

Couldn't hurt. :)

Thanks!

[gspencergoog]

I'm converting this issue over to cover all the static methods that take a nullOk parameter.

We have a bunch of static member functions that have a nullOk argument to them, but we're really inconsistent with how we apply it. Sometimes it will return null in some circumstances even if nullOk is false (e.g. CupertinoDynamicColor.resolve()), sometimes it only works when asserts are enabled (e.g. Actions.find), and other times it throws a FlutterError in release mode (e.g. MediaQuery.of()). Sometimes I think the checks are assert-only because of performance, though, so we can't just make them all throw FlutterError all the time. At the least, we should probably update the API docs for each one that describes what behavior is expected.

[shyndman]

Thanks Greg.

When the time comes, I'd love to be CC'd on that PR if you get a chance.

[gspencergoog]

Sure, will do.

[gspencergoog]

Related: #67163

[gspencergoog]

Here's a design doc for doing this: https://flutter.dev/go/eliminating-nullok-paramters (I know, I'm fixing the name typo in the go link).