-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
chore(gatsby): convert gatsby-link to typescript #37543
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,8 @@ | ||
{} | ||
{ | ||
"presets": [["babel-preset-gatsby-package"]], | ||
"overrides": [ | ||
{ | ||
"presets": [["babel-preset-gatsby-package", { "browser": true }]] | ||
} | ||
] | ||
} |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,17 @@ | ||
import * as React from "react" | ||
import { NavigateFn, LinkProps } from "@reach/router" // These come from `@types/reach__router` | ||
|
||
declare global { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The types should be autogenerated from the source now and the manual file should no longer exist |
||
/* eslint-disable @typescript-eslint/interface-name-prefix */ | ||
interface Window { | ||
___push: (to: string) => void | ||
___replace: (to: string) => void | ||
___navigate: (to: string, options?: NavigateOptions<{}>) => void | ||
___loader: { enqueue: (arg0: string) => {}, hovering: (arg0: string) => {} } | ||
} | ||
/* eslint-disable @typescript-eslint/interface-name-prefix */ | ||
} | ||
|
||
// eslint-disable-next-line @typescript-eslint/naming-convention | ||
export interface GatsbyLinkProps<TState> extends LinkProps<TState> { | ||
/** A class to apply when this Link is active */ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,49 +8,81 @@ import { | |
createMemorySource, | ||
createHistory, | ||
LocationProvider, | ||
NavigateOptions, | ||
} from "@reach/router" | ||
import { Link, navigate, withPrefix, withAssetPrefix } from "../" | ||
|
||
type IntersectionObserverType = jest.Mock< | ||
{ | ||
observe: (ref: Record<string, unknown>) => void | ||
unobserve: (ref: Record<string, unknown>) => void | ||
disconnect: () => void | ||
trigger: (ref: Record<string, unknown>) => void | ||
}, | ||
[cb: any] | ||
> | ||
|
||
interface ICustomNodeJsGlobal extends NodeJS.Global { | ||
__BASE_PATH__: string | undefined | ||
__PATH_PREFIX__: string | undefined | ||
___navigate: ( | ||
to: string, | ||
options?: NavigateOptions<Record<string, unknown>> | ||
) => void | ||
___loader: { | ||
enqueue: (arg0: string) => Record<string, unknown> | ||
hovering?: (arg0: string) => Record<string, unknown> | ||
} | ||
IntersectionObserver: IntersectionObserverType | ||
} | ||
|
||
declare const global: ICustomNodeJsGlobal | ||
|
||
beforeEach(() => { | ||
global.__BASE_PATH__ = `` | ||
global.__PATH_PREFIX__ = `` | ||
}) | ||
|
||
afterEach(cleanup) | ||
|
||
// eslint-disable-next-line @typescript-eslint/explicit-function-return-type | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This PR has a lot of eslint-disable for the explicit function return type. We have that linting rule for a reason 😅 Please remove all instances of that eslint-disable and add the return types |
||
const getWithPrefix = (pathPrefix = ``) => { | ||
global.__BASE_PATH__ = pathPrefix | ||
return withPrefix | ||
} | ||
|
||
// eslint-disable-next-line @typescript-eslint/explicit-function-return-type | ||
const getInstance = (props, pathPrefix = ``) => { | ||
getWithPrefix()(pathPrefix) | ||
return <Link {...props} /> | ||
} | ||
|
||
// eslint-disable-next-line @typescript-eslint/explicit-function-return-type | ||
const getNavigate = () => { | ||
global.___navigate = jest.fn() | ||
return navigate | ||
} | ||
|
||
const getWithPrefix = (pathPrefix = ``) => { | ||
global.__BASE_PATH__ = pathPrefix | ||
return withPrefix | ||
} | ||
|
||
// eslint-disable-next-line @typescript-eslint/explicit-function-return-type | ||
const getWithAssetPrefix = (prefix = ``) => { | ||
global.__PATH_PREFIX__ = prefix | ||
return withAssetPrefix | ||
} | ||
|
||
const setup = ({ sourcePath = `/`, linkProps, pathPrefix = `` } = {}) => { | ||
// eslint-disable-next-line @typescript-eslint/explicit-function-return-type | ||
const setup = ({ sourcePath = `/`, pathPrefix = ``, linkProps = {} } = {}) => { | ||
const intersectionInstances = new WeakMap() | ||
// mock intersectionObserver | ||
global.IntersectionObserver = jest.fn(cb => { | ||
const instance = { | ||
observe: ref => { | ||
observe: (ref: Record<string, unknown>): void => { | ||
intersectionInstances.set(ref, instance) | ||
}, | ||
unobserve: ref => { | ||
unobserve: (ref: Record<string, unknown>): void => { | ||
intersectionInstances.delete(ref) | ||
}, | ||
disconnect: () => {}, | ||
trigger: ref => { | ||
disconnect: (): void => {}, | ||
trigger: (ref: Record<string, unknown>): void => { | ||
cb([ | ||
{ | ||
target: ref, | ||
|
@@ -62,19 +94,20 @@ const setup = ({ sourcePath = `/`, linkProps, pathPrefix = `` } = {}) => { | |
|
||
return instance | ||
}) | ||
|
||
global.__BASE_PATH__ = pathPrefix | ||
const source = createMemorySource(sourcePath) | ||
const history = createHistory(source) | ||
|
||
const utils = render( | ||
<LocationProvider history={history}> | ||
<Link | ||
{...linkProps} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why was that change necessary? |
||
to="/" | ||
className="link" | ||
style={{ color: `black` }} | ||
activeClassName="is-active" | ||
activeStyle={{ textDecoration: `underline` }} | ||
{...linkProps} | ||
> | ||
link | ||
</Link> | ||
|
@@ -386,7 +419,7 @@ describe(`ref forwarding`, () => { | |
}) | ||
|
||
it(`handles a RefObject (React >=16.4)`, () => { | ||
const ref = React.createRef(null) | ||
const ref = React.createRef() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why was that change necessary? |
||
setup({ linkProps: { ref } }) | ||
|
||
expect(ref.current).toEqual(expect.any(HTMLElement)) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,16 +8,24 @@ describe(`isLocalLink`, () => { | |
expect(isLocalLink(`https://www.gatsbyjs.com`)).toBe(false) | ||
}) | ||
it(`returns undefined if input is undefined or not a string`, () => { | ||
// eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
// @ts-ignore purposefully wrong | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can use |
||
expect(isLocalLink(undefined)).toBeUndefined() | ||
// eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
// @ts-ignore purposefully wrong | ||
expect(isLocalLink(-1)).toBeUndefined() | ||
}) | ||
// TODO(v5): Unskip Tests | ||
it.skip(`throws TypeError if input is undefined`, () => { | ||
// eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
// @ts-ignore error case | ||
expect(() => isLocalLink(undefined)).toThrowError( | ||
`Expected a \`string\`, got \`undefined\`` | ||
) | ||
}) | ||
it.skip(`throws TypeError if input is not a string`, () => { | ||
// eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
// @ts-ignore error case | ||
expect(() => isLocalLink(-1)).toThrowError( | ||
`Expected a \`string\`, got \`number\`` | ||
) | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,11 +1,23 @@ | ||||||
import { rewriteLinkPath } from "../rewrite-link-path" | ||||||
import type { TrailingSlash } from "gatsby-page-utils" | ||||||
|
||||||
import { rewriteLinkPath, type rewriteLinkPathType } from "../rewrite-link-path" | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
We don't use the "inline type" syntax (or however you might want to call that) |
||||||
|
||||||
interface ICustomNodeJsGlobal extends NodeJS.Global { | ||||||
__TRAILING_SLASH__: TrailingSlash | undefined | string | ||||||
__PATH_PREFIX__: string | undefined | ||||||
} | ||||||
|
||||||
declare const global: ICustomNodeJsGlobal | ||||||
|
||||||
beforeEach(() => { | ||||||
global.__TRAILING_SLASH__ = `` | ||||||
global.__PATH_PREFIX__ = undefined | ||||||
}) | ||||||
|
||||||
const getRewriteLinkPath = (option = `legacy`, pathPrefix = undefined) => { | ||||||
const getRewriteLinkPath = ( | ||||||
option: string = `legacy`, | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of a |
||||||
pathPrefix: string | undefined = undefined | ||||||
): rewriteLinkPathType => { | ||||||
global.__TRAILING_SLASH__ = option | ||||||
global.__PATH_PREFIX__ = pathPrefix | ||||||
return rewriteLinkPath | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file change is not necessary since we use
microbundle
to generate the output.You can check out https://github.com/gatsbyjs/gatsby/tree/master/packages/gatsby-script to see how it's used (also has relevant
package.json
bits)