dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.19k stars 1.57k forks source link

Allow users to access hidden and incomplete HTML APIs #42200

Open sigmundch opened 4 years ago

sigmundch commented 4 years ago

Several APIs in dart:html are incomplete and some have been hidden in our code generator as private classes. Users have run into scenarios where they want to use these classes but they find that they are incomplete, and the standard way of using Js-interop doesn't work in dart2js.

See two examples:

The users noticed that the API is hidden, so they tried using @JS to access it directly. This creates an issue because dart2js associates the dart:html interceptor with those types, so their JS-interop interfaces don't work. The only mechanism that works for them is to: store the objects in a dynamic variable and invoke it's methods via js_util.callMethod.

Some ways to address the original issue would be to:

The former would work right away, but could potentially be a source of breaking changes in the future if we were to add back the definition in the future. The latter requires compiler work (support for external extension methods is not available), so the fix will not be available as quickly.

ryanheise commented 3 years ago

Some ways to address the original issue would be to:

  • delete our definition - so that the js-interop definitions can work without conflict

  • expose our definition (even if empty), and allow extension methods to include new external members that can be implemented via js-interop.

With the second option, note that there are some native classes in dart:html that are incomplete not just due to missing members, but also due to missing parameters of existing methods, or the incorrect type signature of existing methods (e.g. MediaSession.setActionHandler). In the latest SDK code, the compiler detects conflicting class names, but if it eventually goes further and detects conflicting member names, this may be something to consider, as it might prevent extension methods working as a solution.

Otherwise, I think the least disruptive migration path would be that if an app or plugin defines its own @JS class that conflicts with a native one, the @JS definition should override the native one. I don't know how feasible that is at all, but I am interested in a less-disruptive path in general, since plugins need to be written in a way that works on all flutter channels (stable for iOS/Android, beta for web, dev for macos, ...)

sigmundch commented 3 years ago

Thanks @ryanheise for sharing your insights here and in #44211!

Your point on existing methods having incorrect signatures is very valid, and something we want to improve on. We would love for dart:html to always be up to date, but given the slower update and release process of this library, we want to ensure there is an alternative so no one is blocked when the library is out of date.

To address some of your comments:

... but if it eventually goes further and detects conflicting member names

I am not sure I followed your concern here. Is this referring to names in external members of JS-interop classes? Or names from extension methods?

The former are likely to show a lot of false positives, so we can't really reject them. If someone has a JS object that happens to have a property named title it would all the sudden conflict with Element.title in dart:html, making the JavaScript code hard to use.

The latter I hope it can be resolved by the compiler without conflicts.

if an app or plugin defines its own @JS class that conflicts with a native one, the @JS definition should override the native one.

Unfortunately this is not possible yet today because it breaks composition of libraries. Imagine you have one library written against dart:html and a second library adds a conflicting @JS definition, we could break the code of the original library if we override which definition to use.

We are, however, aiming to get close to this desired scenario. It all comes down to static or virtual dispatch calls. The source of conflict comes form virtual calls, where we have to use the same semantics in the whole program. However, relying more on static dispatch mechanisms (like static extension methods), will let us generate code differently on a library that uses those static calls.

ryanheise commented 3 years ago

... but if it eventually goes further and detects conflicting member names

I am not sure I followed your concern here. Is this referring to names in external members of JS-interop classes? Or names from extension methods?

Let's take an example from dart:html:

@Native("MediaSession")
class MediaSession extends Interceptor {
  // ...
  void setActionHandler(String action, MediaSessionActionHandler? handler)
      native;
}

Now let's say we want to use the extension method approach to fix the incorrect parameter type of setActionHandler. Sure, we would have to choose a different Dart method name so that it doesn't conflict at the Dart level, but then we would also need an annotation to map it to the same JS name that is implied by the original native implementation of this method above. So what I'm saying is that if you were to eventually do some sort of similar conflict checking, you would inadvertently block the ability of a 3rd party project to use its own conflicting version of setActionHandler.

The former are likely to show a lot of false positives, so we can't really reject them. If someone has a JS object that happens to have a property named title it would all the sudden conflict with Element.title in dart:html, making the JavaScript code hard to use.

I was assuming that any sort of checking here would be scoped to the class of that member. Something like: raise a conflict if there is a native MediaSession.setActionHandler method and also a @JS MediaSession.setActionHandler method. My only comment was to suggest that if extension methods were to be offered as a way to patch the incomplete dart:html implementation, this seems simple enough if you only consider adding missing members, but I would want a way to be able to patch MediaSession.setActionHandler. If I make an extension method with a different Dart method name, say setActionHandler_2, and then use an annotation to point it to the same JS setActionHandler(#), I would then only hope that no new "conflict detection" checks are added in the future that would stop the compile and say that you couldn't define a method with the same name as an existing native method (in that class).

sigmundch commented 3 years ago

Got it, that makes perfect sense and we too share that concern and don't want an error that would prevent you from doing that.

Thanks for clarifying!