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

Lack of documentation for migration from js to js_interop #55352

Open felix-ht opened 8 months ago

felix-ht commented 8 months ago

With the recent beta release of wasm for flutter i wanted to try compling my application to the new target. As (somewhat) expected i ran into some js interop issues.

I tried to switch to the new package import 'dart:js_interop'; but struggled to get it working as there seems to be very litte documentation on how to do the migration.

Old code

@JS('mapboxgl')
library mapboxgl.interop.ui.control.navigation_control;

import 'package:js/js.dart';
import 'package:mapbox_gl_dart/src/interop/interop.dart';

@JS()
@anonymous
class NavigationControlOptionsJsImpl {
  external bool get showCompass;
  external bool get showZoom;
  external bool get visualizePitch;

  external factory NavigationControlOptionsJsImpl({
    bool? showCompass,
    bool? showZoom,
    bool? visualizePitch,
  });
}

@JS('NavigationControl')
class NavigationControlJsImpl {
  external NavigationControlOptionsJsImpl get options;

  external factory NavigationControlJsImpl(
      NavigationControlOptionsJsImpl options);

  external onAdd(MapboxMapJsImpl map);

  external onRemove();
}

Diff

diff --git a/lib/src/interop/ui/control/navigation_control_interop.dart b/lib/src/interop/ui/control/navigation_control_interop.dart
index 985a203..0257ce2 100644
--- a/lib/src/interop/ui/control/navigation_control_interop.dart
+++ b/lib/src/interop/ui/control/navigation_control_interop.dart
@@ -1,7 +1,7 @@
 @JS('mapboxgl')
 library mapboxgl.interop.ui.control.navigation_control;

-import 'package:js/js.dart';
+import 'dart:js_interop';
 import 'package:mapbox_gl_dart/src/interop/interop.dart';

 @JS()

The error i am getting is Error: The '@JS' annotation from 'dart:js_interop' can only be used for static interop, either through extension types or '@staticInterop' classes.

I found some documentation in https://dart.dev/interop/js-interop/usage

e.g.

import 'dart:js_interop';

@JS('Date')
extension type JSDate._(JSObject _) implements JSObject {
  external JSDate();

  external static int now();
}

Before converting all the code in my dart js warper i would like to be sure that this is the correct approach. It would be great if you could also provide some concrete side by side examples of old js vs new js_interop code in the documentation.

Config

Flutter 3.21.0-1.0.pre.2 • channel beta • https://github.com/flutter/flutter.git
Framework • revision c398442c35 (3 weeks ago) • 2024-03-12 22:26:24 -0700
Engine • revision 0d4f78c952
Tools • Dart 3.4.0 (build 3.4.0-190.1.beta) • DevTools 2.33.1
kevmoo commented 8 months ago

CC @srujzs @MaryaBelanger

duck-dev-go commented 5 months ago

Is there any update on this? It seems nearly impossible to use wasm without more documentation on this.

srujzs commented 5 months ago

I think Marya has been working on an interop tutorial first and then this, but feel free to ask me questions in the meantime on how to port specific lines of code.

The general model is similar to package:js and we dive into some of the major differences here: https://dart.dev/interop/js-interop/past-js-interop#package-js.


Looking at the OP edits:

Error: The '@JS' annotation from 'dart:js_interop' can only be used for static interop, either through extension types or '@staticInterop' classes.

This is mildly covered on that webpage, but there are two @JS annotations - one from package:js (which is meant for package:js classes and can be unsound) and one from dart:js_interop (which is meant for extension types and is sound).

Before converting all the code in my dart js warper i would like to be sure that this is the correct approach.

In general, you should move code from classes to extension types on JSObject. external members can be more or less copied over to the extension type, with the caveat that the types you can use on such a member are restricted: https://dart.dev/interop/js-interop/js-types#requirements-on-external-declarations-and-function-tojs. The above code seems to only use primitives and interop types, so the migration would be rather simple (assuming MapboxMapJsImpl is also migrated to an interop extension type):

@JS('mapboxgl')
library mapboxgl.interop.ui.control.navigation_control;

import 'dart:js_interop';

import 'package:mapbox_gl_dart/src/interop/interop.dart';

// No need for `@JS` if not renaming, and no need for `@anonymous` as you can
// write an external constructor with named parameters directly (as shown below).
extension type NavigationControlOptionsJsImpl._(JSObject _) implements JSObject {
  external bool get showCompass;
  external bool get showZoom;
  external bool get visualizePitch;

  external factory NavigationControlOptionsJsImpl({
    bool? showCompass,
    bool? showZoom,
    bool? visualizePitch,
  });
}

@JS('NavigationControl')
extension type NavigationControlJsImpl._(JSObject _) implements JSObject {
  external NavigationControlOptionsJsImpl get options;

  external factory NavigationControlJsImpl(
      NavigationControlOptionsJsImpl options);

  external onAdd(MapboxMapJsImpl map);

  external onRemove();
}
duck-dev-go commented 5 months ago

Following the example with extension types, I can now no longer implement my own interface.

extension type JavascriptXHROptions._(JSObject _) implements JSObject, XHROptions

Which will give me an error saying XHROptions is not a supertype of JSObject. But I use my own interface for dependency inversion throughout my code. How can I with this new approach implement my own interfaces so my other projects won't break?

srujzs commented 5 months ago

What is XHROptions here? A package:js class? If so, the error makes sense: the representation type is not a subtype of XHROptions. Interop extension types are not supposed to compose package:js types so changing the representation type to XHROptions won't work either (you'll get a compile-time error saying JavascriptXHROptions is not an interop extension type now).

Instead, if you want to still reuse the interface of XHROptions, use a cast:

extension type JavascriptXHROptions._(JSObject _) implements JSObject {}
...
final javascriptXHROptions = ...;
(javascriptXHROptions as XHROptions).someMethod();

Note that package:js can't be used anywhere in your program when compiling to Wasm. If you make XHROptions an interop extension type, you can implement it like you have.

duck-dev-go commented 5 months ago

XHROptions is my own custom abstract class that serves as an interface only.

abstract class XHROptions {
  XHROptions({
    required String method,
    required Map<String, String> headers,
    required bool withCredentials,
  });

  String get method;
  set method(String value);

  Map<String, String> get headers;
  set headers(Map<String, String> value);

  bool get withCredentials;
  set withCredentials(bool value);
}

I just took it as an example as I have many of these interfaces. But my point was that before with package:js I was able to implement that interface like so

@JS()
@anonymous
class JavascriptXHROptions implements XHROptions {

But with the new approach

extension type JavascriptXHROptions._(JSObject _) implements JSObject, XHROptions

This now seems like it's no longer possible. Because XHROptions is not a super type of JSObject, and I don't want it to be. I want the interace to be separated from any js related coupling. Instead I use it for dependency inversion in my other packages to avoid them being tightly coupled to any web or js related package. But now it seems I cannot implement it anymore with this new approach.

So how do I migrate my package js code so that it doesn't break all my other packages because I cannot use the interfaces anymore.

srujzs commented 5 months ago

I see, you're just using it as an interface. Extension types by design do not allow virtual/dynamic dispatch, so you can't have an interop extension type implement an unrelated Dart class. One option to make this work (that will be slower) is a wrapper class:

extension type JavascriptXHROptions._(JSObject _) implements JSObject {
  // whatever interop members you might need
  JavascriptXHROptions({String method});
  external String method;
}

class JavascriptXHROptionsImpl implements XHROptions {
  late final JavaScriptXHROptions _options;

  JavascriptXHROptionsImpl({required String method}) {
    _options = JavascriptXHROptions(method);
  }

  String get method => _options.method;
  set method(String value) => _options.method = value;
}
duck-dev-go commented 5 months ago

Thanks for the example.

The irony, the very thing extension types would solve is the very thing they can't solve in a way.

IMO most people would never couple their project tightly to javascript code as flutter is a multi platform framework and in general dependency inversion is very commonly used in the flutter ecosystem. So the new extension type mostly just adds more boilerplate code for framework users. Where the old way of doing it with the js package likely has similar performance to this approach but less boilerplate code.

I'm a flutter user btw.

srujzs commented 5 months ago

Maybe. :) package:js classes disallowed you from having non-external members, so if you needed to do anything besides just directly call the JS member (with the same name as the interface API since you couldn't do renaming), you'd likely need to use a wrapper class anyways.

I've mostly seen that approach with more granular APIs that call a number of JS members and handle the results, so you need a wrapper class regardless in that case. However, in the case where you just need a set of options, like XHROptions, I can definitely see it being worse to use extension types.

duck-dev-go commented 5 months ago

In most cases we don't need to have non-external members on a JS class. Also wouldn't you be able to write extensions for the package:js classes, it seems to work for me? I'm still not seeing the advantage of using an extension type for this, but perhaps I'm overlooking something.

srujzs commented 5 months ago

Sorry, my comment might have been confusing - it was in reference to specifically implementing a shared interface.

Also wouldn't you be able to write extensions for the package:js classes, it seems to work for me?

Right, but that's the same as extension types in that the non-external methods wouldn't have virtual dispatch since they're extension methods. So, they couldn't be used to implement an interface, but if the external JS members directly implement the interface you need, you don't need the non-external members, and therefore the package:js class would be better.

I'm still not seeing the advantage of using an extension type for this, but perhaps I'm overlooking something.

For dependency inversion, there isn't an advantage. I was mostly pointing out that it might not be worse than package:js classes depending on the use case.

felix-ht commented 4 months ago

@srujzs your example is exactly what i ended up doing

Converting over 8200 lines of pure interface code took like two days, so its very much manageable.

The bigger issues i faced showed up as i tired to run the code after it was complied to wasm as suddenly the JS - dart interface becomes much stricter so any existing issue in the interface suddely start throwing errors. If you just complie to JS many issues don't show up. Managed to fix them tho.

I also ran into issues with other lib's ablity to compile to wasm, so any futher work was put on hold untill the ecsosystem catches up. Most of these issues i expect to have been resolved by now.

@duck-dev-go Pushed the full project if you need something to compare against:

https://github.com/Ocell-io/mapbox-gl-dart/tree/new-js_interop

srujzs commented 4 months ago

Nice! Yeah, I expect the change in what types are allowed will be the bigger lift than the change to extension types.

The bigger issues i faced showed up as i tired to run the code after it was complied to wasm as suddenly the JS - dart interface becomes much stricter so any existing issue in the interface suddely start throwing errors. If you just complie to JS many issues don't show up. Managed to fix them tho.

Can you elaborate what you mean by this? Do you mean you saw errors going from package:js to dart:js_interop or going from compiling to JS to compiling to Wasm? With dart:js_interop, we should hopefully have the same errors around restricted types regardless of whether you compile to JS or Wasm.

felix-ht commented 4 months ago

Is saw the error after porting to js_interop and had a basic running version with js. Some issues only showed up after i tried to compling to wasm. This was a multistage process. dart2wasm showed issues during compiling, while dart2js complied just fine. After that i also had some runtime errors with dart2wasm that were tricky to debug, as there is no debugger for wasm, and the error message / stacktrace was lacking as there weren't even debug symbols.

These two commit hold the changes required to get wasm running.

https://github.com/Ocell-io/mapbox-gl-dart/commit/e811c3d4db82b7d2299c2f620f08a1f8f28ec593

https://github.com/Ocell-io/mapbox-gl-dart/commit/d7366bab7135d0de7c17b8fe83b95fc0f5470d03

felix-ht commented 4 months ago

Looking at the code again it seems that all (most?) issues where with the js object container layer not with the interop itself.

srujzs commented 4 months ago

Ah I see, this is partially a change in imports from dart:js_util to dart:js_interop. We don't have any errors on the JS backends telling users to use dart:js_interop instead because the old JS interop libraries are not deprecated yet. There's a lint request to add that and in turn encourage migration, though. Thanks for sharing!

pedromassango commented 1 month ago

This is concerning, when the team recommends migrating from on package to another and no migration guide is provided, ideally we shouldn't assume that everyone knows how to do it :(

This really breaks the UX of using Dart