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

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

Open
brianquinlan opened this issue Mar 1, 2022 · 15 comments
Labels
request Requests to resolve a particular developer problem

Comments

@brianquinlan
Copy link

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).

@brianquinlan brianquinlan added the request Requests to resolve a particular developer problem label Mar 1, 2022
@Levi-Lesches
Copy link

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

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

lrhn commented Mar 1, 2022

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

jakemac53 commented Mar 1, 2022

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

  • are defined globally (via a --define flag or similar global config for the entire build)
  • are tied to a specific platform (web, vm, etc)
  • are tied to sound or unsound modes

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

jakemac53 commented Mar 1, 2022

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

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

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

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

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

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

The solution suggested in #2133 (comment) 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
Copy link

filiph commented Aug 27, 2024

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: dart-lang/sdk#49379.

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

@brianquinlan
Copy link
Author

brianquinlan commented Aug 27, 2024

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

lrhn commented Aug 27, 2024

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

filiph commented Sep 2, 2024

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

lrhn commented Sep 2, 2024

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.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
request Requests to resolve a particular developer problem
Projects
None yet
Development

No branches or pull requests

6 participants