| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
This provides an alternative to the common pattern of `document.getElementById("...") as HTMLWhateverElement` which casts away the possibility of `null`, consequently hiding a potential type error.
|
Thanks for the PR! This section of the codebase is owned by Kagami Sascha Rosylight (@saschanaz) - if they write a comment saying "LGTM" then it will be merged. |
Sorry, something went wrong.
|
Might be worth pointing out npm run baseline-accept in the "contribution guidelines" 🍞 |
Sorry, something went wrong.
|
I think the guideline has been that, if you are not actually using the type parameter in the signature then it should not exist, and rather you should cast from the return type. Jake Bailey (@jakebailey), is that still the case? |
Sorry, something went wrong.
|
Yes, this effectively a type assertion in disguise. It's not good to have this kind of signature. An explicit assertion is much preferred. |
Sorry, something went wrong.
|
I understand the risks but I don't think this is a good idea. (If we were to have something like this, it'd at least have to be T extends ... = ... |
Sorry, something went wrong.
|
My case for it: querySelector and querySelectorAll have it. It is very common for the T to be a type assertion (e.g. JSON.parse) Also updated to require that T extends HTMLElement |
Sorry, something went wrong.
I have no idea why these have it, I doubt we would choose to do so if the same decision were made today. It's patently unsafe and hides the type assertion from tools that intend to find unsafety.
JSON.parse doesn't do this? Unless you're talking about third party patches? |
Sorry, something went wrong.
tbh I have no idea about the tooling landscape here (my version of Vite is called PHP) but to me a ! seems more explicit than the as ... just casting away the | null
My mistake, the typing I got is from json-with-bigint |
Sorry, something went wrong.
|
querySelector has it because if you do querySelector("input") then you know it's going to be HTMLInputElement. Not much for getElementById. |
Sorry, something went wrong.
|
I pinky promise I know what my .html/.php file looks like 😄 |
Sorry, something went wrong.
|
querySelector has overloads that have nice return types, but then they end with: querySelector<E extends Element = Element>(selectors: string): E | null;
querySelectorAll<E extends Element = Element>(selectors: string): NodeListOf<E>;I'm not sure if that's because it's required to make the overload checks happy, but I don't think so 😦 |
Sorry, something went wrong.
That same argument could apply to all of TS 😄 "I know better than the compiler" is supposed to be casts and !, not type vars... But I understand the tension and the nullness problem here. |
Sorry, something went wrong.
Sorry, something went wrong.
|
For me the added null is a very good thing, With this PR we could migrate to the generics and will get the null for free. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
This provides an alternative to the common pattern of document.getElementById("...") as HTMLWhateverElement which casts away the possibility of null, consequently hiding a potential type error.