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

Add Platform.isWeb #79413

Open
johnpryan opened this issue Mar 30, 2021 · 10 comments
Open

Add Platform.isWeb #79413

johnpryan opened this issue Mar 30, 2021 · 10 comments
Labels
c: new feature Nothing broken; request for a new capability c: proposal A detailed proposal for a change to Flutter framework flutter/packages/flutter repository. See also f: labels. P3 Issues that are less important to the Flutter project platform-web Web applications specifically team-web Owned by Web platform team triaged-web Triaged by Web platform team

Comments

@johnpryan
Copy link
Contributor

johnpryan commented Mar 30, 2021

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
Copy link
Member

import 'dart:io';
import 'package:flutter/foundation.dart';

extension PlatformHelpers on Platform {
  static bool isWeb => kIsWeb;
}

@jonahwilliams
Copy link
Member

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

@darshankawar darshankawar added in triage Presently being triaged by the triage team framework flutter/packages/flutter repository. See also f: labels. platform-web Web applications specifically c: proposal A detailed proposal for a change to Flutter passed first triage c: new feature Nothing broken; request for a new capability and removed in triage Presently being triaged by the triage team labels Mar 31, 2021
@johnpryan
Copy link
Contributor Author

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
Copy link
Member

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

@Hixie
Copy link
Contributor

Hixie commented Mar 31, 2021

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
Copy link
Member

jonahwilliams commented Mar 31, 2021

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
Copy link
Contributor Author

johnpryan commented Mar 31, 2021

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
Copy link
Member

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
Copy link
Contributor

Hixie commented Mar 31, 2021

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.

@yjbanov yjbanov added the P3 Issues that are less important to the Flutter project label Apr 1, 2021
@dumazy
Copy link
Contributor

dumazy commented Apr 7, 2021

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;

@flutter-triage-bot flutter-triage-bot bot added multiteam-retriage-candidate team-web Owned by Web platform team triaged-web Triaged by Web platform team labels Jul 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: new feature Nothing broken; request for a new capability c: proposal A detailed proposal for a change to Flutter framework flutter/packages/flutter repository. See also f: labels. P3 Issues that are less important to the Flutter project platform-web Web applications specifically team-web Owned by Web platform team triaged-web Triaged by Web platform team
Projects
None yet
Development

No branches or pull requests

6 participants