Update advice for platform detection

[timsneath]

The general tone of this advice is good, but it unnecessarily complicates things, since it adds a dependency on a third-party package that doesn't have a stable null-safe version, when all that is needed is to flip the order so that Dart can short-circuit execution.

I've also removed the macOS example, because the use of RTL to reorder buttons seems dangerous. I've not tested this, but I suspect this may have other side-effects. For example, this code assumes that the default platform is LTR, so may have opposite results on an Arabic or Hebrew workstation.

[timsneath]

/cc @esDotDev, who I think was the original page author?

[esDotDev]

Hey Tim,
UniversalPlatform
what we found in production is that relying on short-circuit to avoid errors is unnecessarily brittle, as the errors do not manifest until runtime, making it easy for exception to sneak into the code unless you are constantly testing on web (which is slow).

From our experience, it tends to be much nicer/simpler to just remove this sharp edge from the equation and have nice safe API you can call in any order. Which is why we recommended just using the plugin, saves the users some pain that they will almost surely experience if they have to rely on comparison order to prevent RTE's.

I am the original author of UniversalPlatform as well. Despite it being marked as pre-release it's very stable, not really having changed at all in 16mths other than some tiny tweaks for NNBD. I just need to remove the pre-release tag.

Button Order
Regarding the button stuff, I'm fairly sure this is safe code, and Windows and MacOS do not reverse their primary button direction when using RTL language, and we're trying to match that OS-level behavior (whether right or wrong, we just don't want to stand out).

We could run some tests to try and confirm this if you like. I'd be more in favor of adding a caveat to this effect, than removing the discussion entirely, as this is one of the key differences between OS's that will immediately make an app feel weird if it's wrong.

With all that said, happy to proceed as you wish, just wanted to flesh out the rationale behind some of these sections.

[timsneath]

Thanks Shawn! Appreciate the fast feedback.

• So minbar, let's make UniversalPlatform a 1.0. But I think that the problem of relying on a package is slightly borne out by this: as you say, other than null safety migration, the package is 16 months old, so maintenance is always a challenge. Can a customer rely on this being updated? Are you planning to add support for embedded platforms like Tizen?
• Conceptually, I'd prefer that we don't rely on packages where we can easily show what Flutter itself provides. I'd have no problem with a reference to UniversalPlatform, but I'm not sure that we want to document it as the way that Flutter handles this -- it's the fact that this is the official documentation that puts me off.
• I'm not sure why you think short-circuiting is brittle? Dart has worked this way since the very start... Can you help me understand the scenario where the code above is going to suddenly break if you're not testing on web?
• On the button front, imagine a button like Icons(Icon.back). I think your code will show the back button pointing right on macOS and pointing left on Windows/Linux etc. instead of following the local language, based off this. But I may have misunderstood the example -- again, I'm just reading it, not running it.

[esDotDev]

Hey Tim,

• UniversalPlatform is probably my smallest and most popular package, adding support for another platform would only be a single line, so I'm sure I can maintain it easily, but I totally understand your concern here with referencing 3rd party packages at all. fwiw, we reference several more packages as well when it comes to popups and context menus.
• That's fair! Probably enough to just call it out, as a package you can use if you don't want to worry about short-circuting.

fwiw, you can see basically the entire package here, and how simple it really is: https://github.com/gskinnerTeam/flutter-universal-platform/blob/master/lib/universal_platform.dart

Regarding the short circuit is brittle thing:

• It's very easy for a developer to put some code like this in their view:
  leftPadding = Platform.isMacOS? 20 : 0, and this will run perfectly everywhere except for web where it will crash. Developer must constantly remember to do something like if(!kIsWeb && Platform.iOS) all the time, or write their own little set of utility methods as you describe (which gets pretty close to UniversalPlatform)
• Many developers aren't aware of the short circuit thing at all. So it's extra cognitive overhead to have to explain that to each member of the team and have them remember it. It's also a little confusing to read, because it's making the check more complex than it should be, just to avoid a runtime error (ie, a future overly-clever developer may be tempted to remove this seemingly pointless kIsWeb check, and not notice that they just injected a bug on 1 platform)
• The risk/reward is hard to square here, because the crash is so hard, and the fix so easy, we will basically not allow developers on our team to rely on short-circuiting unless they encapsulate it into a shared method.

Maybe best approach is to just recommend a little static class/package, with methods similar to what you have there, and then recommend developers use a utility class rather than reference Platform directly? But flesh it out with all the API you may want to use, in a safe way:

bool get isMacOS => !kIsWeb && Platform.isMacOS;
bool get isWindows => !kIsWeb && Platform.isWindows;
bool get isLinux => !kIsWeb && Platform.isLinux;
bool get isAndroid => !kIsWeb && Platform.isAndroid;
bool get isIOS => !kIsWeb && Platform.isIOS;

bool get isMobileDevice => isIOS || isAndroid;
bool get isDesktopDevice => !isMobileDevice;
bool get isMobileDeviceOrWeb => kIsWeb || isMobileDevice;
bool get isDesktopDeviceOrWeb => kIsWeb || isDesktopDevice;

The nice thing about a package here though, is it really is pretty annoying to have to copy and paste these little snippet classes from project to project. Ideally the "best" approach would be to remove this rough edge from the SDK somehow, but not sure if there are any plans for that: flutter/flutter#36126

• Let me look into the button content thing to make sure this technique is safe. Otherwise I'll try and come up with some other way to change the direction the retains the internal structure of each btn.

[esDotDev]

I should confess, when I wrote this lib 16mths ago, I had no idea that short-circuiting like this would work, so I wrote it with conditional imports, which I guess I didn't need at all :p

[timsneath]

I really like your idea -- include a slightly broader list of properties that folk can cut and paste directly into the code, and then reference a package that adds a nice bow on the top for those who just want to rely on a module. That way we both educate people (which fundamentally is the point of documentation) as well as giving them practical options.

Thanks for checking on the button thing!

[sfshaza2]

LGTM