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

Use webidl2js named/indexed properties everywhere #1129

Open
8 of 10 tasks
domenic opened this issue May 11, 2015 · 6 comments
Open
8 of 10 tasks

Use webidl2js named/indexed properties everywhere #1129

domenic opened this issue May 11, 2015 · 6 comments

Comments

@domenic
Copy link
Member

domenic commented May 11, 2015

Now that we have support for named and indexed properties, we should figure out how to use them everywhere---while not regressing performance.

Existing collections implemented in jsdom, to update

There's also Window, but that's a whole bunch more work, as it needs a special named properties object, and performance is likely to be even more delicate.

New collections we can now implement

@Joris-van-der-Wel
Copy link
Member

Some are marked as [OverrideBuiltins] http://www.w3.org/TR/WebIDL/#OverrideBuiltins:

  • HTMLFormElement
  • DOMStringMap
  • Document

(but not HTMLCollection and Window).

This means that the named properties will shadow any property (e.g. toString, attributes, firstChild, proto). This will break a lot. For this to work, jsdom must only use getters/setters/methods that are not exposed (for example, everything protected by symbols).

IMO, this part of the spec is completely insane and I would rather pretend [OverrideBuiltins] does not exist. Even if the browsers all implement it this way.

@Joris-van-der-Wel
Copy link
Member

I think it is okay to do it for DOMStringMap though, it has no default properties except for what Object provides. So it would be kinda like {}

@domenic
Copy link
Member Author

domenic commented May 14, 2015

Yeah, I'd be happy leaving [OverrideBuiltins] as a TODO for the far future when none of jsdom goes through the public interface of any other part of jsdom.

@domenic domenic changed the title Use NamedPropertiesTracker everywhere Use webidl2js named properties everywhere Aug 20, 2017
@domenic
Copy link
Member Author

domenic commented Aug 20, 2017

Renamed and updated OP in light of #1934!

@domenic domenic changed the title Use webidl2js named properties everywhere Use webidl2js named/indexed properties everywhere Aug 21, 2017
@TimothyGu
Copy link
Member

An update: NodeList and HTMLCollection now use webidl2js. NamedNodeMap (used for element.attributes) was moved to webidl2js, but Proxy performance was then found to be too slow for NamedNodeMap. DOMTokenList has been implemented in jsdom, but has not been transitioned yet. It's on my radar though. Additionally, DOMStringMap (used for element.dataset) and HTMLOptionsCollection (a la selectElement.options) have been implemented.

@domenic
Copy link
Member Author

domenic commented Aug 21, 2017

Thanks, updated OP further. I misremembered about NodeList.

TimothyGu added a commit to TimothyGu/jsdom that referenced this issue Aug 21, 2017
TimothyGu added a commit to TimothyGu/jsdom that referenced this issue Sep 11, 2017
TimothyGu added a commit to TimothyGu/jsdom that referenced this issue Sep 16, 2017
TimothyGu added a commit to TimothyGu/jsdom that referenced this issue Sep 25, 2017
domenic pushed a commit to TimothyGu/jsdom that referenced this issue Sep 30, 2017
domenic pushed a commit to TimothyGu/jsdom that referenced this issue Sep 30, 2017
domenic pushed a commit to TimothyGu/jsdom that referenced this issue Sep 30, 2017
domenic pushed a commit to TimothyGu/jsdom that referenced this issue Sep 30, 2017
domenic pushed a commit to TimothyGu/jsdom that referenced this issue Sep 30, 2017
domenic pushed a commit that referenced this issue Sep 30, 2017
domenic pushed a commit that referenced this issue Nov 19, 2017
This also switches the ownership of "attribute list" and the associated
cache to Element, which is a more literal reading of the spec and
simplifies things here in jsdom.

Part of #1129.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants