Introduce composedPath for Events

[43081j]

Fixes #522 and microsoft/TypeScript#17974.

The original PR (#413) missed out one of the two changes it aimed to make.

This'll allow:

event.composedPath();

[msftclas]

All CLA requirements met.

[mhegazy]

Can we add https://dom.spec.whatwg.org/##dom-event-composedpath spec to the generator instead?

[43081j]

@mhegazy would that involve adding Event.widl to the input files? is that what you mean?

im not so familiar with this repo so may need to be pointed in the right direction

[mhegazy]

Sorry, i should have added more info.

We are moving to generate the lib file directelly from widl files. today we have a "base" input file that comes fom the edge team (see https://github.com/Microsoft/TSJS-lib-generator/blob/master/inputfiles/browser.webidl.preprocessed.json) then we add widl files on top of it (see contents of https://github.com/Microsoft/TSJS-lib-generator/tree/master/inputfiles/idl).

To add a new widl file you need to:

1. Edit https://github.com/Microsoft/TSJS-lib-generator/blob/master/inputfiles/idlSources.json to add the new file
2. run npm run fetch to download the new widl file
3. run npm run build to generate the .d.ts file based on the widl file

Now, since we are just starting this process, there is usually some additional work that needs to be done, e.g. adding a missing type, or updating the overrides in https://github.com/Microsoft/TSJS-lib-generator/blob/master/inputfiles/overridingTypes.json based on the addtion. you can find examples in https://github.com/Microsoft/TSJS-lib-generator/pulls?q=is%3Apr+is%3Aclosed+author%3Asaschanaz

Also cc @saschanaz

[43081j]

in this case wouldn't the widl be the entire DOM standard though? (dom.spec.whatwg.org)

if so im happy to have a go

[mhegazy]

@saschanaz have more info. but we can try adding https://dom.spec.whatwg.org and see how that goes.

[43081j]

@saschanaz given something like the last commit, how would you then go about fixing up the missing types?

it seems its because the comments file contains many entries which don't have an associated type in the IDL (they exist in other specs, for example ondragend is in the web APIs IDL which we dont yet have).

[saschanaz]

Yeah... The DOM spec depends on https://html.spec.whatwg.org/multipage/webappapis.html and https://html.spec.whatwg.org/multipage/dom.html. Adding them will generate huge diffs, but will be helpful for future progress.

[saschanaz]

It seems we need a way to selectively accept IDL types to make things more progressive.

[43081j]

yup im happy to throw a bunch of them in at once but it quickly blows up so would be a rather big PR.

[43081j]

on a side note, adding the HTML DOM spec results in an error when trying to resolve exposures...

[Exposed=Window,
 OverrideBuiltins]
interface DOMStringMap {
  getter DOMString (DOMString name);

this results in an anonymous getter which has a null name:

{
  "name": null,
  "signature": [
    {
      "type": "object",
      "param": [
        {
          "name": "name",
          "type": "DOMString"
        }
      ]
    }
  ],
  "getter": 1,
  "exposed": "Window"
}

because of the name: null it seems, we end up trying "exposed" in null here.

i think our check needs to account for the fact that typeof null === "object":

if (typeof obj[key] === "object" && obj[key] !== null)

[saschanaz]

I think #524 can help reducing this PR.

[43081j]

aha yes thats what i had just partly tried to commit.

i had moved a bunch to the GlobalEventHandlers namespace in the comments file. ill wait on #524 in that case

[43081j]

@saschanaz i may need some guidance from you around that.

i've noticed adding an idl file usually results in missing types because comments.json has comments for properties which now exist elsewhere (e.g. GlobalEventHandlers, DocumentOrShadowRoot, etc).

should i just be moving those? like i had in my last commit. and what about ones which don't exist in a spec?

for example this now errors because apparently onmsthumbnailclick can't be found but we have it in both comments.json and addedTypes.

[saschanaz]

should i just be moving those?

Yeah, that would work.

for example this now errors because apparently onmsthumbnailclick can't be found but we have it in both comments.json and addedTypes.

Those types should be simply removed.

[43081j]

ah in actual fact that one exists in the preprocessed file and our comments, but still causes a not found error..

ill wait until the other PR is merged though too so i dont redo the same changes.

[saschanaz]

ah in actual fact that one exists in the preprocessed file and our comments

Probably the comments file is the culprit, maybe removing onmsthumbnailclick from the file will fix it. The preprocessed file should be kept intact, though.

[saschanaz]

Would you try rebasing?

[43081j]

will give it a go.

couple of issues (note to self really):

• addEventListener is being duplicated for some reason
• removeEventListener too
• Many events have lost their specificity (e.g. Event instead of MouseEvent)

[saschanaz]

The event listener issues are fixed by #525. The events one... you have to add them manually as in #526.

[43081j]

there's quite some crossover with those PRs so i will just wait again.

i had already included the DOM spec. let me know and ill rebase if you get that in master

i also fixed a couple of bugs in the emitter on my way

[43081j]

also @saschanaz this currently fails tests

edit: nevermind i sorted this

tests now fail because we don't have UIEvent in webworker.generated.d.ts

[43081j]

@saschanaz any idea what happened here? looks like it duplicated the two signatures in every occurrence...

maybe i missed an added type or something?

[43081j]

FYI it now seems mostly right but:

• webworker is missing UIEvent somehow
• all the tags which use HTMLElement seem to have vanished from the tag map

[mhegazy]

one more rebase

[43081j]

here you go @mhegazy

im reading through the baseline diff just to double check too

[saschanaz]

This looks strange, hmm.

[43081j]

the freaky indentation? that was bothering me too, but couldn't figure out why its this way

[43081j]

the indentation is because we join the comment lines by an indented *, but this comment/statement is top level so shouldn't have any indentation

[43081j]

@saschanaz what about this?

i can revert the last 2 commits if you don't think we should do this.

i basically stripped the forced indentation from comments.json and made the emitter split by newline so each comment line gets printed with the correct indent level (as the printer already handles this).