dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.28k stars 1.58k forks source link

Bug: Conditional Exports Do Not Detect Invalid Arguments #59575

Closed mcmah309 closed 1 day ago

mcmah309 commented 1 day ago

Conditional exports do not detect invalid if(..) statements. I created an example repo to show this:

https://github.com/mcmah309/conditional_export_issue

Further:

If you replace the export in this file with either of the other commented out conditional exports, then the test will pass only on one platform instead of failing on both -

Original Failed CI Run: https://github.com/mcmah309/conditional_export_issue/actions/runs/11942744134 Linux Export First: https://github.com/mcmah309/conditional_export_issue/actions/runs/11942753955 Windows Export First: https://github.com/mcmah309/conditional_export_issue/actions/runs/11942761382

The expected behavior is to fail at compilation time for invalid if(..) statements. And hopefully allow platform compilation, like what is being attempted.

dart-github-bot commented 1 day ago

Summary: Conditional exports in Dart fail when using multiple if statements for platform checks. A provided example demonstrates the issue and its failure in CI.

lrhn commented 1 day ago

This looks like it's working as intended.

You can write any sequence of dot-separated identifiers as the condition. It's not a Dart expression that is evaluated, it's a key into the compilation environment, so the export:

export 'src/stub.dart' 
  if (Platform.isLinux) 'src/linux.dart'
  if (Platform.isWindows) 'src/windows.dart';

will use src/linux if const String.fromEnvironment("Platform.isLinux") == "true". Which it isn't. And "Platform.isWindows" is also not defined in the compilation environment, so it chooses neither of the conditional URIs.

The conditions are not invalid, they're just not configured.

The conditional imports currently only work with keys of the form dart.library.name, where dart.library.name is set to "true" in the complation environment if dart:name is an available platform library.

There are no available and usable compilation environment keys that expose the target platform. (Maybe there should be, see #54785.)

mcmah309 commented 20 hours ago

Thanks for the explanation. Then maybe this issue should have been turned into a documentation issue rather than being closed. Since I can't find any documentation related to this issue. The only documentation related to conditional imports I can find is here https://dart.dev/guides/libraries/create-packages#conditionally-importing-and-exporting-library-files and it does not go into any detail on what is going on or options available. It just gives a single example.

mcmah309 commented 20 hours ago

will use src/linux if const String.fromEnvironment("Platform.isLinux") == "true"

The conditional imports currently only work with keys of the form dart.library.name, where dart.library.name is set to "true" in the complation environment if dart:name is an available platform library.

Taking these statements, is it possible to pass keys to the compilation environment?

flutter run --dart-define Platform.isLinux=true -d linux

Does not seem to have any effect.

mcmah309 commented 19 hours ago

A created a PR to improve the docs

https://github.com/dart-lang/site-www/pull/6228