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.3k stars 1.59k forks source link

web compilers should not allow imports to 'dart:io' #46917

Open mraleph opened 3 years ago

mraleph commented 3 years ago

DDC prohibits such import outright, but dart2js allows it to a certain extent.

In grpc package we have import 'dart:io' show HttpStatus; and it works in dart2js builds just fine (which means it passes all tests - which are run through dart2js), but breaks when people try to run the code through webdev serve.

lrhn commented 3 years ago

It's a mistake that dart2js allows the import at all. The dart:io library is not available on the web, and pretending that it is just leads to confusion. If it's actually useful, it leads to breaks when we fix that bug. We have configurable imports, so there should not be any need to import dart:io unconditionally on the web.

If you need HttpStatus on the web, it can be imported through dart:html.

sigmundch commented 3 years ago

Some context and clarifications:

I agree that the inconsistent experience between dart2js on the command line or web compilers via webdev is not great, though. I'm with Lasse here - we should make the breaking change and disallow the imports on the web compilers again.

mraleph commented 3 years ago

I want to add one more data point here: this discrepancy got our grpc package in a very weird place. And making this breaking change is going to make it even weirder. We have platform specific bits pulled into platform independent bits through shared code generated by the protoc compiler. For example, there is a class called ServiceCall - recently I have merged a PR which exposed (on server side) client certificate of the underlying connection through a getter on this class X509Certificate? get clientCertificate. This change has passed all the tests but then it turned out that it broke webdev compilation for gRPC Web because it added a dart:io import into a code which theoretically should never be referenced on the client side - but is pulled through generated code.

Now when I started to look at disentangling this mess, I discovered that we run a most of our tests on native and web platforms - but this only works because dart2js happily compiles dart:io imports... So when breaking change comes we would have to go through the whole package and test corpus and disentangle truly platform independent parts and tests from platform dependent parts... It's going to be a lot of work.