Skip to content
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

[js] Update js.html externs to Aug 2018 WebIDLs #7354

Merged

Conversation

haxiomic
Copy link
Member

@haxiomic haxiomic commented Aug 15, 2018

Motivation
The existing js.html externs are from 2 years ago and missing a number of important and popular APIs. This PR brings them up to date with the latest Mozilla WebIDLs as well making a number of typing improvements and bugfixes

--

The externs were generated using a fork of HaxeFoundation/html-externs and fork of @markknol's document generator.

The changes include many missing fields, bug fixes, new APIs and new classes. Some of the headline changes are:

  • Add WebGL2 externs and doc: WebGL2RenderingContext.hx
  • Add missing fields to TypedArray externs 18dd40a
  • Add Encrypted Media Extensions API
  • Update and add missing classes from the Web Audio API
  • Update and add missing classes from the Web RTC API
  • Update and add missing classes from the Web Animations API

Issues resolved include:

The PR deprecates removes the following files (which represent types no longer available in browsers or were misplaced originally):

  • js/html/AnimationEffectReadOnly.hx
  • js/html/AnimationEffectTiming.hx
  • js/html/AnimationEffectTimingProperties.hx
  • js/html/AnimationEffectTimingReadOnly.hx
  • js/html/AppletElement.hx
  • js/html/ApplicationCache.hx
  • js/html/AudioChannel.hx
  • js/html/AudioContextState.hx
  • js/html/CSSCharsetRule.hx
  • js/html/CSSPrimitiveValue.hx
  • js/html/CSSUnknownRule.hx
  • js/html/CSSValue.hx
  • js/html/CSSValueList.hx
  • js/html/ComputedTimingProperties.hx
  • js/html/ContentElement.hx
  • js/html/DOMCursor.hx
  • js/html/DOMTransaction.hx
  • js/html/DOMTransactionEvent.hx
  • js/html/DOMTransactionEventInit.hx
  • js/html/DesktopNotification.hx
  • js/html/DesktopNotificationCenter.hx
  • js/html/ElementRegistrationOptions.hx
  • js/html/KeyframeEffectReadOnly.hx
  • js/html/NotifyPaintEvent.hx
  • js/html/RGBColor.hx
  • js/html/RecordErrorEvent.hx
  • js/html/RecordErrorEventInit.hx
  • js/html/Rect.hx
  • js/html/ShadowElement.hx
  • js/html/audio/OfflineAudioCompletionEvent.hx
  • js/html/svg/AltGlyphElement.hx
  • js/html/svg/ZoomEvent.hx
  • js/html/AudioContextState.hx (moved to js.html.audio)

The following files have been removed without deprecation note because they represent internal types that should not have been generated (so no one will be using them in the wild)

  • js/html/ChromeFilePropertyBag.hx
  • js/html/CommandEvent.hx
  • js/html/DummyInterface.hx
  • js/html/DummyInterfaceWorkers.hx
  • js/html/LifecycleCallbacks.hx
  • js/html/MessagePortList.hx
  • js/html/ObjectURLOptions.hx
  • js/html/SimpleGestureEvent.hx
  • js/html/UncaughtRejectionObserver.hx
  • js/html/svg/Document.hx
  • js/html/XMLStylesheetProcessingInstruction.hx

WebGL extensions have been renamed and moved to js.html.webgl.extensions, this is to avoid naming conflicts with the previous scheme (WEBGL_color_buffer_float and EXT_color_buffer_float were previously incorrectly mapped to the same file).

@haxiomic
Copy link
Member Author

Regarding deprecations: at the moment I've retained deprecated files but I'm happy to delete these files for the 4.0.0 release as I don't expect them to have much real-world use

@haxiomic
Copy link
Member Author

Hold off merging just yet; I'm working through popular projects and checking for issues

@nadako
Copy link
Member

nadako commented Sep 5, 2018

@Simn @ncannasse this looks like Haxe 4 material to me

@nadako nadako added the platform-javascript Everything related to JS / JavaScript label Sep 5, 2018
@Simn Simn added this to the Release 4.0 milestone Sep 5, 2018
@haxiomic
Copy link
Member Author

haxiomic commented Sep 5, 2018

There’s a few more tweaks on the way but I expect it to be complete soon :)

@haxiomic
Copy link
Member Author

haxiomic commented Sep 9, 2018

I'd say the new externs are ready to go

I've tested sample projects for popular libraries:

Library Result
Heaps No errors
OpenFL 1 error, has since been fix in OpenFL
Kha HTML5 No errors
Flixel No errors

@Simn
Copy link
Member

Simn commented Sep 9, 2018

@nadako: I'll let you pull the trigger on this.

@nadako nadako merged commit d8e720f into HaxeFoundation:development Sep 9, 2018
@nadako
Copy link
Member

nadako commented Sep 9, 2018

This is an awesome contribution, thanks a lot! Get ready for some backward-compatibility issues though :)

@Simn
Copy link
Member

Simn commented Sep 9, 2018

Indeed, thanks a lot for this!

I have added you to the Haxe core team so you can deal with the fallout.

@nadako
Copy link
Member

nadako commented Sep 9, 2018

Hmm, I wonder why OpenFL compiled for you, I'm getting an error because of the missing print method here: https://github.com/openfl/openfl/blob/aee77ad4bf8626f5e55862c11297919ec255fcf0/src/openfl/printing/PrintJob.hx#L118

@haxiomic
Copy link
Member Author

haxiomic commented Sep 9, 2018

Thanks @nadako for finding that one – what project are you compiling? I was testing OpenFL samples which compiled ok but It looks like they didn't touch that class

I'll run through another validation pass and try and touch every file

I've opened a PR for the fixes: #7405

@nadako
Copy link
Member

nadako commented Sep 9, 2018

what project are you compiling

some personal one with --macro include("openfl") :)

Scrolls the page until the element gets into the view.
**/
@:overload( function( ?arg : ScrollIntoViewOptions) : Void {} )
function scrollIntoView( ?arg : Bool ) : Void;
Copy link
Contributor

@kLabz kLabz Mar 4, 2019

Choose a reason for hiding this comment

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

I'm late to the party, but this change is problematic.

Having ?arg : Bool instead of ?arg : ScrollIntoViewOptions as the primary signature for scrollIntoView breaks type inference and makes the use of ScrollIntoViewOptions really annoying.

Is there a way to prioritize in the generator?

Edit: there seem to be other APIs with the same problem, e.g. animate below.

Copy link
Member

Choose a reason for hiding this comment

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

Relying on type-inference on overloaded functions is asking for trouble. But if this is a case where we flipped the default then it might make sense to stick with the previous order.

Copy link
Contributor

Choose a reason for hiding this comment

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

Are we supposed to ECheckType every "object" argument in overloaded functions?

This seems really annoying to use =/

el.scrollIntoView(({behavior: SMOOTH, block: NEAREST} :js.html.ScrollIntoViewOptions));

Copy link
Member Author

@haxiomic haxiomic Mar 4, 2019

Choose a reason for hiding this comment

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

The order comes from the WebIDLs but I could try to come up with a heuristic to pick the 'best' primary signature
(relevant part of the generator: https://github.com/HaxeFoundation/html-externs/blob/master/bin/Haxe.py#L990)

Copy link
Member

Choose a reason for hiding this comment

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

But that is already typed. How is type-inference even involved here? A small example would probably help.

Copy link
Contributor

@kLabz kLabz Mar 4, 2019

Choose a reason for hiding this comment

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

Sorry, it's probably a vocabulary issue from me. And also some wrong guesses (animate seems fine for example).

I cannot do this:
el.scrollIntoView({behavior: SMOOTH, block: NEAREST}); without adding the ECheckType. I get an Unknown identifier : SMOOTH error.

Edit: try-haxe example

Copy link
Member

Choose a reason for hiding this comment

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

Ah I see. That's probably something we should try to fix in the compiler, but I'm not sure if that's really possible with our overload architecture. Please open an issue about this, preferable with an example that doesn't require any JS externs.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, will do!

Copy link
Member Author

@haxiomic haxiomic Mar 4, 2019

Choose a reason for hiding this comment

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

In the most recent versions of the compiler, EitherType seems to be better at inference (resolves this specific case) so I could switch to that in the externs (try-haxe)

However I guess the best solution would be to somehow have the same level of inference for overloads

(I actually initially switched to overloads for these cases because it had better type inference than EitherType)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform-javascript Everything related to JS / JavaScript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants