Add typings for 'to' property of Gatsby Link

[Nikki1993]

This should fix #3525 (comment)

Based on documentation 'to' property always takes a string. I also prettified the file to be more consistent as certain lines had semicolons, some didn't etc.

[gatsbybot]

Deploy preview for gatsbygram ready!

Built with commit 65cf341

https://deploy-preview-3552--gatsbygram.netlify.com

[KyleAMathews]

to actually can be an object too (recent change — where did you read it's string only as we should fix that) #3407

Perhaps just make the type any? Or if react-router has good typescript typings, we could copy (or can you import types in typescript?) them.

[Nikki1993]

Could simply do type any. Looking at react router revealed custom type that I didnt dig too much into.

The string part came to as I was reading samples on how to use Gatsby link. Nearly all of them had either strings or props that is a string.

[KyleAMathews]

Yeah let's just do any. Most of the time you do only string but there are more advanced use cases where the object form is helpful.

[Nikki1993]

@KyleAMathews done, switched to any.

[KyleAMathews]

Sweet!

Another quick question — does the semicolon matter at all in type files?

[nsimonson]

Originally I removed that explicit type because I incorrectly assumed it would inherit the to prop from LinkProps. any is a pretty weak type. If it has to be explicitly typed it’d be better to use ‘LocationDescriptor‘ from history. That’s a union between a string and the location object.

[KyleAMathews]

@nsimonson that sounds pretty reasonable — how would we do that?

[Nikki1993]

@KyleAMathews in the interface, semicolons are not necessary at all.

[Nikki1993]

@nsimonson I am assuming then you would do smth like this

import * as React from "react"
import { NavLinkProps } from "react-router-dom"
import { LocationDescriptor } from "history";

export interface GatsbyLinkProps extends NavLinkProps {
  onClick?: (event: any) => void
  className?: string
  style?: any
  to: LocationDescriptor
}

export const navigateTo: (path: string) => void

export const withPrefix: (path: string) => string

export default class GatsbyLink extends React.Component<GatsbyLinkProps, any> {}

[KyleAMathews]

@Nikki1993 want to make the change to use locationDescriptor?

[fabien0102]

Normally it's not necessary, it extends NavLinkProps that extends LinkProps .

And LinkProps has already the correct definition for to 😉

// node_modules\@types\react-router-dom\index.d.ts
export interface LinkProps extends React.AnchorHTMLAttributes<HTMLAnchorElement> {
    to: H.LocationDescriptor;
    replace?: boolean;
}
export class Link extends React.Component<LinkProps, any> {}

export interface NavLinkProps extends LinkProps {
    activeClassName?: string;
    activeStyle?: React.CSSProperties;
    exact?: boolean;
    strict?: boolean;
    isActive?<P>(match: match<P>, location: H.Location): boolean;
    location?: H.Location;

[fabien0102]

And for the semicolumns I don't care, we just need to be consistent, actually we have some other typescript files with semicolumns (https://github.com/gatsbyjs/gatsby/blob/master/examples/using-typescript/src/html.tsx for example) so if you want to change the convention, I think that a real PR for this, (with all the typescript files linted with the same linter rules) should be better 😉

[Nikki1993]

@KyleAMathews will do, was without a laptop so PR update inc

@fabien0102 well if the type existed I wouldn't have made the PR 😄 I thought it was odd that 'to' property was missing but that "hack" with LocationDescriptor makes the original error in here #3525 (comment) go away so go figure 😕

[fabien0102]

@Nikki1993 It's very strange, I have used this gatsby link in my starter and the to props works as expected, I have an error when the to props is missing or if I provide a number 😉

Can you try to check on your node_modules with Go to definition to see if the definitions that I pasted are there?

[Nikki1993]

@fabien0102 what the... I have a feeling I might drop yarn off the balcony soon. I reinstalled everything with simple rm -rf node_modules && yarn and the 'to' has a correct definition as it should have from the start.

I am gonna go on a witch hunt and say yarn has been quite unreliable lately, especially when it comes to typings and definitions, especially with reactjs + typescript combo.

Gonna close this as there is no problem with typings but most likely with package manager messing up the types.

[uccmen]

I see that to is still taking in string guys...