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

Missing string indexer on NamedNodeMap interface in lib.dom.d.ts? #30928

Open
nickytonline opened this issue Apr 15, 2019 · 3 comments
Open

Missing string indexer on NamedNodeMap interface in lib.dom.d.ts? #30928

nickytonline opened this issue Apr 15, 2019 · 3 comments
Milestone

Comments

@nickytonline
Copy link

@nickytonline nickytonline commented Apr 15, 2019

For context, see sindresorhus/refined-github#1783, specifically this commit, sindresorhus/refined-github@48baae0.

Also, just posting my tweet about this so that people can find discussions about the issue. See https://twitter.com/nickytonline/status/1117524648982392833

I'll restate what I tweeted as I probably should have posted my question here originally.

The NamedNodeMap interface in lib.dom.ts does not allow for a string indexer, even though vanilla JS supports this in browsers, e.g. someDomElement.attribute['aria-label'].value.

We have code like this in the Refined GitHub extension, so for the time being, I've gone ahead via a declaration merge for NamedNodeMap

interface NamedNodeMap {
      [key: string]: Attr;
}

I can't tell from the MDN docs for NamedNodeMap if it's standard or not. All they seem to mention is "Attr nodes' indexes may differ among browsers" which wouldn't apply to access by the attribute name.

Just wondering if this was omitted by mistake or is it because this is not considered WHATWG DOM standard? I went to https://dom.spec.whatwg.org/#interface-namednodemap and unless I'm reading it incorrectly, I believe it states that using a string indexer is valid.

Thoughts? Happy to PR this up if it's valid.

@nickytonline nickytonline changed the title Missing string indexer on NamedNodeMap? Missing string indexer on NamedNodeMap interface in lib.dom.d.ts? Apr 15, 2019
@Jessidhia

This comment has been minimized.

Copy link

@Jessidhia Jessidhia commented Apr 17, 2019

There are two indices specified; a numeric and a string index:

"NamedNodeMap object’s supported property indices are the numbers in the range zero to its attribute list’s size minus one, unless the attribute list is empty, in which case there are no supported property indices."

This is the numeric index; typical ArrayLike<Attr>.

"A NamedNodeMap object’s supported property names are the return value of running these steps:

  1. Let names be the qualified names of the attributes in this NamedNodeMap object’s attribute list, with duplicates omitted, in order.
  2. If this NamedNodeMap object’s element is in the HTML namespace and its node document is an HTML document, then for each name in names:
    1. Let lowercaseName be name, in ASCII lowercase.
    2. If lowercaseName is not equal to name, remove name from names.
  3. Return names."

This is the named string index. It's case-sensitive in SVG documents/elements, forced-lowercase in HTML documents.

We can't represent this in typescript, but only the numeric indices are enumerable. Any string index is enumerable: false. This also means that Object.values would use the wrong overload! Fortunately both indices (where defined) have Attr instances.

@saschanaz

This comment has been minimized.

Copy link
Contributor

@saschanaz saschanaz commented Apr 26, 2019

AFAIK this is intentionally disabled because TS didn't support string indexer when any other member has a different type than the indexer returns. But looks like somehow that behavior changed. Still causes errors when creating new types.

@trusktr

This comment has been minimized.

Copy link

@trusktr trusktr commented Dec 28, 2019

I ran into this today porting some old code relying on the format element.attributes[name].value.

Maybe the string index should actually be

interface NamedNodeMap {
      [key: string]: Attr | undefined;
}

? Otherwise it will thing every access of any string results in Attr, which isn't the case. Should this be applied to number indices too?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.