Skip to content

feat(js_interop_gen): support automatically generated WebIDL typings via dogfooded generator#554

Open
kevmoo wants to merge 3 commits into
more_workfrom
webidl_dts
Open

feat(js_interop_gen): support automatically generated WebIDL typings via dogfooded generator#554
kevmoo wants to merge 3 commits into
more_workfrom
webidl_dts

Conversation

@kevmoo

@kevmoo kevmoo commented May 29, 2026

Copy link
Copy Markdown
Member

Extracts and registers the automated WebIDL AST typings generator inside 'tool/update_webidl_bindings.dart'.
Implements an elegant, stateless AST Renamer & Traversal Pass inside 'transform.dart' mapping raw type definitions recursively to standard nomenclatures under strict non-nullable nullabilities, keyword translations, and custom class property injections.
Deduplicates implements declarations globally to prevent clashing superclass annotations, generating pristine clean bindings with zero compiler source modifications.

…via dogfooded generator

Extracts and registers the automated WebIDL AST typings generator inside 'tool/update_webidl_bindings.dart'.
Implements an elegant, stateless AST Renamer & Traversal Pass inside 'transform.dart' mapping raw type definitions recursively to standard nomenclatures under strict non-nullable nullabilities, keyword translations, and custom class property injections.
Deduplicates implements declarations globally to prevent clashing superclass annotations, generating pristine clean bindings with zero compiler source modifications.
@kevmoo kevmoo requested review from nikeokoronkwo and srujzs May 29, 2026 16:56

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the JS interop generator to support dogfooding of WebIDL typings, introducing a renaming map, custom type nullability fixes, and generated bindings for webidl2. Key changes include AST updates, a new transformation pass, and generated Dart files for the interop API. Feedback on the changes highlights a critical compile-time error in the generated code due to void property overrides, which should be filtered out. Additionally, reviewers noted an abstraction leak from hardcoding WebIDL-specific types in the core AST library and suggested optimizing JS interop getters by removing redundant hasProperty checks.

}

// 2. Apply custom type nullabilities and property name fixes
if (decl is TypeDeclaration) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

The generated webidl_api.dart contains several compile-time errors because subtypes (such as InterfaceMixin, CallbackInterface, Namespace, etc.) override properties from their super-interfaces (Node and Interfacelike) with a type of void (e.g., external void parent; and external void inheritance;). In Dart, void is not a compatible subtype of Node? or String?, resulting in static analysis and compilation failures.

Since properties of type void are useless in Dart anyway, we should completely filter them out during the transformation pass. This allows the generated classes to cleanly inherit the correct types (Node? parent and String? inheritance) from their super-interfaces.

        if (decl is TypeDeclaration) {
          // Remove useless properties of type 'void' to prevent compile-time override conflicts
          decl.properties.removeWhere((prop) {
            final type = prop.type;
            return type is BuiltinType && type.name == 'void';
          });

Comment thread js_interop_gen/lib/src/ast/types.dart Outdated
Comment on lines +903 to +908
if (name == 'RHS' && spec.name == 'value') {
emittedType = refer('String');
}
if (name == 'ValueDescription' && spec.name == 'type') {
emittedType = refer('String');
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Hardcoding specific WebIDL types like 'RHS' and 'ValueDescription' inside the core AST library (types.dart) is an abstraction leak. js_interop_gen is designed to be a general-purpose JS interop generator from TypeScript definitions, so its core AST nodes should remain completely generic and free of library-specific overrides.

Instead of hardcoding these inside types.dart, we should pass these overrides dynamically via TypeOptions or DeclarationOptions from transform.dart, or handle them during the transformation pass.

Comment on lines +13 to +27
bool get union {
final jsObj = this as JSObject;
if (jsObj.hasProperty('union'.toJS).toDart) {
return (jsObj.getProperty('union'.toJS) as JSBoolean).toDart;
}
return false;
}

String get generic {
final jsObj = this as JSObject;
if (jsObj.hasProperty('generic'.toJS).toDart) {
return (jsObj.getProperty('generic'.toJS) as JSString).toDart;
}
return '';
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using hasProperty followed by getProperty results in two separate JS interop roundtrips for each getter access. We can optimize this by directly calling getProperty and checking if the returned value is of the expected type using isA<T>(). This is more efficient, idiomatic, and safer.

  bool get union {
    final jsObj = this as JSObject;
    final prop = jsObj.getProperty('union'.toJS);
    return prop.isA<JSBoolean>() ? (prop as JSBoolean).toDart : false;
  }

  String get generic {
    final jsObj = this as JSObject;
    final prop = jsObj.getProperty('generic'.toJS);
    return prop.isA<JSString>() ? (prop as JSString).toDart : '';
  }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant