Skip to content

Update sources #893

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
wants to merge 13 commits into from
Closed

Update sources #893

wants to merge 13 commits into from

Conversation

MattiasBuelens
Copy link
Contributor

@MattiasBuelens MattiasBuelens commented Aug 4, 2020

This collects the latest updates to the IDL definitions and MDN API descriptions.

Some tooling changes were necessary to deal with a few updated specifications:

In order to facilitate review, I've done two baseline update commits: first after running fetch-idl, and then after running fetch-mdn. If you prefer, I can squash these into one commit.

@@ -11081,28 +11122,60 @@ declare var Notification: {
interface OES_element_index_uint {
}

declare var OES_element_index_uint: {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, I suppose we would first need #873 to land? These now have [LegacyNoInterfaceObject] in their IDL...

Copy link
Member

Choose a reason for hiding this comment

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

#873 is now merged.

@MattiasBuelens MattiasBuelens mentioned this pull request Aug 4, 2020
@MattiasBuelens MattiasBuelens marked this pull request as draft August 6, 2020 22:03
Copy link
Contributor Author

@MattiasBuelens MattiasBuelens left a comment

Choose a reason for hiding this comment

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

Rebased and reworked! 😁

Note that I cheated a bit with the "Update IDL" commit: I updated all IDLs except for Streams. These need more tweaking and cleanup in our overrides, which I'm doing in #890. 😉

@@ -15,6 +15,7 @@ export const baseTypeConversionMap = new Map<string, string>([
["object", "any"],
["sequence", "Array"],
["record", "Record"],
["undefined", "void"],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Web IDL has switched from void to undefined, see whatwg/webidl#60 and whatwg/webidl#906. Many specifications have already made the switch, so we need to support this.

For now, I'm mapping this back to TypeScript's void type. However, we could also move this into sameTypes and actually emit undefined in the generated type definitions. Thoughts? 🙂

Copy link
Member

Choose a reason for hiding this comment

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

I like this change except in unions. There is one in CustomElementRegistry:

    get(name: string): CustomElementConstructor | void;

You can't use ?. from the value returned by the accessor. This is admittedly a failing of Typescript, but T | void is rare type for all T.

Not sure whether it's easier to fix the emitter or to add an overridingType entry for the accessor

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can add a special case in the emitter to replace void with undefined if it appears in a union type. Right now, it's just for this one method in CustomElementRegistry, but maybe similar methods pop up in the future for which this would apply. 🙂

@@ -3096,7 +3092,7 @@
"typedefs": {
"typedef": [
{
"override-type": "Blob | BufferSource | FormData | URLSearchParams | ReadableStream<Uint8Array> | string",
"override-type": "ReadableStream<Uint8Array> | XMLHttpRequestBodyInit",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

BodyInit is now defined as:

typedef (Blob or BufferSource or FormData or URLSearchParams or USVString) XMLHttpRequestBodyInit;

typedef (ReadableStream or XMLHttpRequestBodyInit) BodyInit;

This change re-aligns our override-type with the new definition.

@@ -108,6 +108,10 @@
"url": "https://www.w3.org/TR/css-overscroll-1/",
"title": "CSS Overscroll Behavior"
},
{
"url": "https://www.w3.org/TR/css-pseudo-4/",
"title": "CSS Pseudo-Elements"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is being used by the Web Animation specification, so we need to include its types. Fortunately, there aren't that many new types.

Copy link
Contributor

Choose a reason for hiding this comment

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

Neither Chrome nor Firefox provides a stable CSSPseudoElement implementation, so I would just suppress this type.

@MattiasBuelens MattiasBuelens marked this pull request as ready for review September 10, 2020 20:58
@@ -15,6 +15,7 @@ export const baseTypeConversionMap = new Map<string, string>([
["object", "any"],
["sequence", "Array"],
["record", "Record"],
["undefined", "void"],
Copy link
Member

Choose a reason for hiding this comment

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

I like this change except in unions. There is one in CustomElementRegistry:

    get(name: string): CustomElementConstructor | void;

You can't use ?. from the value returned by the accessor. This is admittedly a failing of Typescript, but T | void is rare type for all T.

Not sure whether it's easier to fix the emitter or to add an overridingType entry for the accessor

"DOMTokenList": "The DOMTokenList interface represents a set of space-separated tokens. Such a set is returned by Element.classList, HTMLLinkElement.relList, HTMLAnchorElement.relList, HTMLAreaElement.relList, HTMLIframeElement.sandbox, or HTMLOutputElement.htmlFor. It is indexed beginning with 0 as with JavaScript Array objects. DOMTokenList is always case-sensitive.",
"DataTransfer": "The DataTransfer object is used to hold the data that is being dragged during a drag and drop operation. It may hold one or more data items, each of one or more data types. For more information about drag and drop, see HTML Drag and Drop API.",
"DataTransferItem": "The DataTransferItem object represents one drag data item. During a drag operation, each drag event has a dataTransfer property which contains a list of drag data items. Each item in the list is a DataTransferItem object.",
"DataTransferItemList": "The DataTransferItemList object is a list of DataTransferItem objects representing items being dragged. During a drag operation, each DragEvent has a dataTransfer property and that property is a DataTransferItemList.",
"DedicatedWorkerGlobalScope": "The DedicatedWorkerGlobalScope object (the Worker global scope) is accessible through the self keyword. Some additional global functions, namespaces objects, and constructors, not typically associated with the worker global scope, but available on it, are listed in the JavaScript Reference. See also: Functions available to workers.",
"DelayNode": "The DelayNode interface represents a delay-line; an AudioNode audio-processing module that causes a delay between the arrival of an input data and its propagation to the output.",
"DeviceAcceleration": "A DeviceAcceleration object provides information about the amount of acceleration the device is experiencing along all three axes.",
"DeviceAcceleration": "REDIRECT DeviceMotionEventAcceleration",
Copy link
Member

Choose a reason for hiding this comment

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

can this be turned into @deprecated instead of, or in addition to, REDIRECT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could change mergeApiDescriptions() to first check if the comment from MDN starts with "REDIRECT", and if so ignore the comment and mark it as deprecated instead.

On another note, is there a reason why Interface does not have a deprecated flag? I kind of expected markAsDeprecated() to also mark the entire interface as deprecated.

"webkitanimationend": Event;
"webkitanimationiteration": Event;
"webkitanimationstart": Event;
"webkittransitionend": Event;
Copy link
Member

Choose a reason for hiding this comment

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

this is probably fine, but it's slightly weird to me that webkit* event handlers are being added. Isn't that webkit-specific?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Apparently these have been added to the HTML standard itself, see whatwg/html#5188. 🤷

Should I add them to removedTypes.json? I see there's already a couple of WebKit-specific removals in there.

@saschanaz
Copy link
Contributor

Are we confident that this won't introduce any nonexistent or highly experimental APIs? Updating every spec by a single PR makes hard to check them.

@MattiasBuelens
Copy link
Contributor Author

Are we confident that this won't introduce any nonexistent or highly experimental APIs? Updating every spec by a single PR makes hard to check them.

Hmm, good point. I basically ran npm run fetch-idl and npm run fetch-mdn, thinking that this was the usual process. I have not investigated whether all of these new APIs are actually "stable" yet.

I suppose it would be safer to land the tooling changes first, and then create separate PRs for each updated specification, each with their own review?

@MattiasBuelens
Copy link
Contributor Author

Closing this PR in favor of #920, which contains only the tooling changes.

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