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

Description of prefer-query-selector is misleading WRT “consistency” #342

Closed
Alhadis opened this issue Aug 5, 2019 · 3 comments · Fixed by #408
Closed

Description of prefer-query-selector is misleading WRT “consistency” #342

Alhadis opened this issue Aug 5, 2019 · 3 comments · Fixed by #408

Comments

@Alhadis
Copy link

Alhadis commented Aug 5, 2019

The description of the prefer-query-selector rule states:

Prefer .querySelectorAll() over .getElementsByClassName() and .getElementsByTagName()
It's better to be consistent.

"Consistency" has nothing to do here. These methods aren't synonymous with each other: .getElementsByTagName() and .getElementsByClassName() return an HTMLCollection object, which updates dynamically as elements are added or removed from the DOM:

const pages = document.getElementsByClassName("page");
console.log(pages.length); // 1

const newPage = Object.assign(document.createElement("div"), {className: "page"});
document.body.appendChild(newPage);
console.log(pages.length) // 2

Conversely, .querySelectorAll() returns a static NodeList which does not reflect changes to the DOM:

const live = document.getElementsByTagName("div");
const dead = document.querySelectorAll("div");
live.length === 3;
dead.length === 3;

document.body.appendChild(document.createElement("div"));
live.length === 4;
dead.length === 3;

The rule — or at least its description — should be amended to acknowledge this.

@sindresorhus
Copy link
Owner

"Consistency" has nothing to do here.

"Consistency" as in always using the same method that behaves the same.

These methods aren't synonymous with each other:

I'm well aware of how they work.


But sure, we can make the difference clearer.

@HERBOD7
Copy link

HERBOD7 commented Sep 28, 2019

@sindresorhus I can do it.

hkulur added a commit to hkulur/eslint-plugin-unicorn that referenced this issue Oct 3, 2019
Removed the word consistent, which seemed to cause confusion
fisker added a commit that referenced this issue Oct 4, 2019
)

* Fix for issue  #342

Removed the word consistent, which seemed to cause confusion

* Update prefer-query-selector.md

* Update docs/rules/prefer-query-selector.md

Co-Authored-By: fisker Cheung <lionkay@gmail.com>
@feedanal
Copy link

This rule basically breaks code that relies on a "live" HTMLCollection returned by .getElementsByTagName() and .getElementsByClassName(). I was under impression that linters are NOT supposed to change code functionality, and prefer-query-selector goes against this idea, maybe I have wrong expectations for linters, dunno.
I can disable it for sure, but why on Earth would someone replace one method with a non-equivalent one??

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

Successfully merging a pull request may close this issue.

4 participants