sdk icon indicating copy to clipboard operation
sdk copied to clipboard

dev_compiler fails when hot reloading with JS interrop

Open Schwusch opened this issue 3 years ago • 9 comments

When hot reloading I get this error:

lib/main.dart:18:27: Error: The argument type 'void Function(USBDevice)' can't be assigned to the parameter type 'void Function(JavaScriptObject)'.
 - 'USBDevice' is from 'package:js_demonstration/other.dart' ('lib/other.dart').
 - 'JavaScriptObject' is from 'dart:_interceptors'.
    usb?.subscribeConnect(onConnect);

This is when hot reloading any change in main.dart in this Flutter project: https://github.com/Schwusch/dart_js_bug_repro. It is the smallest reproducible code example I could make. When making a change and hot reloading in other.dart, everything works as expected and the code is hot reloaded. I am using a M1 MacOSX 12.4 Monterey.

Reproduced with both:

flutter --version
Flutter 3.1.0-0.0.pre.1423 • channel master •
https://github.com/flutter/flutter.git
Framework • revision 8fe82bbe60 (2 days ago) • 2022-06-29 10:16:05 +0300
Engine • revision 8c36371fa7
Tools • Dart 2.18.0 (build 2.18.0-234.0.dev) • DevTools 2.14.1

and:

flutter --version
Flutter 3.0.4 • channel stable • https://github.com/flutter/flutter.git
Framework • revision 85684f9300 (22 hours ago) • 2022-06-30 13:22:47 -0700
Engine • revision 6ba2af10bb
Tools • Dart 2.17.5 • DevTools 2.12.2

main.dart

import 'package:flutter/material.dart';

import 'other.dart';

void main() {
  runApp(const MyApp());
}

class MyApp extends StatelessWidget {
  const MyApp({super.key});

  doTheThing(USBDevice device) {}

  void onConnect(USBDevice device) => doTheThing(device);

  @override
  Widget build(BuildContext context) {
    usb?.subscribeConnect(onConnect);
    return const MaterialApp(
      home: Scaffold(
        body: Center(child: Text('hello world')),
      ),
    );
  }
}

other.dart

@JS()
library web_usb;

import 'package:js/js.dart';
import 'package:js/js_util.dart';

@JS('navigator.usb')
external Usb? get usb;

@JS('USB')
@staticInterop
class Usb {}

typedef USBConnectionEventListener = void Function(USBDevice event);

extension PropsUsb on Usb {
  void subscribeConnect(USBConnectionEventListener listener) {
    callMethod(this, 'addEventListener', [
      'connect',
      allowInterop((USBConnectionEvent event) => listener(event.device))
    ]);
  }
}

@JS()
@staticInterop
class USBDevice {
  external USBDevice();
}

@JS()
@staticInterop
class USBConnectionEvent {
  external USBConnectionEvent();
}

extension PropsUSBConnectionEvent on USBConnectionEvent {
  USBDevice get device => getProperty(this, 'device');
}

Schwusch avatar Jul 01 '22 18:07 Schwusch

It makes sense that reloading main doesn't work while reloading web_usb does since void Function(JavaScriptObject) <: void Function(USBDevice). I think this is a result of erasing the types in both libraries and then when hot reloading, doing type-checking without erasing the reloaded library.

Perhaps the right thing here is to erase before type-checking, but that may have its own issues.

srujzs avatar Jul 02 '22 00:07 srujzs

Late response to this but I've been working on a similar bug on modular compilation and erasure that I believe should fix this. The fix should be to do a global transform once the whole program is compiled (before code generation). This way, reloading modules should not result in inconsistencies. I'll respond back once that's landed.

srujzs avatar Jul 29 '22 18:07 srujzs

Thanks a lot for the update. Are you referring to https://dart-review.googlesource.com/c/sdk/+/253000/ ?

Great work, I saw you landed a few others as well. I look forward to not working around the bug ☺️

Schwusch avatar Aug 01 '22 16:08 Schwusch

Yup, it has landed: https://github.com/dart-lang/sdk/commit/61abaeda3f84e6d45c4a70689b08b798713bc5c1

A dev version should be available in the next few hours. Flutter rolls might take a little while (I believe they're on the cadence of several days, but I'm not sure). Please let me know if you still come across this issue then, thanks!

srujzs avatar Aug 01 '22 18:08 srujzs

I pulled from Flutter master release:

Flutter 3.1.0-0.0.pre.2032 • channel master • https://github.com/flutter/flutter.git
Framework • revision 195fa0b728 (2 hours ago) • 2022-08-02 05:39:06 -0400
Engine • revision 4b19256979
Tools • Dart 2.19.0 (build 2.19.0-53.0.dev) • DevTools 2.16.0

And the problem still persists. I take it the fix landed in 2.19.0-52.0.dev per this commit: https://github.com/dart-lang/sdk/commit/cdad34a0392362c0e580048918cb6ea481e84662 Am I thinking correctly?

Schwusch avatar Aug 02 '22 12:08 Schwusch

Thanks for trying this! I fear you're right after trying this locally as well. :(

We'll likely need to pipe this lower into DDC somewhere where we can do a global transform at the point where the different uses of DDC intersect (expression evaluation, this, etc.). I'll respond back once I have something.

srujzs avatar Aug 03 '22 00:08 srujzs

This might not help you @srujzs, but I've worked around the problem by creating a wrapper/proxy class for the static interop code.

That fixes hot restart in the GUI code for me at least.

Schwusch avatar Aug 04 '22 06:08 Schwusch

I now noticed the wrapper/proxy solution actually broke the functionality, since the callback is no longer called. Maybe the closure passed to allowInterop broke because of it?

Schwusch avatar Aug 04 '22 12:08 Schwusch

Yeah, that should work since that's a Dart class and we're not doing any transformations there. It is a bit cumbersome though, which is unfortunate.

I think that breaks because the listener in the allowInterop call is being passed a USBDevice, instead of the wrapper class you created. You can probably work around this using:

typedef USBConnectionEventListener = void Function(UsbDeviceWrapper event);

extension PropsUsb on Usb {
  void subscribeConnect(USBConnectionEventListener listener) {
    callMethod(this, 'addEventListener', [
      'connect',
      allowInterop((USBConnectionEvent event) => listener(UsbDeviceWrapper(event.device)))
    ]);
  }
}

srujzs avatar Aug 04 '22 19:08 srujzs

Hi @Schwusch, I think this should be fixed with 2.19.0-351.0.dev looking at some modular tests. This should be in Flutter master by now if you'd like to give it a try.

srujzs avatar Nov 02 '22 23:11 srujzs

Hi, I just checked with latest on master, and it seems to be working in all the use cases I have, thank you 🙏 I'd say this issue is resolved. Great work 💪

Schwusch avatar Nov 07 '22 08:11 Schwusch

Awesome, thanks for trying it out!

srujzs avatar Nov 07 '22 17:11 srujzs