Duplicated Ids!!! 😲
I notice in the comments there is mention of more than 1 element with the same Id
and that getElementById
will only return one element, which is correct for the majority of browsers depending on the version.
Even if getElementById
acted the same across browsers As a developer intentionally using duplicated Id
is VERY BAD! as it will force the browser into Quirks Mode
effecting performance, layout and can effect how search engine bots crawl your site.
Though Google does say...
“Although we do recommend using valid HTML, it’s not likely to be a factor in how Google crawls and indexes your site.”
... from SEJ 6 Reasons Why Google Says Valid HTML Matters that links to Google Browser Compatibility support page
This is not a guarantee and not something you should trust your sites SEO on.
The simple and safe rule is. NEVER duplicate Id's!!!
Your code
Ignoring the above and looking at your code without regard to the DOM and that getElementById
can return an array you can improve the code (readability and maintainability [1.]) as follows
function queryDOM(selector) {
const VALIDATOR = /[a-z0-9!@#$%^&*)(+=._-]/i, NS = "http://www.w3.org/1999/xhtml";
const type = selector[0], query = selector.slice(1);
if (type === ".") { return [...document.getElementsByClassName(query)] }
if (type === "#") { return [document.getElementById(query)] }
if (! VALIDATOR.test(selector)) { throw new RangeError('Invalid Selector') }
return [...document.getElementsByTagNameNS(NS, selector)];
}
or for the code newbies that need the spacing
function queryDOM(selector) {
const VALIDATOR = /[a-z0-9!@#$%^&*)(+=._-]/i;
const NS = "http://www.w3.org/1999/xhtml";
const type = selector[0];
const query = selector.slice(1);
if (type === ".") {
return [...document.getElementsByClassName(query)];
}
if (type === "#") {
return [document.getElementById(query)];
}
if (! VALIDATOR.test(selector)) {
throw new RangeError("Invalid Selector");
}
return [...document.getElementsByTagNameNS(NS, selector)];
}
Reasoning
- As a function declaration rather than a function expression so that the function is accessible at any time after parsing.
- Extract the first character and remaining query in one spot to avoid logic/code duplication.
- Remove the redundant
else
and supporting code to reduce noise.
- Use ES6+ spread operator
...
(AKA spread syntax) to create arrays from iteratable objects.
- Use the simpler bracket access for single character in string rather than the more verbose
String.startsWith
which is meant for sub strings longer than one character.
- Use
String.slice
rather than String.substring
- Renamed function to hold to camel-case convention of upperCase acronyms and rearranging to stop clash with JS convention of PascalCase only functions used with the new token (Note your function will work as new DOMSelector())
- Renamed argument to not imply its an array (plurals only for arrays or array like objects)
- Throw a range error more suited to the error type (Really should not be throwing for this type of issue)
- Remove the global flag from the
RegExp
- Removed redundant
^
, $
, +
and back slashes inside the []
from RegExp
- Added case insensitive flag to
RegExp
Declared RegExp
as a constant to take out of the statement to make it more readable.
As you include capitals in the validation of the selector I assume you are unaware that getElementsByTagName
first converts the query string to lowercase. Thus I changed the function to use getElementsByTagNameNS
The getElementsByTagNameNS
which namespace (NS) to use is not known from your code and thus the example assumes "http://www.w3.org/1999/xhtml". Warning quirks mode will effect the namespace.
Additionally
The function has a failure if the selector argument is not a string. As the function does throw I assume you only use it inside a try catch
block and you handle the error when it occurs.
Why the complications!
As your code relies on some assumptions that are flawed and do not compile with valid HTML I can not approve the above as worthy of anything but as an example of what not to do.
The code should use querySelectorAll
and covert its iteratable return to an array. I would also drop the "DOM"
in the name (what else could you be querying?) and include a node to focus the query.
Also removing the validation as query string can be used to search attributes that may conflict with the validation.
function query(str, node = document) { return [...node.querySelectorAll(str)] }
Or if you control code placement and use the "use strict"
directive you can use an arrow function expression to assign a named constant to prevent accidental overwriting of the function.
"use strict"; // << Must be at top of code.
const query = (str, node = document) => [...node.querySelectorAll(str)];
This is not at all replacement of jQuery
BTW
A reminder. NEVER duplicate Id's!!!
See I added 3 exclamation points and made it bold to indicate how important this warning is.
Notes
[1.] Readability and maintainability are highly subjective quantities and as such reflex my view only. I have 40Years of experience (35 as a professional) if that counts for anything.
querySelectorAll
? \$\endgroup\$