[dart2js] Bundles targeting browser pay cost of Uri Windows handling for Node

[parlough]

Using any of Uri.directory, Uri.file, and uri.toFilePath without explicitly specifying windows: false every time results in all of the Uri code for handling Windows file paths being included due to special handling added for Node in 5a74d8a. This technically also results in an extra check that will always evaluate to false.

Usages of these constructors and functions are often indirect, such as through other packages, making this quite hard to avoid.

I feel this cost is likely not worth it for handling the node case, and my initial feeling is that it would actually be quite surprising for my Dart compiled to JS to behave differently on Node than in the browser. When I compile to JS (or Wasm) I
personally expect I'm opting in to consistent behavior in respect to my code no matter the platform.

Anyone requiring platform specific Uri file behavior on Node (or any other non-browser JS runtime) should probably explicitly give a value to the windows parameter or use Node's https://nodejs.org/api/url.html#urlpathtofileurlpath and https://nodejs.org/api/url.html#urlfileurltopathurl functions.

[sigmundch]

Thanks @parlough!

To make sure I understand, sounds like your goal is to not pay for the cost of the isWindows logic if possible, at least not by default, correct?

I'm wondering if there is still a way to achieve that through code tree-shaking. One idea here is that we could add an extra, but constant condition to _isWindows, so we can more easily tree-shake the code when we know it's not needed.

Today, you have to run dart2js with --server-mode in order to use the output in Node. In server-mode, dart2js drops support for dart:html, so const bool.fromEnvironment('dart.library.html') will be false (while it's true otherwise). So if we update _isWindowsCached to:

 static final bool _isWindowsCached = 
      !const bool.fromEnvironment('dart.library.html') && JS(
      'bool',
      'typeof process != "undefined" && '
      'Object.prototype.toString.call(process) == "[object process]" && '
      'process.platform == "win32"');

Then dart2js will recognize that _isWindowsCached is always false when compiling regular web programs.

In the future, I'd prefer to not expose this via dart.library.html, but use something else that is more directly representing the compiler flags. That's something @fishythefish and I have discussed a few times in the context of the branch pruning transformation.

[parlough]

Thanks for the ideas and clarifying questions! :D

To make sure I understand, sounds like your goal is to not pay for the cost of the isWindows logic if possible, at least not by default, correct?

Yes, that's correct. We now recommend package:http rather than the dart:io and dart:html http APIs, but through a chain of usages, this ends up increasing bundle size significantly even for simple usages (dart-lang/http#1097). Uri is a not insignificant portion of that, partially from the extra Windows functionality which ends up unused in the browser. While the ideal solution is #29420, enabling tree shaking of the Windows logic would be a good win in the mean time 😄.

Today, you have to run dart2js with --server-mode in order to use the output in Node. In server-mode, dart2js drops support for dart:html, so const bool.fromEnvironment('dart.library.html') will be false (while it's true otherwise). So if we update _isWindowsCached to:

If that solution is acceptable to the web compilers team, I'm all for that :). I didn't consider it here as I wasn't sure how that will work after dart:html is eventually dropped since dart:js_interop is agnostic of targeting browsers.

I will note that I think the current custom handling is potentially confusing though, as it isn't documented, and only accounts for Node, so behavior could be different on other runtimes (Deno, Bun, etc.) that don't have similar process/platform APIs.

...I think this leads in to a separate/larger discussion of how the web compilers (especially dart2wasm) should be considered/represented in the new package:platform work (lrhn/platform.dart#4). Running JS outside the browser is extremely common now and Wasm likely even more so, so a coherent story there would be great, and perhaps would allow consistent decisions for the behavior of Dart's core libraries when using these compilers, and how/when they differ from the browser behavior.