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

Update dom.js: change NodeList to NodeList<HTMLElement> #150

Merged
merged 1 commit into from Dec 3, 2014

Conversation

ptmt
Copy link
Contributor

@ptmt ptmt commented Nov 30, 2014

There is a problem with code like this:

document.querySelectorAll('.selector')[0].style.display = 'block';

because querySelectorAll returns NodeList of Element, not Node https://developer.mozilla.org/en-US/docs/Web/API/element.

I've changed NodeList to NodeList<T> and signatures of some relevant functions:

querySelector(selector: string): HTMLElement;
querySelectorAll(selector: string): NodeList<HTMLElement>;

though HTMLElement is no exactly correct (it could be any of derived classes of Element such as SVGElement, for example) I don't know better solution.
Also, there is inconsistency with HTMLCollection which is collection of Element and will produce error with code sample.

@ptmt
Copy link
Contributor Author

ptmt commented Nov 30, 2014

Sorry for these unserious and unrelevant pull requests, may be #151 helps a bit to avoid such repetitive issues.

@gabelevi
Copy link
Contributor

gabelevi commented Dec 3, 2014

Cool, this looks good to me!

gabelevi added a commit that referenced this pull request Dec 3, 2014
Update dom.js: change NodeList to NodeList<HTMLElement>
@gabelevi gabelevi merged commit 22f9c6e into facebook:master Dec 3, 2014
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.

None yet

2 participants