Remove dart:js from dart2wasm

[srujzs]

This library was added to support allowInterop. All of the methods throw and importing dart:js returns a static error. However, doing conditional imports e.g. if (dart.library.js) succeeds in dart2wasm, because the library still exists in libraries.json. Since allowInterop is now no longer defined in dart:js, we should just remove this library altogether from the platform.

[mkustermann]

/cc @osa1 Could we disallow importing dart:js just like we (after cl/368343) disallow importing dart:ffi (until we remove dart:js entirely)?

[srujzs]

There's already a static error we've added to disallow importing dart:js in dart2wasm:

sdk/pkg/_js_interop_checks/lib/js_interop_checks.dart

Line 541 in 2250404

if (_disallowedInteropLibrariesInDart2Wasm

[osa1]

Do we need to make any changes in Flutter for this? I've created an SDK CL to remove dart:js from the platform: https://dart-review.googlesource.com/c/sdk/+/368642.

[mkustermann]

Do we need to make any changes in Flutter for this? I've created an SDK CL to remove dart:js from the platform: https://dart-review.googlesource.com/c/sdk/+/368642.

Maybe this issue also includes dart:js_util (which we currently use internally in the implementation of dart:js_interop)

[srujzs]

I don't think Flutter uses dart:js anywhere. I'm not sure why your CL results in package:js complaining about exporting dart:_js_annotations now. That should be tangential but I'll have to take a deeper look.

Maybe this issue also includes dart:js_util (which we currently use internally in the implementation of dart:js_interop)

There are a few migrations that need to happen in Flutter and elsewhere (as well as migrating our own implementation of dart:js_interop like you note) before package:js and dart:js_util can be removed from dart2wasm. I'd expect it to be harder than removing dart:js.

[sigmundch]

FYI - we run again into this issue. I've created https://dart-review.googlesource.com/c/sdk/+/380583 to attempt again. It's very similar to the change @osa1 created a while back, but I reduced the scope a bit to see if we can make it easier to land.

Do you recall @osa1 what issues you ran into (aside from the bot failures that the CL shows?)

Alternatively, we could use the "supports: false" line in the libraries.json to start incrementally (I'll prepare a change to do so for dart:js_util, since we still have cases where we load code from it).

[osa1]

@sigmundch I don't remember what the problem was. I think I was busy with other things and didn't want to spent too much time on this at the time and stopped after seeing the test failures.

[srujzs]

(I'll prepare a change to do so for dart:js_util, since we still have cases where we load code from it)

Just a heads up: we still have allowlisted code (the engine and package:test for example) that imports some members from dart:js_util running in dart2wasm that need to be migrated before we can remove that.

[simolus3]

Some Dart programs rely on detecting the compiler in conditional imports (to e.g. switch implementations depending on whether we have 64bit ints).
Dropping dart:js support from dart2wasm gives us a way to uniquely identify DDC and dart2js by checking for dart.library.js, which is great.

However, the 3.5 release announcement also described plans to eventually discontinue all of the legacy SDK web bindings (including dart:js) in favor of package:web and dart:js_interop. I'm worried that this will bring back the problem of not being able to identify dart2wasm in conditional imports - will this be considered or is there a proper way to set up conditional imports now that avoids this future problem?

[mkustermann]

Some Dart programs rely on detecting the compiler in conditional imports (to e.g. switch implementations depending on whether we have 64bit ints).
Dropping dart:js support from dart2wasm gives us a way to uniquely identify DDC and dart2js by checking for dart.library.js, which is great.

However, the 3.5 release announcement also described plans to eventually discontinue all of the legacy SDK web bindings (including dart:js) in favor of package:web and dart:js_interop. I'm worried that this will bring back the problem of not being able to identify dart2wasm in conditional imports - will this be considered or is there a proper way to set up conditional imports now that avoids this future problem?

/cc @dart-lang/language-team Maybe it's time to allow more than dart.library.* in conditional imports ( #54785)?

[sigmundch]

Good observation @simolus3, indeed!

@mkustermann - I agree, I made a similar suggestion in dart-lang/crypto#180 (comment) 😄 - We need to start using tests that directly correspond to what developers want to know. For example, if you want to condition on a platform, then use a variable that describes the platform, like dart.tool.dart2wasm, rather than an indirect observation, like whether a library is available.

[natebosch]

IIRC the bazel support is implemented as a fixed list of platforms and what variables they correspond to, so adding variables like dart.tool.dart2wasm might be aligned. Whatever we do with configurable imports we should plan to get input about the bazel implementation.