4 Replies
Cleaned it up to:
There's no need to cast
quoteList
to HTMLElement | null
typescript can infer that itself
Same goes for quotes
Instead of doing plum.querySelector('svg')
and casting the return, you can do plum.getElementByTagName('svg')
for the same reason
Don't cast element
and card
(still the same reason)
Instead of if (!(element.tagName === "A")) return;
you can put what follows inside the if (and remove the !(...)
Instead of document . querySelecto rAll(
"section#quotes ul li")
you can do quoteList?.getElementsByTagName('li')
So basically:
- remove the type casts (you shouldn't need to use them).
- don't use negative guard closes if that is the only thing in the block
- use the most appropriate DOM query method for what you need (that has the double advantage of making the code clearer AND it tends to be more efficient (even if the performance differential here is pretty much inexistent))I would say that sometimes you do need to cast some elements, but stay consistent in how you do. Sometimes you use the
as
keyword while other times you declare a variable and the type all at once. Try to be as consistent as possible.
You also don't need to query section#quotes
since id's are, by definition, unique. So you can simply do document.querySelector("#quotes")
.
You can try to use the ?
operator to skip one explicit check. And since you don't actually use the card
except for further querying down the DOM you can use method chaining. Normally, I would prefer early return to avoid nesting but since there's only a single instruction it makes sense to put all the conditions under the same if
statement.
Thank you so much guys! This is exactly why I asked. I knew my JS and TS needed a lot of work haha. I'll try this stuff out when I get on my computer 🙂