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

Allow setting the Fallback via global #29

Closed
fregante opened this issue Dec 3, 2022 · 11 comments
Closed

Allow setting the Fallback via global #29

fregante opened this issue Dec 3, 2022 · 11 comments

Comments

@fregante
Copy link
Contributor

fregante commented Dec 3, 2022

The current solution to change the fallback from Element to HTMLElement is to copy the contents of the shim and make a change, like I did in https://github.com/refined-github/refined-github/blob/71c16f38d5d7853e57bd3e36852175b30eed89c3/source/helpers/types.d.ts#LL7

This also currently isn't possible in the strict parser:

What do you think about using a global like SelectorFallback in this module so that I can override it globally by just using:

// global.d.ts

declare SelectorFallback = HTMLElement;
@g-plane
Copy link
Owner

g-plane commented Dec 5, 2022

If I remember, TypeScript wouldn't allow you to re-declare something.

@g-plane
Copy link
Owner

g-plane commented Feb 21, 2023

After https://github.com/g-plane/typed-query-selector/releases/tag/v2.9.0 , you don't need that helper to shim HTMLElement any more.

@fregante
Copy link
Contributor Author

In the end I didn't use this because I had altered my types to accept arrays of selectors too: el.closest(['a', 's']) which is technically wrong, but it works (because ['a', 's'].toString() === 'a,s')

https://github.com/refined-github/refined-github/blob/714f51c81394b2186c40b5ee1248663f12293fe5/source/helpers/types.d.ts/#L10

However 4534a0c isn't technically correct because any HTML5 document can still contain SVG tags. Try this on https://developer.mozilla.org/en-US/

document.querySelector('svg') instanceof HTMLElement // => false
document.querySelector('svg') instanceof SVGElement // => true

@fregante
Copy link
Contributor Author

In practice however this only happens when both of these are true:

  • typed-query-selector fails to parse the selector (or is dynamic)
  • the user actually selects an element inside SVG (rare)

@g-plane
Copy link
Owner

g-plane commented Feb 21, 2023

Oh, I didn't realize that. You're right, but we can leave it and re-consider it once we receive feedback about the types.

@stof
Copy link

stof commented Mar 15, 2023

This also happens if the user selects inside MathML (which does not use HTMLElement either).

It would be great to move those "unsafe" fallbacks to a separate shim file to make them opt-in

@stof
Copy link

stof commented Apr 14, 2023

@g-plane is there any plan to split those unsafe fallbacks to a separate shim ?

@fregante
Copy link
Contributor Author

It's better to revert this change and default to Element for correctness

@stof
Copy link

stof commented Apr 14, 2023

I'm also fine with a complete revert, as that would bring back correctness

@g-plane
Copy link
Owner

g-plane commented Apr 17, 2023

It's reverted in v2.10.0.

@stof
Copy link

stof commented Apr 17, 2023

Thanks

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

No branches or pull requests

3 participants