From 6f7d6c01da4e31c215bc6424c0b732f739851adb Mon Sep 17 00:00:00 2001 From: Eyas Date: Fri, 10 Apr 2020 22:34:10 -0400 Subject: [PATCH] react-router-dom: Correct type for `ref` in Link (#43428) * no-op: Run prettier --write on react-router-dom. * Allow correct "ref" to be passed Link. "ref" can be used rather than "innerRef" to return a ref to the element inside of . See: [1]: reacttraining.com/react-router/web/api/Link/innerref-function Where it says: > As of React Router 5.1, if you are using React 16 you should not > need this prop because we forward this ref to the underlying . > Use a normal ref instead.Allows access to the underlying ref of the > component. [2]: https://github.com/ReactTraining/react-router/blob/master/packages/react-router-dom/modules/Link.js#L74-L121 Link is the result of forwardRef rather than an actual React class. This implies a few things: 1. It is a function (see the return type of React.ForwardRefExoticComponent) rather than a class 2. Its reference is to an inner element, rather than what's in React.Component, which is a Ref to the element itself. This introduces a potential incompatibility where users can't use "new Link" anymore. But that reflects reality (Link is callable not newable). Since Link was previously exported as a class, we need to continue to support it being used as both a type and a value. Since Link was exported as a generic, we need to continue to support that. As a result: 1. Link is exported as a function that happens to implement the type corresponding to the result of `forwardRef>`. 2. Link is still exported as a type (interface) which is the same as that of the return value of `forwardRef<...>`. * Make sure Link can be used as a generic. This is is only supported in TS >= 2.9. I bumped the minimum TS supported version to 2.9 since 2.8 support ends this month anyway. * Apply prettier --write to tests. * Apply same change (forwardRef) to NavLink. 1. https://reacttraining.com/react-router/web/api/NavLink NavLink is a version of with the same props. 2. NavLink is similarly implemented in terms of forwardRef, so all the justification from the previous commit applies. See: https://github.com/ReactTraining/react-router/blob/master/packages/react-router-dom/modules/NavLink.js * To maintain TS 2.8 compatibility, use React.createElement to test Link<{foo:number}> and NavLink. When TS 2.8 compatibility is no longer needed, this CL can be reverted as-is. --- types/react-router-dom/index.d.ts | 45 ++++++++++--------- .../react-router-dom-tests.tsx | 25 ++++++++--- 2 files changed, 43 insertions(+), 27 deletions(-) diff --git a/types/react-router-dom/index.d.ts b/types/react-router-dom/index.d.ts index 6cb00d15eb..c6025b5113 100644 --- a/types/react-router-dom/index.d.ts +++ b/types/react-router-dom/index.d.ts @@ -9,7 +9,7 @@ // Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped // TypeScript Version: 2.8 -import { match } from "react-router"; +import { match } from 'react-router'; import * as React from 'react'; import * as H from 'history'; @@ -39,7 +39,7 @@ export { export interface BrowserRouterProps { basename?: string; - getUserConfirmation?: ((message: string, callback: (ok: boolean) => void) => void); + getUserConfirmation?: (message: string, callback: (ok: boolean) => void) => void; forceRefresh?: boolean; keyLength?: number; } @@ -47,7 +47,7 @@ export class BrowserRouter extends React.Component {} export interface HashRouterProps { basename?: string; - getUserConfirmation?: ((message: string, callback: (ok: boolean) => void) => void); + getUserConfirmation?: (message: string, callback: (ok: boolean) => void) => void; hashType?: 'slash' | 'noslash' | 'hashbang'; } export class HashRouter extends React.Component {} @@ -58,23 +58,28 @@ export interface LinkProps extends React.AnchorHTMLAttribut replace?: boolean; innerRef?: React.Ref; } -export class Link extends React.Component< - LinkProps, - any -> {} +export function Link( + // TODO: Define this as ...params: Parameters> when only TypeScript >= 3.1 support is needed. + props: React.PropsWithoutRef> & React.RefAttributes, +): ReturnType>; +export interface Link + extends React.ForwardRefExoticComponent< + React.PropsWithoutRef> & React.RefAttributes + > {} export interface NavLinkProps extends LinkProps { - activeClassName?: string; - activeStyle?: React.CSSProperties; - exact?: boolean; - strict?: boolean; - isActive?( - match: match, - location: H.Location, - ): boolean; - location?: H.Location; + activeClassName?: string; + activeStyle?: React.CSSProperties; + exact?: boolean; + strict?: boolean; + isActive?(match: match, location: H.Location): boolean; + location?: H.Location; } -export class NavLink extends React.Component< - NavLinkProps, - any -> {} +export function NavLink( + // TODO: Define this as ...params: Parameters> when only TypeScript >= 3.1 support is needed. + props: React.PropsWithoutRef> & React.RefAttributes, +): ReturnType>; +export interface NavLink + extends React.ForwardRefExoticComponent< + React.PropsWithoutRef> & React.RefAttributes + > {} diff --git a/types/react-router-dom/react-router-dom-tests.tsx b/types/react-router-dom/react-router-dom-tests.tsx index 0fefcbb195..e55739292e 100644 --- a/types/react-router-dom/react-router-dom-tests.tsx +++ b/types/react-router-dom/react-router-dom-tests.tsx @@ -5,19 +5,17 @@ import * as H from 'history'; const getIsActive = (extraProp: string) => (match: match, location: H.Location) => !!extraProp; interface Props extends NavLinkProps { - extraProp: string; + extraProp: string; } export default function(props: Props) { - const {extraProp, ...rest} = props; - const isActive = getIsActive(extraProp); - return ( - - ); + const { extraProp, ...rest } = props; + const isActive = getIsActive(extraProp); + return ; } type OtherProps = RouteComponentProps<{ - id: string; + id: string; }>; const Component: React.FC = props => { @@ -39,7 +37,20 @@ const MyLink: React.FC = props => = node => {}; ; +; +; +; const ref = React.createRef(); ; +; +; +; ; + +React.createElement & React.RefAttributes>(Link, { + to: { pathname: 'abc', state: { foo: 5 } }, +}); +React.createElement & React.RefAttributes>(NavLink, { + to: { pathname: 'abc', state: { foo: 5 } }, +});