Move external constructors to factories on @staticInterop classes

[srujzs]

This CL is bringing in a lot of new changes that I want to avoid, but I don't think I can avoid it without using the source files from when this library was last generated. Therefore, I don't want it to be landed.

@jodinathan can I have you cherry-pick the changes to move these classes to factory and regenerate with your existing MDN files instead?

[jodinathan]

I will update the package as soon as 2.18 is out, ok?

[srujzs]

Sure, that works for me - I need to migrate other sources as well anyways. Thanks!

[jodinathan]

@srujzs I have already reconstructed the bindings but I am getting an error while serving the example app:

import 'package:js_bindings/js_bindings.dart';

void main() {
  window.document.title = 'JS Bindings example';
  document.title = 'JS Bindings example'; // also compile error
}

[SEVERE] build_web_compilers:ddc on example/main.ddc.module: Error compiling dartdevc module:js_bindings|example/main.sound.ddc.js

example/main.dart:4:10: Error: The getter 'document' isn't defined for the class 'JavaScriptObject'.
 - 'JavaScriptObject' is from 'dart:_interceptors'.
Try correcting the name to the name of an existing getter, or defining a getter or field named 'document'.
  window.document.title = 'JS Bindings example';
         ^^^^^^^^

document and window are both top level getters:

@JS()
external Window get window;

@JS()
external Document get document;

the pubspec:

name: js_bindings
description: Complete JS bindings interop with autocomplete and documentation.
version: 0.0.6
repository: https://github.com/jodinathan/js_bindings

environment:
  sdk: '>=2.18.0 <3.0.0'

dependencies:
  meta: ^1.4.0
  js: ^0.6.4

dev_dependencies:
  archive: ^3.1.2
  dart_style: ^2.0.1
  collection: ^1.15.0
  glob: ^2.0.1
  html: ^0.15.0
  http: ^0.13.3
  lints: ^2.0.0
  recase: ^4.0.0
  tcp_scanner: ^2.0.5
  test: ^1.16.0
  build_runner: ^2.2.0
  build_web_compilers: ^3.2.5

[srujzs]

Is this when you're doing a hot reload/restart? Modular compilation with DDC is still semi-broken, because types are erased in the component. Since these changes persist in modular compilation, when the front end does type checks, it sees the erased type instead. It's a hard problem to fix, and I'm not sure we'll get to it before views. The workaround for this issue is to not use hot reload/restart, which might mean re-building the app when you make changes? I'm not too familiar with how build_web_compilers uses modular compilation.

Related issue here: #49383

[jodinathan]

it happens with a clean webdev serve:

[jonathanrezende@joe js_bindings]$ rm -Rf .dart_tool/
[jonathanrezende@joe js_bindings]$ dart pub get
Resolving dependencies... (((1.8s)
  frontend_server_client 2.1.3 (3.0.0 available)
bGot dependencies!
dev [jonathanrezende@joe js_bindings]$ webdev serve example
[INFO] Building new asset graph completed, took 3.3s
[INFO] Checking for unexpected pre-existing outputs. completed, took 1ms
[INFO] Serving `example` on http://127.0.0.1:8080
[INFO] Generating SDK summary completed, took 6.1s
[SEVERE] build_web_compilers:ddc on example/main.ddc.module: Error compiling dartdevc module:js_bindings|example/main.sound.ddc.js

example/main.dart:4:10: Error: The getter 'document' isn't defined for the class 'JavaScriptObject'.
 - 'JavaScriptObject' is from 'dart:_interceptors'.
Try correcting the name to the name of an existing getter, or defining a getter or field named 'document'.
  window.document.title = 'JS Bindings example';
         ^^^^^^^^

[jodinathan]

@srujzs it worked ok with dart2js

[srujzs]

I wonder if this is because it doesn't include changes from https://github.com/dart-lang/sdk/commit/61abaeda3f84e6d45c4a70689b08b798713bc5c1 and some of the previous changes after 2.18. Can you try this with a version >= 2.19.0-52.0.dev?

[jodinathan]

@srujzs it worked with 2.19 but then another error:

// create the buttons to use in the example
  final div = (document.createElement('div') as HTMLDivElement)
    ..id = 'someDiv'
    ..innerHTML = 'This div was created on the fly. '
        'Node.elementNode: ${Node.elementNode}. '
        'Map: ${map.keys.map((k) => '$k: ${map[k]}').join(', ')}. '
    ..style.setProperty('border', '1px solid black') // runtime error here
    ..style.setProperty('margin', '10px');

errors.dart:263 Uncaught Error: Expected a value of type 'Object', but got one of type 'Null'
    at Object.throw_ [as throw] (errors.dart:263:21)
    at Object.castError (errors.dart:99:3)
    at Object.cast [as as] (operations.dart:470:10)
    at Object.as (core_patch.dart:73:24)
    at Object._callMethodUnchecked3 (js_util_patch.dart:134:3)
    at Object.PropsCSSStyleDeclaration$124setProperty [as PropsCSSStyleDeclaration|setProperty] (cssom_1.dart:479:15)

the js interop

@JS()
@staticInterop
class ElementCSSInlineStyle {
  external factory ElementCSSInlineStyle();
}

extension PropsElementCSSInlineStyle on ElementCSSInlineStyle {
  CSSStyleDeclaration get style => js_util.getProperty(this, 'style');
  StylePropertyMap get attributeStyleMap =>
      js_util.getProperty(this, 'attributeStyleMap');
}

[srujzs]

That's this method, right? https://github.com/jodinathan/js_bindings/blob/335efa22c84ad2a6248c08eae52bbe919438d6c2/lib/bindings/cssom_1.dart#L317

Should the return value be void instead? CSSStyleDeclaration.setProperty returns undefined. https://developer.mozilla.org/en-US/docs/Web/API/CSSStyleDeclaration/setProperty

[srujzs]

Any update on this? :)

[jodinathan]

taking a look at it now

[jodinathan]

@srujzs published 0.0.7 that works ok except the approach of using enum instead of String:

@anonymous
@JS()
@staticInterop
class RequestInit {
  external factory RequestInit._(
      {required String method,
      dynamic headers,
      dynamic body,
      required String referrer,
      required String referrerPolicy,
      required String mode,
      required String credentials,
      required String cache,
      required String redirect,
      required String integrity,
      required bool keepalive,
      AbortSignal? signal,
      required String duplex,
      dynamic window});

  factory RequestInit(
          {required String method,
          dynamic headers,
          dynamic body,
          required String referrer,
          required ReferrerPolicy referrerPolicy,
          required RequestMode mode,
          required RequestCredentials credentials,
          required RequestCache cache,
          required RequestRedirect redirect,
          required String integrity,
          required bool keepalive,
          AbortSignal? signal,
          required RequestDuplex duplex,
          dynamic window}) =>
      RequestInit._(
          method: method,
          headers: headers,
          body: body,
          referrer: referrer,
          referrerPolicy: referrerPolicy.name,
          mode: mode.name,
          credentials: credentials.name,
          cache: cache.name,
          redirect: redirect.name,
          integrity: integrity,
          keepalive: keepalive,
          signal: signal,
          duplex: duplex.name,
          window: window);
}

void main() {
  var init = RequestInit(
      method: 'GET',
      referrer: '',
      duplex: RequestDuplex.half,
      referrerPolicy: ReferrerPolicy.origin,
      mode: RequestMode.cors,
      credentials: RequestCredentials.omit,
      cache: RequestCache.noCache,
      redirect: RequestRedirect.follow,
      integrity: '',
      keepalive: true);

  print('init $init');
}

the above throws an error when in DDC.

[srujzs]

Yes, I believe we don't automatically convert enums across the external layer, so they'll need to be stringified. Thanks for migrating!

[jodinathan]

@srujzs yes.

That is the reason that I've created a layer factory before the external factory constructor, but it doesn't work with DDC.

[srujzs]

Right (if I'm understanding correctly) - non-external factories are still a TODO for me (hopefully soon): https://github.com/dart-lang/sdk/issues/46967