Skip to content

Handle LegacyFactoryFunctions in generator #124

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

Closed
srujzs opened this issue Dec 7, 2023 · 9 comments
Closed

Handle LegacyFactoryFunctions in generator #124

srujzs opened this issue Dec 7, 2023 · 9 comments

Comments

@srujzs
Copy link
Contributor

srujzs commented Dec 7, 2023

Some interfaces in the Web IDL have a LegacyFactoryFunction defined in their metadata e.g. HTMLAudioElement that tells us that the factory for that interface is defined differently e.g. HTMLAudioElement here: https://html.spec.whatwg.org/multipage/media.html#the-audio-element.

The surface area of this metadata is small, affecting 3 interfaces today, all of which are elements and can be instantiated using createElement.

@ryanheise
Copy link

Copying your reply from flutter/flutter#139125 :

This is a bug in package:web. I filed an issue here: #124. The workaround is simple for now: document.createElement('audio').

There are no custom elements involved in package:web. It lowers the constructor to the intuitive lowering new HTMLAudioElement() in JS. That JS code returns that error, since the working way is to do new Audio() instead.

Marking it closed and tracking it in the package instead.

Now is this a future compatible workaround, in the sense of, wiil the document.createElement('audio') approach continue to work once web addresses this internally in the future?

@srujzs
Copy link
Contributor Author

srujzs commented Dec 8, 2023

Yes, it should. createElement is a key DOM API, so it should not change even if we fix this issue.

@ryanheise
Copy link

Ah, got it. In my head, I was completely off track in what I had thought you meant.

Now that I understand what you meant, I guess my question just to confirm is, can I assume that the typecast document.createElement('audio') as HTMLAudioElement or document.createElement('audio') as AudioElement is future compatible?

@srujzs
Copy link
Contributor Author

srujzs commented Dec 8, 2023

AudioElement is a typedef that we made to make it easier to migrate off of dart:html. At some far future date, we'd like to deprecate and remove that "glue" layer when it's no longer needed. So, I'd prefer the first one if you're worried about future compatibility, which should continue to work.

It's worth noting that this package is still not at v1, mostly because we want to keep adding features and migrate to extension types before we do so. document.createElement('audio') as HTMLAudioElement will still almost certainly not break, but something to keep in mind. Of course, if a breaking change comes along, we will release a breaking version, so your code should continue to work unless you update your pubspec.

@amrgetment
Copy link

I had the same issue for

  final HTMLMetaElement metaElement = HTMLMetaElement()
    ..name = 'google-signin-client_id'
    ..content = flavor.googleWebClientId;

Is this issue for all HTML elements or audio element only?

@ryanheise
Copy link

Is this issue for all HTML elements or audio element only?

The issue states that it's not all, not one, but exactly "three". The surface area is small. (HTMLMetaElement is not one of them.)

@srujzs
Copy link
Contributor Author

srujzs commented Feb 21, 2024

@ryanheise is correct here: this is a different bug even though it has similar results. Responded in #183.

@rutvik110
Copy link
Contributor

@srujzs Based on the following description for [LegacyFactoryFunction] values,

If the [LegacyFactoryFunction] extended attribute appears on an interface, it indicates that the JavaScript global object will have a property with the specified name whose value is a function that can create objects that implement the interface. From IDN docs

Would you say the following would be a correct representation of such factory functions in dart?

HTMLAudioElement Audio(String src){
  return HTMLAudioElement()..src =src;
}

Are there any other specifics that we want to take into account if we have to add support for [LegacyFactoryFunction] in the generator?

@srujzs
Copy link
Contributor Author

srujzs commented Sep 10, 2024

I actually don't think this is needed anymore. These are legacy ways to construct 3 element types. We added default constructors to call document.createElement instead for these and other element types, and therefore, there's no need to call these legacy constructors. I think we can go ahead and close this as the original motivation is handled.

@srujzs srujzs closed this as completed Sep 10, 2024
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

No branches or pull requests

4 participants