Skip to content

Conversation

@sgrekhov
Copy link
Contributor

No description provided.

@sgrekhov
Copy link
Contributor Author

BTW @srujzs what types are allowed in an extension that provides members for @staticInterop class? I didn't fins any documentation about it.

import 'dart:js_interop';

@staticInterop
@JS()
class C {
}

extension Ext on C {
  external String getString();  // Ok
  external int getNumber();     // Ok
  external bool getBool();      // Ok
  external List<double> getList(); // Error: External JS interop member contains invalid types in its function signature: '*List<double>* Function()'.
  external Object getObject();  // Error: External JS interop member contains invalid types in its function signature: '*Object* Function()'.
  external JSObject getJSObject();  // Ok
}

Copy link
Member

@eernstg eernstg left a comment

Choose a reason for hiding this comment

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

Added a handful of comments. A couple of questions came to mind, in particular about the support for having a @staticInterop extension type, and the ability to compile invocations of external extension instance members as raw JavaScript invocations without @JS on the extension.

Copy link
Contributor Author

@sgrekhov sgrekhov left a comment

Choose a reason for hiding this comment

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

One description has been updated. Let’s wait for @srujzs’s response. I couldn’t find any documentation on how this annotation is intended to be used or what its limitations are, so the tests were written assuming that the current behavior is correct

@srujzs
Copy link

srujzs commented Jul 16, 2025

Tricky stuff! My general comments are:

  • Annotating a mixin with @JS or @staticInterop or @anonymous should likely be an warning and we should make our transformer be more resilient to mixins.
  • The ability to declare external extension members is entirely predicated on whether the on-type is a JS interop type. There are two: interop extension types and JS interop classes. If the on-type is not a valid JS interop type, we should expect an error on each of the external extension members.
  • It should be a warning to put @staticInterop on extension types, member declarations, enums, etc. These can safely be a no-op.

This is one of the reasons why I wanted to move away from annotations with extension types with dart:js_interop (besides @JS as we still need renaming): every declaration must be validated that they can use that annotation.

what types are allowed in an extension that provides members for @staticInterop class? I didn't fins any documentation about it.

https://dart.dev/interop/js-interop/js-types#requirements-on-external-declarations-and-function-tojs

edit: Sorry for the back-and-forth around mixins. I believe we can modify our transformer and safely treat JS interop annotations on them as warnings, not errors.

Copy link
Contributor Author

@sgrekhov sgrekhov left a comment

Choose a reason for hiding this comment

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

Thank you! Updated. Added new tests that expect errors/warnings for non JS interop classes annotated with @staticInterop. Removed tests checking that mixin and other types can be used in the interop. PTAL.

@sgrekhov sgrekhov requested review from eernstg and srujzs July 17, 2025 08:49
Copy link
Member

@eernstg eernstg left a comment

Choose a reason for hiding this comment

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

LGTM.

@srujzs, it looks like you're happy about this PR at this time? I couldn't spot any comments from you that haven't been processed, as far as I can see.

I'll land it now, and if you have further comments then we can handle them in an additional PR.

@eernstg eernstg merged commit 40215be into dart-lang:master Jul 17, 2025
2 checks passed
copybara-service bot pushed a commit to dart-lang/sdk that referenced this pull request Jul 18, 2025
2025-07-17 sgrekhov22@gmail.com dart-lang/co19#3180. Add `@staticInterop` tests. Part 1. (dart-lang/co19#3256)
2025-07-15 sgrekhov22@gmail.com dart-lang/co19#3180. Add tests checking `name` value (dart-lang/co19#3255)
2025-07-15 sgrekhov22@gmail.com dart-lang/co19#3180. Add `anonymous` annotation tests (dart-lang/co19#3254)
2025-07-15 sgrekhov22@gmail.com dart-lang/co19#3180. Add JSExport tests. Part 4. (dart-lang/co19#3253)

Cq-Include-Trybots: luci.dart.try:analyzer-linux-release-try,dart2js-minified-linux-d8-try
Change-Id: Ice01c51e219f0ca68f92390db029ca3f8a65b7a8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/441140
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Commit-Queue: Erik Ernst <eernst@google.com>
Reviewed-by: Erik Ernst <eernst@google.com>
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.

3 participants