flutter / flutter

Flutter makes it easy and fast to build beautiful apps for mobile and beyond
https://flutter.dev
BSD 3-Clause "New" or "Revised" License
166.18k stars 27.45k forks source link

Add Platform.isWeb #79413

Open johnpryan opened 3 years ago

johnpryan commented 3 years ago

The kIsWeb constant is the only way to check if the app is running on the web. It would be great if we could recommend something like Platform.isWeb (alongside isLinux, isIOS, etc.)

If it's not possible to achieve in the framework due to #39998 we could consider adding it to package:platform

Related issues:

jonahwilliams commented 3 years ago
import 'dart:io';
import 'package:flutter/foundation.dart';

extension PlatformHelpers on Platform {
  static bool isWeb => kIsWeb;
}
jonahwilliams commented 3 years ago

Of course, if you make it not constant it won't tree shake

johnpryan commented 3 years ago

Maybe we could add a similar extension somewhere in the SDK? Maybe in https://github.com/flutter/flutter/blob/master/packages/flutter/lib/src/foundation/platform.dart?

extension PlatformHelpers on Platform {
  static const bool isWeb = kIsWeb;
}
jonahwilliams commented 3 years ago

I think @Hixie and @zanderso have some stronger thoughts than me on extension methods in the SDK.

Hixie commented 3 years ago

https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#avoid-using-extension

But we should totally do this as part of the work for making Platform work on web. That's probably a Dart SDK bug though. cc @tvolkert who may know more about the dart:io/Platform work.

jonahwilliams commented 3 years ago

The Platform class getters are not constants, so using them is strictly worse than constants in cases where you do have platform specific code.

We don't actually have any built in support for identifiers like kIsLinux, but they could be added by the developer via dart defines. Pushing people to use a runtime interface may be a step backwards instead of forwards.

johnpryan commented 3 years ago

I think it's probably easier for users to keep using Platform.is* than switch to kIs* - but if it's faster to use constants in the implementation that sounds good to me.

I'm not 100% what the ideal API looks like, as discussed in the doc related to #39998 there are different scenarios where it's best to use Theme.of().platform, defaultTargertPlatform, and Platform depending on the use-case. For example, if I'm looking to use a specific layout on the web, I should probably use Theme.of().platform instead. It's also possible that users want to know what OS the browser is running on – isLinux and isMacOS might be useful in a browser context.

jonahwilliams commented 3 years ago

but if it's faster to use constants in the implementation that sounds good to me.

Not that it is faster, but that it allows tree-shaking to work.

Hixie commented 3 years ago

We should make Platform.isFoo const anyway.

FWIW, Theme.of().platform and defaultTargetPlatform will never report web, since web is a metaplatform and not one of the target platforms that is relevant for deciding UI behaviour.

dumazy commented 3 years ago

Concerning kIsWeb, I feel like its implementation is a bit of a code smell.

/// This implementation takes advantage of the fact that JavaScript does not
/// support integers. In this environment, Dart's doubles and ints are
/// backed by the same kind of object. Thus a double `0.0` is identical
/// to an integer `0`. This is not true for Dart code running in AOT or on the
/// VM.
const bool kIsWeb = identical(0, 0.0);

Couldn't conditional imports provide a more robust solution? It seems like Platform is already using this approach to provide different implementations between VM and Web.

import '_platform_io.dart'
  if (dart.library.html) '_platform_web.dart' as _platform;