Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lack of documentation for migration from js to js_interop #55352

Open
felix-ht opened this issue Apr 2, 2024 · 16 comments
Open

Lack of documentation for migration from js to js_interop #55352

felix-ht opened this issue Apr 2, 2024 · 16 comments
Labels
area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. type-documentation A request to add or improve documentation web-js-interop Issues that impact all js interop

Comments

@felix-ht
Copy link

felix-ht commented Apr 2, 2024

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
@nshahan nshahan added web-js-interop Issues that impact all js interop area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. type-documentation A request to add or improve documentation labels Apr 2, 2024
@kevmoo
Copy link
Member

kevmoo commented Apr 2, 2024

CC @srujzs @MaryaBelanger

@duck-dev-go
Copy link

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

@srujzs
Copy link
Contributor

srujzs commented Jun 25, 2024

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
Copy link

duck-dev-go commented Jun 26, 2024

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
Copy link
Contributor

srujzs commented Jun 26, 2024

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
Copy link

duck-dev-go commented Jun 26, 2024

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
Copy link
Contributor

srujzs commented Jun 26, 2024

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
Copy link

duck-dev-go commented Jun 27, 2024

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
Copy link
Contributor

srujzs commented Jun 27, 2024

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
Copy link

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
Copy link
Contributor

srujzs commented Jun 27, 2024

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
Copy link
Author

felix-ht commented Jul 18, 2024

@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
Copy link
Contributor

srujzs commented Jul 18, 2024

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
Copy link
Author

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.

Ocell-io/mapbox-gl-dart@e811c3d

Ocell-io/mapbox-gl-dart@d7366ba

@felix-ht
Copy link
Author

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

@srujzs
Copy link
Contributor

srujzs commented Jul 23, 2024

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. type-documentation A request to add or improve documentation web-js-interop Issues that impact all js interop
Projects
Status: No status
Development

No branches or pull requests

5 participants