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

[js interop] Classes inside namespaces on the window cannot be constructed #54531

Closed
navaronbracke opened this issue Jan 5, 2024 · 8 comments
Labels
area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. web-js-interop Issues that impact all js interop

Comments

@navaronbracke
Copy link

navaronbracke commented Jan 5, 2024

I was updating a JS interop part of a library and encountered that globalThis did not contain a property which should have pointed to a JS class.

The library that I am integrating is ZXing, a barcode scanning library, which supports the web.

To reproduce:

  1. flutter create web_sample --platforms=web
  2. Add the ZXing script in the head tag of the index.html
<script src="https://unpkg.com/@zxing/[email protected]" type="application/javascript"></script>
  1. Add the following code sample to lib/interop.dart
lib/interop.dart
// ./lib/interop.dart
import 'dart:js_interop';

/// A static interop stub for the `Map` class.
///
/// This stub is here because the `js_types` from the Dart SDK do not yet provide a `Map` equivalent:
/// https://github.com/dart-lang/sdk/issues/54365
///
/// See also: https://github.com/dart-lang/sdk/issues/54365#issuecomment-1856995463
///
/// Object literals can be made using [jsify].
@JS('Map')
@staticInterop
class JsMap<K extends JSAny, V extends JSAny> implements JSObject {
  external factory JsMap();
}

extension JsMapExtension<K extends JSAny, V extends JSAny> on JsMap<K, V> {
  external V? get(K key);
  external JSVoid set(K key, V? value);
}

/// The JS interop class for the ZXing BrowserMultiFormatReader.
///
/// See https://github.com/zxing-js/library/blob/master/src/browser/BrowserMultiFormatReader.ts
@JS('ZXing.BrowserMultiFormatReader')
@staticInterop
class ZXingBrowserMultiFormatReader implements JSObject {
  /// Construct a new ZXingBrowserMultiFormatReader.
  ///
  /// The [hints] are the configuration options for the reader.
  /// The [timeBetweenScansMillis] is the allowed time between scans in milliseconds.
  ///
  /// See also: https://github.com/zxing-js/library/blob/master/src/core/DecodeHintType.ts
  external factory ZXingBrowserMultiFormatReader(
    JsMap? hints,
    int? timeBetweenScansMillis,
  );
}
  1. Replace main.dart with
main.dart
import 'dart:js_interop';
import 'dart:js_util';

import 'package:flutter/material.dart';

import 'interop.dart';

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

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

  @override
  Widget build(BuildContext context) {
    return const MaterialApp(
      title: 'Flutter Demo',
      home: MyHomePage(title: 'Flutter Demo Home Page'),
    );
  }
}

class MyHomePage extends StatefulWidget {
  const MyHomePage({super.key, required this.title});

  final String title;

  @override
  State<MyHomePage> createState() => _MyHomePageState();
}

class _MyHomePageState extends State<MyHomePage> {
  ZXingBrowserMultiFormatReader? reader;

  @override
  void initState() {
    super.initState();

    final JSObject? zxing = getProperty(globalThis, 'ZXing');
    final JSObject? multiFormatReader = getProperty(globalThis, 'ZXing.BrowserMultiFormatReader');

    print('ZXing is defined? ${zxing.isDefinedAndNotNull}');
    print('ZXing.BrowserMultiFormatReader is defined? ${multiFormatReader.isDefinedAndNotNull}');

    reader = ZXingBrowserMultiFormatReader(
      JsMap(),
      500,
    );

    print('reader variable is defined? ${reader?.isDefinedAndNotNull}');
    print('reader is a ZXing.BrowserMultiFormatReader? ${instanceOfString(reader, 'ZXing.BrowserMultiFormatReader')}');
  }

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(title: Text(widget.title)),
      body: const Center(),
    );
  }
}
  1. Run the application with flutter run -d chrome
  2. Observe the following log:
ZXing is defined? true
ZXing.BrowserMultiFormatReader is defined? false
reader variable is defined? true
reader is a ZXing.BrowserMultiFormatReader? false
  1. Open the browser console, and enter the expression "globalThis.ZXing.BrowserMultiFormatReader"
    This outputs
class extends L{constructor(t=null,e=500){const r=new ir;r.setHints(t),super(r,e)}decodeBitmap(t){return this.reader.decodeWithState(t)}}

It seems that the globalThis in Dart, is not fully in sync with the browser equivalent?
I have also tried using the callAsConstructor<T>() utility, where I had ZXing as a separate interop type,
which had BrowserMultiFormatReader as a JSFunction, but that ended up with the same result. (probably because that also looks at globalThis)

dart info

#### General info

- Dart 3.2.3 (stable) (Tue Dec 5 17:58:33 2023 +0000) on "macos_x64"
- on macos / Version 14.1.2 (Build 23B92)
- locale is en-BE

#### Process info

| Memory |  CPU | Elapsed time | Command line                                                                               |
| -----: | ---: | -----------: | ------------------------------------------------------------------------------------------ |
|  57 MB | 0.0% |     03:23:00 | dart devtools --machine --allow-embedding                                                  |
|  59 MB | 0.0% |        02:51 | dart devtools --no-launch-browser                                                          |
| 963 MB | 0.0% |     03:23:00 | dart language-server --protocol=lsp --client-id=VS-Code --client-version=3.80.0            |
| 138 MB | 0.0% |     03:23:00 | flutter_tools.snapshot daemon                                                              |
| 202 MB | 1.3% |        03:20 | flutter_tools.snapshot run -d chrome                                                       |
| 602 MB | 0.0% |        03:19 | frontend_server.dart.snapshot --sdk-root <path>/ --incremental --target=dartdevc --experimental-emit-debug-metadata -DFLUTTER_WEB_AUTO_DETECT=true -DFLUTTER_WEB_CANVASKIT_URL=https:<path>/ --output-dill <path>/app.dill --packages <path>/package_config.json -Ddart.vm.profile=false -Ddart.vm.product=false --enable-asserts --track-widget-creation --filesystem-root <path>/flutter_tools.9p94Xi --filesystem-scheme org-dartlang-app --initialize-from-dill build/a072907fec955372e484c180f3334617.cache.dill.track.dill --platform file:<path>/ddc_outline_sound.dill --verbosity=error --sound-null-safety |

MacOS Sonoma 14.1.2
Google Chrome 120.0.6099.199 (Official Build) (x86_64)
Flutter 3.16.5 / Dart 3.2.3

@mit-mit mit-mit added the area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. label Jan 5, 2024
@sigmundch sigmundch added the web-js-interop Issues that impact all js interop label Jan 5, 2024
@sigmundch
Copy link
Member

Thanks for filling the issue and reaching out.

We define globalThis in terms of self. The fact that ZXing is found, but the underlying class is not is a bit strange and lead me to believe the problem may be elsewhere. One thing that stands out to me is that in

final JSObject? multiFormatReader = getProperty(globalThis, 'ZXing.BrowserMultiFormatReader');

I don't believe the API supports using ., this would be equivalent to reading globalThis['ZXing.BrowserMultiFormatReader'] in the console. It is possible that changing that line to:

final JSObject? miltiFormatReader = getProperty(zxing, 'BrowserMultiFormatReader');

Will give you an actual object instead. Does that work for you?

If so, it's possible the instanceofString check has a similar limitation.

@navaronbracke
Copy link
Author

navaronbracke commented Jan 5, 2024

Thanks for clarifying the difference with getProperty.

However, I didn't quite get the results of what I was expecting still.
So I did the following:

  1. Define a JS interop stub on the Window to get the ZXing object, using @staticInterop + class + extension
  2. Grab the final ZXing zxing = web.window.zxing; variable
  3. Then get the constructor function, using your suggestion final JSFunction? multiFormatReader = getProperty(zxing, 'BrowserMultiFormatReader');
  4. final BrowserMultiFormatReader reader = multiFormatReader.callAsConstructor<BrowserMultiFormatReader>();
  5. The T in step 4 is another JS interop stub, but for the reader this time, which implements JSAny
  6. reader.isDefinedAndNotNull is true, likewise doing an instanceof('BrowserMultiFormatReader') (in the scope of the zxing object, not globalThis) results in true
  7. When actually using the final BrowserMultiFormatReader reader, I get TypeErrors at runtime (in JS), saying that the object is still null, despite the earlier checks

Maybe also important to mention for context is that the older version (which I am trying to update away from dart:html/package:js)

was defined like this:

import 'dart:html';

import 'package:js/js.dart';

@JS('ZXing.BrowserMultiFormatReader')
@staticInterop
class JsZXingBrowserMultiFormatReader {
  external factory JsZXingBrowserMultiFormatReader(
    dynamic hints,
    int timeBetweenScansMillis,
  );
}

Back then this was allowed it seems.

@sigmundch
Copy link
Member

Thanks for all the additional input. I believe this is likely related to another issue we saw recently in the compiler. I just filed an issue with more details here: #54534

I still need to double check and test this locally, but it is likely that the new interop constructor is triggering the issue with the precedence of the new operator.

@navaronbracke
Copy link
Author

navaronbracke commented Jan 23, 2024

@sigmundch as the main bug has been fixed now, shall we close this one as well, or do you want me to independently verify the fix? (although I think we now have a test in the SDK that covers this pattern, which would make my point moot?)

@sigmundch
Copy link
Member

Thanks for following up! Yes!

I've started the cherry-pick process to make sure this is included in the next stable release of Dart and Flutter. Please do keep us posted if you still run into trouble

@navaronbracke
Copy link
Author

Will do!

@navaronbracke
Copy link
Author

navaronbracke commented Jan 25, 2024

@sigmundch So it seems to be working at HEAD now, thanks! I tested it with the following sample, and it is WAI:

import 'dart:js_interop';
import 'dart:js_util';

void main() {
  final zxing = getProperty(globalThis, 'ZXing');
  final browserMultiFormatReaderConstructor = getProperty(zxing, 'BrowserMultiFormatReader');
  final browserMultiFormatReader = callConstructor(browserMultiFormatReaderConstructor, const []);

  print('instance is correct type: ${instanceof(browserMultiFormatReader, browserMultiFormatReaderConstructor)}');

  // This is false, because the dot notation is not supported by the tooling.
  print('instance is correct type: ${instanceOfString(browserMultiFormatReader, 'ZXing.BrowserMultiFormatReader')}');

  print(getProperty(browserMultiFormatReader, 'isMediaDevicesSuported')); // true
}

Should I file a bug for supporting the dot notation in the JS() annotation? (as that was supported in the old style interop iirc) Edit: I just did that, so we don't forget :)

@sigmundch
Copy link
Member

whohoo 🎉 🎉 ! thank you so much for verifying and keeping us posted!

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. web-js-interop Issues that impact all js interop
Projects
None yet
Development

No branches or pull requests

3 participants