Fix @reach/router Link base type
#3546
Conversation
| @@ -71,7 +71,7 @@ declare module '@reach/router' { | |||
| |}>; | |||
|
|
|||
| declare export type LinkProps<State> = { | |||
| ...$Shape<HTMLAnchorElement>, | |||
| ...React$ElementProps<string>, | |||
There was a problem hiding this comment.
Why not ...React$ElementProps<'a'>?
There was a problem hiding this comment.
@reach/router spreads Link props over <a/> element: https://github.com/reach/router/blob/master/src/index.js#L385-L398
There was a problem hiding this comment.
My thought was to make it more generic. I could imagine that Link component would render another element like span.
There was a problem hiding this comment.
@reach/router is very accessibility-centric, so I don't think they will ever allow using spans as links
There was a problem hiding this comment.
They actually even advertise using native links as a feature =)
https://reach.tech/router/accessibility
This comment has been minimized.
This comment has been minimized.
|
Doesn't feel like the right fix.
|
|
@pascalduez what would be the right one? |
Oh I see. Looks like Flow doesn't validate DOM props at all: It should probably be fixed with facebook/flow#7569 |
I guess |
|
What's missing to be able to move here, I guess adding tests to assert the proper intended behaviour? |
Other notes: This change fixes the problem when
flow@1.106.xexpects all props ofHTMLAnchorElementto be present in aLinkinstance.