preactjs / preact-router

:earth_americas: URL router for Preact.
http://npm.im/preact-router
MIT License
1.01k stars 156 forks source link

Standard Element.matches API overridden on router element #463

Open turjmner8 opened 10 months ago

turjmner8 commented 10 months ago

It looks like the current implementation of preact-router breaks the standard Element API on the router element itself. The router's logical 'matches' are stored as the prop 'matches' on the router element, and any JS that tries to call the standard Element.matches function on that element does not work as expected.

https://www.w3schools.com/jsref/met_element_matches.asp

If possible, it would be better to store the logical 'matches' under a different name to avoid conflicts with the standard Element API.

rschristian commented 10 months ago

Can you provide a situation in which this is actually an issue?

turjmner8 commented 10 months ago

I work on a component library that has various container components that walk their child DOM (and sometimes parent DOM) to find specific elements via the 'matches' Element API. A customer recently logged a bug against us where this logic was falling over, and we tracked this down to their use of preact-router on their page.

Uncaught (in promise) TypeError: element.matches is not a function

turjmner8 commented 10 months ago

We were able to get our customer to work around this by wrapping the contents of their Router in a simple Component. If someone is using a setup with a regular HTMLElement as a child of their Router, then the cloned HTMLElement has its 'matches' function overridden. If they instead use a Component as the child, whatever HTMLElement that component actually produces does not get affected, and no issue is seen.

Another possible solution for this could be to only copy the 'matches' props down if the child being cloned is a Component and not a plain HTMLElement.

For example, this fiddle example provided in the preact-router does not have any issues: https://jsfiddle.net/developit/qc73v9va/

But the div used as the 'demo fallback route' in this sample code would fall over if any JS tries to call 'matches' on the div actually rendered by the Router for that route: https://github.com/preactjs/preact-router#active-matching--links