From 4ffe16a0616ada7d15fab448d9d283a6bd2c3f3b Mon Sep 17 00:00:00 2001 From: Joe Haddad Date: Thu, 6 Aug 2020 14:50:22 -0400 Subject: [PATCH 1/5] Replace broken `prop-types-exact` package --- packages/next/client/link.tsx | 139 +++++++++++++++++++++++++--------- packages/next/package.json | 1 - 2 files changed, 103 insertions(+), 37 deletions(-) diff --git a/packages/next/client/link.tsx b/packages/next/client/link.tsx index 33f1affaadb82..2b81b23c35020 100644 --- a/packages/next/client/link.tsx +++ b/packages/next/client/link.tsx @@ -1,15 +1,21 @@ import React, { Children } from 'react' import { UrlObject } from 'url' import { - PrefetchOptions, - NextRouter, + addBasePath, isLocalURL, + NextRouter, + PrefetchOptions, + resolveHref, } from '../next-server/lib/router/router' -import { execOnce } from '../next-server/lib/utils' import { useRouter } from './router' -import { addBasePath, resolveHref } from '../next-server/lib/router/router' type Url = string | UrlObject +type RequiredKeys = { + [K in keyof T]-?: {} extends Pick ? never : K +}[keyof T] +type OptionalKeys = { + [K in keyof T]-?: {} extends Pick ? K : never +}[keyof T] export type LinkProps = { href: Url @@ -20,6 +26,8 @@ export type LinkProps = { passHref?: boolean prefetch?: boolean } +type LinkPropsRequired = RequiredKeys +type LinkPropsOptional = OptionalKeys let cachedObserver: IntersectionObserver const listeners = new Map void>() @@ -145,6 +153,97 @@ function linkClicked( function Link(props: React.PropsWithChildren) { if (process.env.NODE_ENV !== 'production') { + function createPropError(args: { + key: string + expected: string + actual: string + }) { + return new Error( + `Failed prop type: The prop \`${args.key}\` expects a ${args.expected} in \`Link\`, but got \`${args.actual}\` instead.` + + (typeof window !== 'undefined' + ? "\nOpen your browser's console to view the Component stack trace." + : '') + ) + } + + // TypeScript trick for type-guarding: + const requiredPropsGuard: Record = { + href: true, + } as const + const requiredProps: LinkPropsRequired[] = Object.keys( + requiredPropsGuard + ) as LinkPropsRequired[] + requiredProps.forEach((key: LinkPropsRequired) => { + if (key === 'href') { + if ( + props[key] == null || + (typeof props[key] !== 'string' && typeof props[key] !== 'object') + ) { + if (typeof window === 'undefined') { + // In development, we need to let `` render on the server. + // Otherwise, it's impossible to track down the error. + props = { ...props, href: '' } + } else { + throw createPropError({ + key, + expected: '`string` or `object`', + actual: props[key] === null ? 'null' : typeof props[key], + }) + } + } + } else { + // TypeScript trick for type-guarding: + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const _: never = key + } + }) + + // TypeScript trick for type-guarding: + const optionalPropsGuard: Record = { + as: true, + replace: true, + scroll: true, + shallow: true, + passHref: true, + prefetch: true, + } as const + const optionalProps: LinkPropsOptional[] = Object.keys( + optionalPropsGuard + ) as LinkPropsOptional[] + optionalProps.forEach((key: LinkPropsOptional) => { + if (key === 'as') { + if ( + props[key] && + typeof props[key] !== 'string' && + typeof props[key] !== 'object' + ) { + throw createPropError({ + key, + expected: '`string` or `object`', + actual: typeof props[key], + }) + } + } else if ( + key === 'replace' || + key === 'scroll' || + key === 'shallow' || + key === 'passHref' || + key === 'prefetch' + ) { + if (props[key] != null && typeof props[key] !== 'boolean') { + throw createPropError({ + key, + expected: '`boolean`', + actual: typeof props[key], + }) + } + } else { + // TypeScript trick for type-guarding: + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const _: never = key + } + }) + // This hook is in a conditional but that is ok because `process.env.NODE_ENV` never changes // eslint-disable-next-line react-hooks/rules-of-hooks const hasWarned = React.useRef(false) @@ -241,36 +340,4 @@ function Link(props: React.PropsWithChildren) { return React.cloneElement(child, childProps) } -if (process.env.NODE_ENV === 'development') { - const warn = execOnce(console.error) - - // This module gets removed by webpack.IgnorePlugin - const PropTypes = require('prop-types') - const exact = require('prop-types-exact') - // @ts-ignore the property is supported, when declaring it on the class it outputs an extra bit of code which is not needed. - Link.propTypes = exact({ - href: PropTypes.oneOfType([PropTypes.string, PropTypes.object]).isRequired, - as: PropTypes.oneOfType([PropTypes.string, PropTypes.object]), - prefetch: PropTypes.bool, - replace: PropTypes.bool, - shallow: PropTypes.bool, - passHref: PropTypes.bool, - scroll: PropTypes.bool, - children: PropTypes.oneOfType([ - PropTypes.element, - (props: any, propName: string) => { - const value = props[propName] - - if (typeof value === 'string') { - warn( - `Warning: You're using a string directly inside . This usage has been deprecated. Please add an tag as child of ` - ) - } - - return null - }, - ]).isRequired, - }) -} - export default Link diff --git a/packages/next/package.json b/packages/next/package.json index 32b17af47247a..f30b069293a25 100644 --- a/packages/next/package.json +++ b/packages/next/package.json @@ -101,7 +101,6 @@ "postcss": "7.0.32", "process": "0.11.10", "prop-types": "15.7.2", - "prop-types-exact": "1.2.0", "react-is": "16.13.1", "react-refresh": "0.8.3", "resolve-url-loader": "3.1.1", From 2a437fe9e72bc3a258055addc0d9b4db82921c2a Mon Sep 17 00:00:00 2001 From: Joe Haddad Date: Thu, 6 Aug 2020 14:55:08 -0400 Subject: [PATCH 2/5] Remove server pass-through --- packages/next/client/link.tsx | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/packages/next/client/link.tsx b/packages/next/client/link.tsx index 2b81b23c35020..71ba21a5571c6 100644 --- a/packages/next/client/link.tsx +++ b/packages/next/client/link.tsx @@ -179,17 +179,11 @@ function Link(props: React.PropsWithChildren) { props[key] == null || (typeof props[key] !== 'string' && typeof props[key] !== 'object') ) { - if (typeof window === 'undefined') { - // In development, we need to let `` render on the server. - // Otherwise, it's impossible to track down the error. - props = { ...props, href: '' } - } else { - throw createPropError({ - key, - expected: '`string` or `object`', - actual: props[key] === null ? 'null' : typeof props[key], - }) - } + throw createPropError({ + key, + expected: '`string` or `object`', + actual: props[key] === null ? 'null' : typeof props[key], + }) } } else { // TypeScript trick for type-guarding: From 89f1f9cdd4a62302be1d236a793394d5544cf0f5 Mon Sep 17 00:00:00 2001 From: Joe Haddad Date: Tue, 18 Aug 2020 11:24:31 -0400 Subject: [PATCH 3/5] Adjust error --- packages/next/client/link.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/next/client/link.tsx b/packages/next/client/link.tsx index 1bfeef3ff5b38..864edffa955c9 100644 --- a/packages/next/client/link.tsx +++ b/packages/next/client/link.tsx @@ -160,7 +160,7 @@ function Link(props: React.PropsWithChildren) { actual: string }) { return new Error( - `Failed prop type: The prop \`${args.key}\` expects a ${args.expected} in \`Link\`, but got \`${args.actual}\` instead.` + + `Failed prop type: The prop \`${args.key}\` expects a ${args.expected} in \`\`, but got \`${args.actual}\` instead.` + (typeof window !== 'undefined' ? "\nOpen your browser's console to view the Component stack trace." : '') From 04aa068439eb12917abc4e432cbf6ba526839570 Mon Sep 17 00:00:00 2001 From: Joe Haddad Date: Tue, 18 Aug 2020 11:24:40 -0400 Subject: [PATCH 4/5] Fix lockfile --- yarn.lock | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/yarn.lock b/yarn.lock index d6acd5d0d5efe..f0ab3d33bc4f3 100644 --- a/yarn.lock +++ b/yarn.lock @@ -14061,14 +14061,6 @@ promzard@^0.3.0: dependencies: read "1" -prop-types-exact@1.2.0: - version "1.2.0" - resolved "https://registry.yarnpkg.com/prop-types-exact/-/prop-types-exact-1.2.0.tgz#825d6be46094663848237e3925a98c6e944e9869" - dependencies: - has "^1.0.3" - object.assign "^4.1.0" - reflect.ownkeys "^0.2.0" - prop-types@15.7.2, prop-types@^15.6.2, prop-types@^15.7.2: version "15.7.2" resolved "https://registry.yarnpkg.com/prop-types/-/prop-types-15.7.2.tgz#52c41e75b8c87e72b9d9360e0206b99dcbffa6c5" @@ -14556,10 +14548,6 @@ reduce-function-call@^1.0.1: dependencies: balanced-match "^1.0.0" -reflect.ownkeys@^0.2.0: - version "0.2.0" - resolved "https://registry.yarnpkg.com/reflect.ownkeys/-/reflect.ownkeys-0.2.0.tgz#749aceec7f3fdf8b63f927a04809e90c5c0b3460" - regenerate-unicode-properties@^8.1.0: version "8.1.0" resolved "https://registry.yarnpkg.com/regenerate-unicode-properties/-/regenerate-unicode-properties-8.1.0.tgz#ef51e0f0ea4ad424b77bf7cb41f3e015c70a3f0e" From 44f7fe4a708f25bda69cb73a974d17829addf114 Mon Sep 17 00:00:00 2001 From: Joe Haddad Date: Tue, 18 Aug 2020 12:06:29 -0400 Subject: [PATCH 5/5] Add link tests --- test/acceptance/ReactRefreshLogBox.test.js | 162 +++++++++++++++++++++ 1 file changed, 162 insertions(+) diff --git a/test/acceptance/ReactRefreshLogBox.test.js b/test/acceptance/ReactRefreshLogBox.test.js index 424ae2a326eb1..a57cb7e831e99 100644 --- a/test/acceptance/ReactRefreshLogBox.test.js +++ b/test/acceptance/ReactRefreshLogBox.test.js @@ -1073,3 +1073,165 @@ test('logbox: anchors links in error messages', async () => { await cleanup() }) + +test(' component props errors', async () => { + const [session, cleanup] = await sandbox() + + await session.patch( + 'index.js', + ` + import Link from 'next/link' + + export default function Hello() { + return + } + ` + ) + + expect(await session.hasRedbox(true)).toBe(true) + expect(await session.getRedboxDescription()).toMatchInlineSnapshot( + `"Error: Failed prop type: The prop \`href\` expects a \`string\` or \`object\` in \`\`, but got \`undefined\` instead."` + ) + + await session.patch( + 'index.js', + ` + import Link from 'next/link' + + export default function Hello() { + return Abc + } + ` + ) + expect(await session.hasRedbox()).toBe(false) + + await session.patch( + 'index.js', + ` + import Link from 'next/link' + + export default function Hello() { + return ( + + Abc + + ) + } + ` + ) + expect(await session.hasRedbox()).toBe(false) + + await session.patch( + 'index.js', + ` + import Link from 'next/link' + + export default function Hello() { + return ( + + Abc + + ) + } + ` + ) + expect(await session.hasRedbox()).toBe(false) + + await session.patch( + 'index.js', + ` + import Link from 'next/link' + + export default function Hello() { + return ( + + Abc + + ) + } + ` + ) + expect(await session.hasRedbox()).toBe(false) + + await session.patch( + 'index.js', + ` + import Link from 'next/link' + + export default function Hello() { + return ( + + Abc + + ) + } + ` + ) + expect(await session.hasRedbox(true)).toBe(true) + expect(await session.getRedboxDescription()).toMatchInlineSnapshot(` + "Error: Failed prop type: The prop \`scroll\` expects a \`boolean\` in \`\`, but got \`string\` instead. + Open your browser's console to view the Component stack trace." + `) + + await session.patch( + 'index.js', + ` + import Link from 'next/link' + + export default function Hello() { + return ( + + Abc + + ) + } + ` + ) + expect(await session.hasRedbox(true)).toBe(true) + expect(await session.getRedboxDescription()).toMatchInlineSnapshot(` + "Error: Failed prop type: The prop \`href\` expects a \`string\` or \`object\` in \`\`, but got \`boolean\` instead. + Open your browser's console to view the Component stack trace." + `) + + await cleanup() +})