Suggestion: Refactor with extension type

[jarrodcolburn]

package:web seems to be using almost exclusive extension types. Mainly of JSObject type.
Should this package refactor the generator code to match? I think yes.

From a Dev Experience, this would allow better integration with package:web as any Dart app that uses package:chrome_extension is likely to coexists with package:web.

[jarrodcolburn]

^So for example MediaCapabilitiesInfo from package:web is just a extension type (that also implements) JSObject

extension type MediaCapabilitiesInfo._(JSObject _) implements JSObject {
  external factory MediaCapabilitiesInfo({
    required bool supported,
    required bool smooth,
    required bool powerEfficient,
  });

[jarrodcolburn]

WIP https://github.com/jarrodcolburn/chrome_extension.dart/tree/refactor_JSObject

Started got somethings working. Taking Session as an example, Running the tool will produce ...

// sessions.dart
extension type ChromeSessions._(JSObject _) implements JSObject { // as extension type
  external Future<Session> restore(String? sessionId); // as external method
  external int get maxSessionResults; // as getter
}

[xvrh]

There are 2 layers in the package:

• The low level binding in lib/src/js which uses dart:js_interop and extension type. It is very similar to package:web.
• The high level API which wraps the low level binding and exposes a nice "darty" API for easy use.

The high level API has several friendly features: transform Event to Stream, use enum instead of raw String...

Initially I though to expose both layers and letting the user choose which one they prefers. But at the end, I found the high level much easier/safer to use and didn't find a reason to not prefer it all the time.
I can be convinced otherwise if you can show me some examples.

[jarrodcolburn]

Thanks. And Sorry, I was completely wrong about the implementation.

But I still think there is ground to move more closely mirror the way package:web.

Mainly by using Generics in your lib/src/js.
For examples

Use Generics for JSPromise example : chrome.action.getTitle

// lib/src/js/action.dart

// current
external JSPromise         getTitle(TabDetails details);

// expected
external JSPromise<String> getTitle(TabDetails details);

Use Generics for JSArray example chrome.action.getBadgeBackgroundColor

// lib/src/js/action.dart (continued) 

// current
external JSPromise               getBadgeBackgroundColor(TabDetails details);

//expected
external JSPromise<JSArray<int>> getBadgeBackgroundColor(TabDetails details);

Doing so will allow your package to not have to generate so much of the "darty" within the package. And rely on the JS<->Dart implementation going on in dart:js_interop

[xvrh]

Good catch about using generics, thanks!
I'll try to add the next time I iterate on this package :-)

[jarrodcolburn]

Also, recommend changing the API as it currently exists to generate a stream.

// current to get stream
chrome.tabs.onAttached 

// recommended 
chrome.tabs.onAttached.toDart

toDart properties more closely align with the js_interop implementation. As there are several in js_interop for example JSArrayToList.toDart

This would allow more unification of the js and dart API.

// people could still call `js` style in same code base if wanted (or while migrating)
chrome.tabs.onAttached.addListener 

There are definitely trades offs to both approaches (neither is perfect). But mimicking js_interop's .toDart and .toJS syntax is probably the most consistent.

[xvrh]

Thanks for the suggestion, I'll think about it.

Also, it's reasonable to expose the underlying wrapped JSObject.

[jarrodcolburn]

which can be done pretty easily be adding a Generic to your Event.
Current implementation

// current
extension type Event   ._(JSObject _) implements JSObject {
                  // ^ no generic
  // addListener, removeListener, ... 
}

Added Generic type implementation

// if you add a generic
extension type Event<T>._(JSObject _) implements JSObject {
// `T` is just the arguments of the callback wrapped in `extension type` to allow indexing

  // addListener, removeListener, ... 
}

Generic Event example (OnZoomChange)

// using OnZoomChange as an example...
extension type Tab._(JSObject _) implements JSObject {
  // current call
  external Event                    get onZoomChange;
  // with generic
  external Event<OnZoomChangeEvent> get onZoomChange;
}
// `OnChangeZoomEvent` wrapper of/returns callback's `arguments` that uses indexing to expose objects 
extension type OnZoomChangeEvent._(JSObject arr) implements JSObject {
  int get tabId => arr[0];
  double get oldZoomFactor => arr[1];
  double get newZoomFactor => arr[2];
  ZoomSettings get zoomSettings => arr[3];
  // if indexing doesn't work above, may need to either
  // A) change arr from JSObject to either `JSArray` (implement indexing?) or `List`
  // B) Use `arr.getProperty()` (from `js_interop_unsafe` ?)  
}
// ^ this is like a simplified `OnZoomChangeZoomChangeInfo` that doesn't need a constructor. 

Although the example is for OnZoomChange is the example, similar implementations for all events.

toDart extension on Event should work for any of these Event's with a generic

// `toDart` can become a simple extension on `Event` class above
extension EventStreamMapper<T> on Event<T> {
   Stream<T> get toDart { 
     final controller = Stream controller<T>();
     T callbackMapper([JSAny? a, JSAny? b, JSAny? c, JSAny? d]){
        return [a,b,c,d] as T; // this may need to be tweaked. just trying to get args
     }
     // use Event's method
     /*this.*/addListener(([JSAny? a, JSAny? b, JSAny? c, JSAny? d]){
        controller.add(callbackMapper([a,b,c,d]))
      }.toJS);
     return controller.stream;
  }
}

I'm not quite sure about the syntax of my callbackMapper function. The irony is that this is a case where I think js_interop is doing some magic, but I actually want it to do less.