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

Add webidl2js‑globals.js to automate install of [Exposed] globals #190

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

ExE-Boss
Copy link
Contributor

@ExE-Boss ExE-Boss commented Apr 3, 2020

TODO:

  • Fix the interface order determination algorithm so that Element goes after Node and EventTarget, but before HTMLElement.
  • Improve the efficiency of the interface order determination algorithm.

Depends on:


Supersedes and closes #101.

review?(@TimothyGu)

@ExE-Boss ExE-Boss changed the title Add webidl2js‑globals.js to handle setup of [Global]s Add webidl2js‑globals.js to automate install of [Exposed] interfaces onto [Global]s Apr 4, 2020
@TimothyGu
Copy link
Member

Perhaps we can be a bit less ambitious, and separate out the parts that add [Exposed] support to install() (which I mentioned in #191) from the parts that aggregates globals into the same file? This way we could immediately start using it in jsdom.

WRT an algorithm to figure out the require order, if the dependency graph turns out to be acyclic then topological sorting should work. If it is cyclic then… we’d have to look into other things.

@ExE-Boss
Copy link
Contributor Author

ExE-Boss commented Apr 4, 2020

The dependency order is already enforced to be acyclic in:

// All interfaces an object of this interface implements.
* allInterfaces(seen = new Set([this.name]), root = this.name) {
yield this.name;
if (this.idl.inheritance && this.ctx.interfaces.has(this.idl.inheritance)) {
if (seen.has(this.idl.inheritance)) {
throw new Error(`${root} has an inheritance cycle`);
}
seen.add(this.idl.inheritance);
yield* this.ctx.interfaces.get(this.idl.inheritance).allInterfaces(seen, root);
}
}

@ExE-Boss ExE-Boss changed the title Add webidl2js‑globals.js to automate install of [Exposed] interfaces onto [Global]s Add webidl2js‑globals.js to automate install of [Exposed] globals Apr 4, 2020
@ExE-Boss ExE-Boss marked this pull request as ready for review April 4, 2020 16:41
@TimothyGu
Copy link
Member

I've rebased this, but it doesn't look like it's ready for review yet? Interface files call webidl2jsGlobal.setupGlobal but it's not defined.

@ExE-Boss
Copy link
Contributor Author

ExE-Boss commented Apr 8, 2020

@TimothyGu That’s a leftover from before I renamed it to just installInterfaces, which is more descriptive of what it does.

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.

2 participants