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

prefer-query-selector: Add getElementsByName #2398

Merged

Conversation

axetroy
Copy link
Contributor

@axetroy axetroy commented Jul 5, 2024

fixed #2374


// No default
}
};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is safe, but I guess no one put quotes or other strange stuff in the name?

A better fix should direct update the raw string

- ('foo')
+ ('[name="foo"]')
- ("foo")
+ ("[name=\"foo\"]")  // Personally, I like double quoted attribute selector.
- (`foo`)
+ (`[name="foo"]`)

Anyway, if it's hard we can leave it as is for now.

@fregante
Copy link
Collaborator

fregante commented Jul 25, 2024

It's a little complicated and maybe too rare to be worth it, but the correct replacement for form elements would be form.elements.namedItem('foo'), not form.querySelector('[name="foo"]')

https://stackoverflow.com/a/76206952/288906

@silverwind
Copy link
Contributor

silverwind commented Jul 25, 2024

It's a little complicated and maybe too rare to be worth it, but the correct replacement for form elements would be form.elements.namedItem('foo'), not form.querySelector('[name="foo"]')

https://stackoverflow.com/a/76206952/288906

If it's to complicated to autofix, I suggest not having a autofixer, just the detection only. I'd certainly not do this namedItem fix because such elements could appear outside a form too.

@fregante
Copy link
Collaborator

I think it's fine as is for now, querySelector is not a wrong fix for this rule. Perhaps the selections of form items would have to be its own rule because it's kinda complicated

@silverwind
Copy link
Contributor

Yes, I think querySelectorAll is not wrong. Why do you think it's wrong?

@fregante
Copy link
Collaborator

querySelector is not a wrong fix for this rule

I said it's not wrong here, but namedItem() would still be the better API to select form items.

@silverwind
Copy link
Contributor

silverwind commented Jul 25, 2024

Ah I see what you mean now. If you have multiple forms on the page with duplicate name fields in them, then yes, only form.elements.namedItem will work, but for the general use case, querySelectorAll is a suitable replacement for getElementsByName.

@sindresorhus sindresorhus merged commit e511ffd into sindresorhus:main Jul 25, 2024
17 checks passed
@axetroy axetroy deleted the prefer-query-selector_getElementsByName branch July 29, 2024 01:56
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.

prefer-query-selector: Add getElementsByName
5 participants