dart-lang / language

Design of the Dart language
Other
2.65k stars 201 forks source link

Platform (iOS, Android, etc.) should be a useable condition for conditional imports #2133

Open brianquinlan opened 2 years ago

brianquinlan commented 2 years ago

I have multiple platform-specific implementations of an HTTP client and I would like to select the appropriate one for the platform e.g. to use one based on NSURLSession on iOS, cronet on Android, etc.

In C++, I would use preprocessor macros to solve this issue e.g.

HttpClient *client;
#ifdef IOS
#include <whatever.h>
client = new NSURLAdapter();
#elif ...

In Python, I would do the configuration at runtime e.g.

import platform

client: HttpClient
if platform.system() == "Windows":
  import whatever
  client = whatever.WindowsClient()
elif  ...

In Dart, I don't know how to solve this problem except in the narrow case of differentiating between web and non-web (using conditional imports).

Levi-Lesches commented 2 years ago

See the Platform class from dart:io. You could do:

import "dart:io";

class Client { }
class WindowsClient extends Client { }
class MacClient extends Client { } 
class AndroidClient extends Client { }
class IPhoneClient extends Client { }

Client getClient() => 
  Platform.isWindows ? WindowsClient() : 
  Platform.isMacOS ? MacClient() : 
  Platform.isAndroid ? AndroidClient() : 
  Platform.isIOS ? IPhoneClient() : 
  throw "Unrecognized platform";
leafpetersen commented 2 years ago

The language leaves it deliberately implementation defined what constants are defined for use in conditional imports, IIRC. The limiting factor is largely around the ability of the build systems to handle multiple configurations efficiently. This issue documents the discussion and eventual resolution. cc @jakemac53 @natebosch who might have more comments here.

lrhn commented 2 years ago

We could introduce more available environment constants, and when some build-systems or environments can't support them, we could introduce lints to disable their use. Then people with concrete needs and no special build-system requirements can make their code more precisely platform specific.

It does risk that some libraries will be incompatible with some build-systems, which can fragment the ecosystem. Not doing so risks reducing everybody to the lowest common denominator.

(I always wanted to know at compile time whether I have web numbers or 64-bit integers!)

jakemac53 commented 2 years ago

Fwiw, the build systems can in theory handle arbitrary constants as long as they either:

Concretely this matters because build_runner and bazel may be compiling multiple apps in a single build, and they share kernel output files (per platform & null safety mode). So they have to agree on all constants (for a given platform & mode).

Both of these systems however allow passing arbitrary arguments to backend compilers, and people may try to pass -D arguments that way, so it could be confusing, but we could put in some special handling to alert them that it won't work.

jakemac53 commented 2 years ago

It is also worth mentioning though that the cost of supporting this might be prohibitively high. We would need to resolve constants in order to discover which imports to select. Using the analyzer to do this is actually challenging because the analyzer has to be ran in the same mode that is being compiled for, which I don't think it supports well today. And it would be costly to evaluate the constants as well.

Levi-Lesches commented 2 years ago

Can someone clarify whether the Platform class is an adequate solution? When you can import dart:io, you can use it to know what platform you're on. And when you can't, you know you're on web.

jakemac53 commented 2 years ago

Can someone clarify whether the Platform class is an adequate solution? When you can import dart:io, you can use it to know what platform you're on. And when you can't, you know you're on web.

The problem is if you want to actually change the import based on the Platform constants. Today you could change the runtime behavior, but not the actual imports, because Platform isn't even available outside of dart:io.

jakemac53 commented 2 years ago

Fwiw as far as platform constants specifically go, it would require the build systems to compile separately for each platform, which they don't have to do for all platforms today (only "targets" which doesn't map 1:1 with platforms).

That would be feasible in theory but it would also make kernel files not be portable across platforms which is not desirable imo.

jakemac53 commented 2 years ago

Actually, these Platform variables are not even constant for this same reason (its only known at runtime, depending on the current OS). So I don't see these Platform variables being something you can use in conditional imports regardless.

brianquinlan commented 2 years ago

Having conditional use cases based on Platform would be sufficient for my use case.

The solution suggested in https://github.com/dart-lang/language/issues/2133#issuecomment-1054923740 works (and what I'm currently using) but, since Platform.is* are all non-const, the code for every platform will end up in standalone binaries.

In my mind, the cleanest solution is more robust conditional imports and conditional dependencies in pubspec.yaml.

FWIW, conditional dependencies in Python look like this:

install_requires:
    pywin32 >= 1.0;platform_system=='Windows'
filiph commented 3 weeks ago

To summarize, I think this issue suggests something like this:

export 'src/hw_none.dart' // Stub implementation
    if (dart.os.windows) 'src/hw_windows.dart' // Windows implementation
    if (dart.os.macos) 'src/hw_macos.dart'; // macOS implementation

Am I right?

I, for one, would find this helpful. A related issue suggests something very similar: https://github.com/dart-lang/sdk/issues/49379.

My question to @jakemac53 is whether this is something that is covered by static metaprogramming?

brianquinlan commented 3 weeks ago

To summarize, I think this issue suggests something like this:

export 'src/hw_none.dart' // Stub implementation
    if (dart.os.windows) 'src/hw_windows.dart' // Windows implementation
    if (dart.os.macos) 'src/hw_macos.dart'; // macOS implementation

Am I right?

That approach would work!

In general, I don't like the conditional import approach because it forces you to split your logic into separate source files, even when the logic is trivial.

lrhn commented 3 weeks ago

With part files with imports, you should get conditional part directives, and with augmentations you should be able to override only that you need to override in those parts. Together that should avoid needing extra libraries or duplicating logic, you can change only the small private part of a library that is conditionally supported.

filiph commented 2 weeks ago

With part files with imports, you should get conditional part directives, and with augmentations you should be able to override only that you need to override in those parts. Together that should avoid needing extra libraries or duplicating logic, you can change only the small private part of a library that is conditionally supported.

I'm not sure I follow. Are part files with imports and augmentations parts of a spec? If there's documentation for these features, just give me a link and I can study these on my own. Maybe I already know these features, just not under those names?

lrhn commented 2 weeks ago

Part files with imports, aka "enhanced parts" and augmentations are both spin-off features from the macro feature. The current feature proposals are in https://github.com/dart-lang/language/tree/main/working/augmentation-libraries (They're both in the same directory because enhanced parts was split out from the augmentation feature, but it is now defined as an orthogonal feature.)