From 838114b85f2e790646fa8e7f2e22eab0aa10fb9b Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Mon, 24 Oct 2022 21:12:32 -0700 Subject: [PATCH 01/12] feat: initial hybrid hook support --- .../next/client/components/layout-router.tsx | 24 ++++-- packages/next/client/components/navigation.ts | 20 ++++- packages/next/client/index.tsx | 24 +++--- packages/next/client/link.tsx | 81 +++++++++++++------ packages/next/client/router.ts | 7 +- packages/next/server/render.tsx | 59 ++++++++------ .../next/shared/lib/app-router-context.ts | 4 +- packages/next/shared/lib/router-context.ts | 2 +- packages/next/shared/lib/router/adapters.ts | 30 +++++++ packages/next/shared/lib/router/router.ts | 9 +++ 10 files changed, 186 insertions(+), 74 deletions(-) create mode 100644 packages/next/shared/lib/router/adapters.ts diff --git a/packages/next/client/components/layout-router.tsx b/packages/next/client/components/layout-router.tsx index 5a4e27caa5bc061..84e6aef9ae0a608 100644 --- a/packages/next/client/components/layout-router.tsx +++ b/packages/next/client/components/layout-router.tsx @@ -25,6 +25,7 @@ import { fetchServerResponse } from './app-router' import { createInfinitePromise } from './infinite-promise' import { ErrorBoundary } from './error-boundary' import { matchSegment } from './match-segments' +import { useRouter } from './navigation' /** * Add refetch marker to router state at the point of the current layout segment. @@ -109,11 +110,13 @@ export function InnerLayoutRouter({ path: string rootLayoutIncluded: boolean }) { - const { - changeByServerResponse, - tree: fullTree, - focusAndScrollRef, - } = useContext(GlobalLayoutRouterContext) + const context = useContext(GlobalLayoutRouterContext) + if (!context) { + throw new Error('invariant global layout router not mounted') + } + + const { changeByServerResponse, tree: fullTree, focusAndScrollRef } = context + const focusAndScrollElementRef = useRef(null) useEffect(() => { @@ -275,7 +278,7 @@ interface RedirectBoundaryProps { } function HandleRedirect({ redirect }: { redirect: string }) { - const router = useContext(AppRouterContext) + const router = useRouter() useEffect(() => { router.replace(redirect, {}) @@ -312,7 +315,7 @@ class RedirectErrorBoundary extends React.Component< } function RedirectBoundary({ children }: { children: React.ReactNode }) { - const router = useContext(AppRouterContext) + const router = useRouter() return ( {children} ) @@ -389,7 +392,12 @@ export default function OuterLayoutRouter({ notFound: React.ReactNode | undefined rootLayoutIncluded: boolean }) { - const { childNodes, tree, url } = useContext(LayoutRouterContext) + const context = useContext(LayoutRouterContext) + if (!context) { + throw new Error('invariant expected layout router to be mounted') + } + + const { childNodes, tree, url } = context // Get the current parallelRouter cache node let childNodesForParallelRouter = childNodes.get(parallelRouterKey) diff --git a/packages/next/client/components/navigation.ts b/packages/next/client/components/navigation.ts index 755922ac789aa56..f12b6d793adefcd 100644 --- a/packages/next/client/components/navigation.ts +++ b/packages/next/client/components/navigation.ts @@ -72,9 +72,15 @@ class ReadonlyURLSearchParams { export function useSearchParams() { staticGenerationBailout('useSearchParams') const searchParams = useContext(SearchParamsContext) + if (!searchParams) { + throw new Error('invariant expected search params to be mounted') + } + + // eslint-disable-next-line react-hooks/rules-of-hooks const readonlySearchParams = useMemo(() => { return new ReadonlyURLSearchParams(searchParams) }, [searchParams]) + return readonlySearchParams } @@ -83,7 +89,12 @@ export function useSearchParams() { */ export function usePathname(): string { staticGenerationBailout('usePathname') - return useContext(PathnameContext) + const pathname = useContext(PathnameContext) + if (pathname === null) { + throw new Error('invariant expected pathname to be mounted') + } + + return pathname } // TODO-APP: getting all params when client-side navigating is non-trivial as it does not have route matchers so this might have to be a server context instead. @@ -106,7 +117,12 @@ export { * Get the router methods. For example router.push('/dashboard') */ export function useRouter(): import('../../shared/lib/app-router-context').AppRouterInstance { - return useContext(AppRouterContext) + const router = useContext(AppRouterContext) + if (router === null) { + throw new Error('invariant expected app router to be mounted') + } + + return router } // TODO-APP: handle parallel routes diff --git a/packages/next/client/index.tsx b/packages/next/client/index.tsx index e08793c9ee8384b..f9ec72ae7ca36bd 100644 --- a/packages/next/client/index.tsx +++ b/packages/next/client/index.tsx @@ -34,6 +34,8 @@ import { ImageConfigContext } from '../shared/lib/image-config-context' import { ImageConfigComplete } from '../shared/lib/image-config' import { removeBasePath } from './remove-base-path' import { hasBasePath } from './has-base-path' +import { AppRouterContext } from '../shared/lib/app-router-context' +import { adaptForAppRouterInstance } from '../shared/lib/router/adapters' const ReactDOM = process.env.__NEXT_REACT_ROOT ? require('react-dom/client') @@ -306,15 +308,19 @@ function AppContainer({ ) } > - - - - {children} - - - + + + + + {children} + + + + ) } diff --git a/packages/next/client/link.tsx b/packages/next/client/link.tsx index 8dec76ea0150a7a..040271488556491 100644 --- a/packages/next/client/link.tsx +++ b/packages/next/client/link.tsx @@ -8,6 +8,7 @@ import { PrefetchOptions, resolveHref, } from '../shared/lib/router/router' +import { formatUrl } from '../shared/lib/router/utils/format-url' import { addLocale } from './add-locale' import { RouterContext } from '../shared/lib/router-context' import { @@ -17,6 +18,7 @@ import { import { useIntersection } from './use-intersection' import { getDomainLocale } from './get-domain-locale' import { addBasePath } from './add-base-path' +import { useRouter } from './components/navigation' type Url = string | UrlObject type RequiredKeys = { @@ -108,12 +110,13 @@ type LinkPropsOptional = OptionalKeys const prefetched: { [cacheKey: string]: boolean } = {} function prefetch( - router: NextRouter, + router: NextRouter | AppRouterInstance, href: string, as: string, options?: PrefetchOptions ): void { if (typeof window === 'undefined' || !router) return + if (!isLocalURL(href)) return // Prefetch the JSON page if asked (only in the client) // We need to handle a prefetch error here since we may be @@ -125,10 +128,13 @@ function prefetch( throw err } }) + const curLocale = options && typeof options.locale !== 'undefined' ? options.locale - : router && router.locale + : 'locale' in router + ? router.locale + : undefined // Join on an invalid URI character prefetched[href + '%' + as + (curLocale ? '%' + curLocale : '')] = true @@ -179,11 +185,7 @@ function linkClicked( scroll, }) } else { - // If `beforePopState` doesn't exist on the router it's the AppRouter. - const method: keyof AppRouterInstance = replace ? 'replace' : 'push' - - // Apply `as` if it's provided. - router[method](as || href, { + router[replace ? 'replace' : 'push'](as || href, { forceOptimisticNavigation: !prefetchEnabled, }) } @@ -202,6 +204,14 @@ type LinkPropsReal = React.PropsWithChildren< LinkProps > +function formatStringOrUrl(urlObjOrString: UrlObject | string): string { + if (typeof urlObjOrString === 'string') { + return urlObjOrString + } + + return formatUrl(urlObjOrString) +} + /** * React Component that enables client-side transitions between routes. */ @@ -357,21 +367,36 @@ const Link = React.forwardRef( } const p = prefetchProp !== false - let router = React.useContext(RouterContext) - // TODO-APP: type error. Remove `as any` - const appRouter = React.useContext(AppRouterContext) as any - if (appRouter) { - router = appRouter - } + const pagesRouter = React.useContext(RouterContext) + const appRouter = useRouter() + const router = pagesRouter ?? appRouter + + // We're in the app directory if there is no pages router. + const isAppRouter = !pagesRouter const { href, as } = React.useMemo(() => { - const [resolvedHref, resolvedAs] = resolveHref(router, hrefProp, true) + if (!pagesRouter) { + const resolvedHref = formatStringOrUrl(hrefProp) + return { + href: resolvedHref, + as: asProp ? formatStringOrUrl(asProp) : resolvedHref, + } + } + + const [resolvedHref, resolvedAs] = resolveHref( + pagesRouter, + hrefProp, + true + ) + return { href: resolvedHref, - as: asProp ? resolveHref(router, asProp) : resolvedAs || resolvedHref, + as: asProp + ? resolveHref(pagesRouter, asProp) + : resolvedAs || resolvedHref, } - }, [router, hrefProp, asProp]) + }, [pagesRouter, hrefProp, asProp]) const previousHref = React.useRef(href) const previousAs = React.useRef(as) @@ -448,7 +473,7 @@ const Link = React.forwardRef( React.useEffect(() => { const shouldPrefetch = isVisible && p && isLocalURL(href) const curLocale = - typeof locale !== 'undefined' ? locale : router && router.locale + typeof locale !== 'undefined' ? locale : pagesRouter?.locale const isPrefetched = prefetched[href + '%' + as + (curLocale ? '%' + curLocale : '')] if (shouldPrefetch && !isPrefetched) { @@ -456,7 +481,7 @@ const Link = React.forwardRef( locale: curLocale, }) } - }, [as, href, isVisible, locale, p, router]) + }, [as, href, isVisible, locale, p, pagesRouter?.locale, router]) const childProps: { onTouchStart: React.TouchEventHandler @@ -495,7 +520,7 @@ const Link = React.forwardRef( shallow, scroll, locale, - Boolean(appRouter), + isAppRouter, p ) } @@ -513,7 +538,7 @@ const Link = React.forwardRef( } // Check for not prefetch disabled in page using appRouter - if (!(!p && appRouter)) { + if (!(!p && isAppRouter)) { if (isLocalURL(href)) { prefetch(router, href, as, { priority: true }) } @@ -533,7 +558,7 @@ const Link = React.forwardRef( } // Check for not prefetch disabled in page using appRouter - if (!(!p && appRouter)) { + if (!(!p && isAppRouter)) { if (isLocalURL(href)) { prefetch(router, href, as, { priority: true }) } @@ -549,18 +574,22 @@ const Link = React.forwardRef( (child.type === 'a' && !('href' in child.props)) ) { const curLocale = - typeof locale !== 'undefined' ? locale : router && router.locale + typeof locale !== 'undefined' ? locale : pagesRouter?.locale // we only render domain locales if we are currently on a domain locale // so that locale links are still visitable in development/preview envs const localeDomain = - router && - router.isLocaleDomain && - getDomainLocale(as, curLocale, router.locales, router.domainLocales) + pagesRouter?.isLocaleDomain && + getDomainLocale( + as, + curLocale, + pagesRouter?.locales, + pagesRouter?.domainLocales + ) childProps.href = localeDomain || - addBasePath(addLocale(as, curLocale, router && router.defaultLocale)) + addBasePath(addLocale(as, curLocale, pagesRouter?.defaultLocale)) } return legacyBehavior ? ( diff --git a/packages/next/client/router.ts b/packages/next/client/router.ts index f739b291209b612..beb0efe0c74415b 100644 --- a/packages/next/client/router.ts +++ b/packages/next/client/router.ts @@ -130,7 +130,12 @@ export default singletonRouter as SingletonRouter export { default as withRouter } from './with-router' export function useRouter(): NextRouter { - return React.useContext(RouterContext) + const router = React.useContext(RouterContext) + if (!router) { + throw new Error('invariant expected pages router to be mounted') + } + + return router } // INTERNAL APIS diff --git a/packages/next/server/render.tsx b/packages/next/server/render.tsx index b602c646895e873..5b08c664f1f8412 100644 --- a/packages/next/server/render.tsx +++ b/packages/next/server/render.tsx @@ -82,6 +82,8 @@ import { ImageConfigContext } from '../shared/lib/image-config-context' import stripAnsi from 'next/dist/compiled/strip-ansi' import { shouldUseReactRoot } from './utils' import { stripInternalQueries } from './internal-utils' +import { adaptForAppRouterInstance } from '../shared/lib/router/adapters' +import { AppRouterContext } from '../shared/lib/app-router-context' let tryGetPreviewData: typeof import('./api-utils/node').tryGetPreviewData let warn: typeof import('../build/output/log').warn @@ -177,6 +179,9 @@ class ServerRouter implements NextRouter { back() { noRouter() } + forward(): void { + noRouter() + } prefetch(): any { noRouter() } @@ -615,6 +620,8 @@ export async function renderToHTML( getRequestMeta(req, '__nextIsLocaleDomain') ) + const appRouter = adaptForAppRouterInstance(router) + let scriptLoader: any = {} const jsxStyleRegistry = createStyleRegistry() const ampState = { @@ -637,32 +644,34 @@ export async function renderToHTML( } const AppContainer = ({ children }: { children: JSX.Element }) => ( - - - { - head = state - }, - updateScripts: (scripts) => { - scriptLoader = scripts - }, - scripts: initialScripts, - mountedInstances: new Set(), - }} - > - reactLoadableModules.push(moduleName)} + + + + { + head = state + }, + updateScripts: (scripts) => { + scriptLoader = scripts + }, + scripts: initialScripts, + mountedInstances: new Set(), + }} > - - - {children} - - - - - - + reactLoadableModules.push(moduleName)} + > + + + {children} + + + + + + + ) // The `useId` API uses the path indexes to generate an ID for each node. diff --git a/packages/next/shared/lib/app-router-context.ts b/packages/next/shared/lib/app-router-context.ts index 42eb5c0931612bb..16702c0dd0a8c9c 100644 --- a/packages/next/shared/lib/app-router-context.ts +++ b/packages/next/shared/lib/app-router-context.ts @@ -59,8 +59,8 @@ export interface AppRouterInstance { prefetch(href: string): void } -export const AppRouterContext = React.createContext( - null as any +export const AppRouterContext = React.createContext( + null ) export const LayoutRouterContext = React.createContext<{ childNodes: CacheNode['parallelRoutes'] diff --git a/packages/next/shared/lib/router-context.ts b/packages/next/shared/lib/router-context.ts index d00df65cfeb5089..4a9d9a575b89827 100644 --- a/packages/next/shared/lib/router-context.ts +++ b/packages/next/shared/lib/router-context.ts @@ -1,7 +1,7 @@ import React from 'react' import type { NextRouter } from './router/router' -export const RouterContext = React.createContext(null as any) +export const RouterContext = React.createContext(null) if (process.env.NODE_ENV !== 'production') { RouterContext.displayName = 'RouterContext' diff --git a/packages/next/shared/lib/router/adapters.ts b/packages/next/shared/lib/router/adapters.ts new file mode 100644 index 000000000000000..5fae3ac15299190 --- /dev/null +++ b/packages/next/shared/lib/router/adapters.ts @@ -0,0 +1,30 @@ +import { AppRouterInstance } from '../app-router-context' +import { NextRouter } from './router' + +/** + * adaptForAppRouterInstance implements the AppRouterInstance with a NextRouter. + */ +export function adaptForAppRouterInstance( + nextRouter: NextRouter +): AppRouterInstance { + return { + back(): void { + nextRouter.back() + }, + forward(): void { + nextRouter.forward() + }, + refresh(): void { + nextRouter.reload() + }, + push(href: string): void { + void nextRouter.push(href) + }, + replace(href: string): void { + void nextRouter.replace(href) + }, + prefetch(href: string): void { + void nextRouter.prefetch(href) + }, + } +} diff --git a/packages/next/shared/lib/router/router.ts b/packages/next/shared/lib/router/router.ts index ec609730f24c7ba..90c6d63968a49b4 100644 --- a/packages/next/shared/lib/router/router.ts +++ b/packages/next/shared/lib/router/router.ts @@ -259,6 +259,7 @@ export function resolveHref( // fallback to / for invalid asPath values e.g. // base = new URL('/', 'http://n') } + try { const finalUrl = new URL(urlAsString, base) finalUrl.pathname = normalizePathTrailingSlash(finalUrl.pathname) @@ -542,6 +543,7 @@ export type NextRouter = BaseRouter & | 'replace' | 'reload' | 'back' + | 'forward' | 'prefetch' | 'beforePopState' | 'events' @@ -1137,6 +1139,13 @@ export default class Router implements BaseRouter { window.history.back() } + /** + * Go forward in history + */ + forward() { + window.history.forward() + } + /** * Performs a `pushState` with arguments * @param url of the route From 59492e0cce5188c4789cc4e25f478165c41ca75e Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Wed, 26 Oct 2022 16:34:19 -0700 Subject: [PATCH 02/12] feat: port useSearchParams and usePathname to pages --- .../next/client/components/app-router.tsx | 2 +- packages/next/client/components/navigation.ts | 2 +- packages/next/client/index.tsx | 36 ++++++---- packages/next/server/render.tsx | 66 +++++++++++-------- .../lib}/hooks-client-context.ts | 4 +- packages/next/shared/lib/router/adapters.ts | 65 ++++++++++++++++-- 6 files changed, 125 insertions(+), 50 deletions(-) rename packages/next/{client/components => shared/lib}/hooks-client-context.ts (86%) diff --git a/packages/next/client/components/app-router.tsx b/packages/next/client/components/app-router.tsx index e0b2c964911f083..a1ed740d1094f62 100644 --- a/packages/next/client/components/app-router.tsx +++ b/packages/next/client/components/app-router.tsx @@ -26,7 +26,7 @@ import { // ParamsContext, PathnameContext, // LayoutSegmentsContext, -} from './hooks-client-context' +} from '../../shared/lib/hooks-client-context' import { useReducerWithReduxDevtools } from './use-reducer-with-devtools' import { ErrorBoundary, GlobalErrorComponent } from './error-boundary' diff --git a/packages/next/client/components/navigation.ts b/packages/next/client/components/navigation.ts index f12b6d793adefcd..39b3d7092521fb7 100644 --- a/packages/next/client/components/navigation.ts +++ b/packages/next/client/components/navigation.ts @@ -11,7 +11,7 @@ import { // ParamsContext, PathnameContext, // LayoutSegmentsContext, -} from './hooks-client-context' +} from '../../shared/lib/hooks-client-context' import { staticGenerationBailout } from './static-generation-bailout' const INTERNAL_URLSEARCHPARAMS_INSTANCE = Symbol( diff --git a/packages/next/client/index.tsx b/packages/next/client/index.tsx index f9ec72ae7ca36bd..c64007f847dbadd 100644 --- a/packages/next/client/index.tsx +++ b/packages/next/client/index.tsx @@ -35,7 +35,15 @@ import { ImageConfigComplete } from '../shared/lib/image-config' import { removeBasePath } from './remove-base-path' import { hasBasePath } from './has-base-path' import { AppRouterContext } from '../shared/lib/app-router-context' -import { adaptForAppRouterInstance } from '../shared/lib/router/adapters' +import { + adaptForAppRouterInstance, + adaptForPathname, + adaptForSearchParams, +} from '../shared/lib/router/adapters' +import { + PathnameContext, + SearchParamsContext, +} from '../shared/lib/hooks-client-context' const ReactDOM = process.env.__NEXT_REACT_ROOT ? require('react-dom/client') @@ -309,17 +317,21 @@ function AppContainer({ } > - - - - {children} - - - + + + + + + {children} + + + + + ) diff --git a/packages/next/server/render.tsx b/packages/next/server/render.tsx index 5b08c664f1f8412..d0fce3954e70606 100644 --- a/packages/next/server/render.tsx +++ b/packages/next/server/render.tsx @@ -82,8 +82,16 @@ import { ImageConfigContext } from '../shared/lib/image-config-context' import stripAnsi from 'next/dist/compiled/strip-ansi' import { shouldUseReactRoot } from './utils' import { stripInternalQueries } from './internal-utils' -import { adaptForAppRouterInstance } from '../shared/lib/router/adapters' +import { + adaptForAppRouterInstance, + adaptForPathname, + adaptForSearchParams, +} from '../shared/lib/router/adapters' import { AppRouterContext } from '../shared/lib/app-router-context' +import { + PathnameContext, + SearchParamsContext, +} from '../shared/lib/hooks-client-context' let tryGetPreviewData: typeof import('./api-utils/node').tryGetPreviewData let warn: typeof import('../build/output/log').warn @@ -645,32 +653,36 @@ export async function renderToHTML( const AppContainer = ({ children }: { children: JSX.Element }) => ( - - - { - head = state - }, - updateScripts: (scripts) => { - scriptLoader = scripts - }, - scripts: initialScripts, - mountedInstances: new Set(), - }} - > - reactLoadableModules.push(moduleName)} - > - - - {children} - - - - - - + + + + + { + head = state + }, + updateScripts: (scripts) => { + scriptLoader = scripts + }, + scripts: initialScripts, + mountedInstances: new Set(), + }} + > + reactLoadableModules.push(moduleName)} + > + + + {children} + + + + + + + + ) diff --git a/packages/next/client/components/hooks-client-context.ts b/packages/next/shared/lib/hooks-client-context.ts similarity index 86% rename from packages/next/client/components/hooks-client-context.ts rename to packages/next/shared/lib/hooks-client-context.ts index b34157d876df44a..83d14c1451a0c6d 100644 --- a/packages/next/client/components/hooks-client-context.ts +++ b/packages/next/shared/lib/hooks-client-context.ts @@ -2,8 +2,8 @@ import { createContext } from 'react' -export const SearchParamsContext = createContext(null as any) -export const PathnameContext = createContext(null as any) +export const SearchParamsContext = createContext(null) +export const PathnameContext = createContext(null) export const ParamsContext = createContext(null as any) export const LayoutSegmentsContext = createContext(null as any) diff --git a/packages/next/shared/lib/router/adapters.ts b/packages/next/shared/lib/router/adapters.ts index 5fae3ac15299190..facd64c2a7effed 100644 --- a/packages/next/shared/lib/router/adapters.ts +++ b/packages/next/shared/lib/router/adapters.ts @@ -1,30 +1,81 @@ +import type { ParsedUrlQuery } from 'node:querystring' import { AppRouterInstance } from '../app-router-context' import { NextRouter } from './router' /** * adaptForAppRouterInstance implements the AppRouterInstance with a NextRouter. + * + * @param router the NextRouter to adapt + * @returns an AppRouterInstance */ export function adaptForAppRouterInstance( - nextRouter: NextRouter + router: NextRouter ): AppRouterInstance { return { back(): void { - nextRouter.back() + router.back() }, forward(): void { - nextRouter.forward() + router.forward() }, refresh(): void { - nextRouter.reload() + router.reload() }, push(href: string): void { - void nextRouter.push(href) + void router.push(href) }, replace(href: string): void { - void nextRouter.replace(href) + void router.replace(href) }, prefetch(href: string): void { - void nextRouter.prefetch(href) + void router.prefetch(href) }, } } + +/** + * transforms the ParsedUrlQuery into a URLSearchParams. + * + * @param query the query to transform + * @returns URLSearchParams + */ +function transformQuery(query: ParsedUrlQuery): URLSearchParams { + const params = new URLSearchParams() + + for (const [name, value] of Object.entries(query)) { + if (Array.isArray(value)) { + for (const val of value) { + params.append(name, val) + } + } else if (typeof value !== 'undefined') { + params.append(name, value) + } + } + + return params +} + +/** + * adaptForSearchParams transforms the ParsedURLQuery into URLSearchParams. + * + * @param router the router that contains the query. + * @returns the search params in the URLSearchParams format + */ +export function adaptForSearchParams(router: NextRouter): URLSearchParams { + if (!router.isReady || !router.query) { + return new URLSearchParams() + } + + return transformQuery(router.query) +} + +/** + * adaptForPathname adapts the `asPath` parameter from the router to a pathname. + * + * @param asPath the asPath parameter to transform that comes from the router + * @returns pathname part of `asPath` + */ +export function adaptForPathname(asPath: string): string { + const url = new URL(asPath, 'http://f') + return url.pathname +} From 0773715d9d004655eb46451086ca04b770be0bf8 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Wed, 26 Oct 2022 17:26:09 -0700 Subject: [PATCH 03/12] test: added tests for supporting app client hooks in pages --- .../app/components/router-hooks-fixtures.js | 26 +++++++++++++++ .../app-dir/app/pages/adapter-hooks/[id].js | 5 +++ .../app/pages/adapter-hooks/[id]/account.js | 13 ++++++++ .../app-dir/app/pages/adapter-hooks/static.js | 9 +++++ test/e2e/app-dir/index.test.ts | 33 +++++++++++++++++++ 5 files changed, 86 insertions(+) create mode 100644 test/e2e/app-dir/app/components/router-hooks-fixtures.js create mode 100644 test/e2e/app-dir/app/pages/adapter-hooks/[id].js create mode 100644 test/e2e/app-dir/app/pages/adapter-hooks/[id]/account.js create mode 100644 test/e2e/app-dir/app/pages/adapter-hooks/static.js diff --git a/test/e2e/app-dir/app/components/router-hooks-fixtures.js b/test/e2e/app-dir/app/components/router-hooks-fixtures.js new file mode 100644 index 000000000000000..fc946354603beb6 --- /dev/null +++ b/test/e2e/app-dir/app/components/router-hooks-fixtures.js @@ -0,0 +1,26 @@ +import { useRouter } from 'next/router' +import { usePathname, useSearchParams } from 'next/navigation' +import { useState } from 'react' +import { useEffect } from 'react' + +export const RouterHooksFixtures = () => { + const router = useRouter() + const searchParams = useSearchParams() + const pathname = usePathname() + + const [value, setValue] = useState(null) + useEffect(() => { + if (!router.isReady) { + return + } + + setValue(searchParams.get('key')) + }, [router.isReady, searchParams]) + + return ( +
+
{value}
+
{pathname}
+
+ ) +} diff --git a/test/e2e/app-dir/app/pages/adapter-hooks/[id].js b/test/e2e/app-dir/app/pages/adapter-hooks/[id].js new file mode 100644 index 000000000000000..6395b062b3f84a5 --- /dev/null +++ b/test/e2e/app-dir/app/pages/adapter-hooks/[id].js @@ -0,0 +1,5 @@ +import { RouterHooksFixtures } from '../../components/router-hooks-fixtures' + +export default function Page() { + return +} diff --git a/test/e2e/app-dir/app/pages/adapter-hooks/[id]/account.js b/test/e2e/app-dir/app/pages/adapter-hooks/[id]/account.js new file mode 100644 index 000000000000000..2c917626f2e3c25 --- /dev/null +++ b/test/e2e/app-dir/app/pages/adapter-hooks/[id]/account.js @@ -0,0 +1,13 @@ +import { RouterHooksFixtures } from '../../../components/router-hooks-fixtures' + +export default function Page() { + return +} + +export const getStaticProps = () => { + return { props: {} } +} + +export const getStaticPaths = () => { + return { fallback: 'blocking', paths: [{ params: { id: '1' } }] } +} diff --git a/test/e2e/app-dir/app/pages/adapter-hooks/static.js b/test/e2e/app-dir/app/pages/adapter-hooks/static.js new file mode 100644 index 000000000000000..4bac4a3f900e7de --- /dev/null +++ b/test/e2e/app-dir/app/pages/adapter-hooks/static.js @@ -0,0 +1,9 @@ +import { RouterHooksFixtures } from '../../components/router-hooks-fixtures' + +export default function Page() { + return +} + +export const getStaticProps = () => { + return { props: {} } +} diff --git a/test/e2e/app-dir/index.test.ts b/test/e2e/app-dir/index.test.ts index 28faf234e6b04cd..aafb7238fe1dbe3 100644 --- a/test/e2e/app-dir/index.test.ts +++ b/test/e2e/app-dir/index.test.ts @@ -1127,6 +1127,39 @@ describe('app dir', () => { describe('client components', () => { describe('hooks', () => { + describe('from pages', () => { + it.each([ + { pathname: '/adapter-hooks/static' }, + { pathname: '/adapter-hooks/1' }, + { pathname: '/adapter-hooks/2' }, + { pathname: '/adapter-hooks/1/account' }, + { pathname: '/adapter-hooks/static', keyValue: 'value' }, + { pathname: '/adapter-hooks/1', keyValue: 'value' }, + { pathname: '/adapter-hooks/2', keyValue: 'value' }, + { pathname: '/adapter-hooks/1/account', keyValue: 'value' }, + ])( + 'should have the correct hooks', + async ({ pathname, keyValue = '' }) => { + const browser = await webdriver( + next.url, + pathname + (keyValue ? `?key=${keyValue}` : '') + ) + + try { + await browser.waitForElementByCss('#router-ready') + expect(await browser.elementById('key-value').text()).toBe( + keyValue + ) + expect(await browser.elementById('pathname').text()).toBe( + pathname + ) + } finally { + await browser.close() + } + } + ) + }) + describe('usePathname', () => { it('should have the correct pathname', async () => { const html = await renderViaHTTP(next.url, '/hooks/use-pathname') From 62ba99ad68b5eb1d5279bafd89b7f65baadb81e6 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Wed, 26 Oct 2022 17:58:38 -0700 Subject: [PATCH 04/12] tests: added test to cover router --- .../app/components/router-hooks-fixtures.js | 24 ++++++++++++++----- .../app-dir/app/pages/adapter-hooks/pushed.js | 3 +++ test/e2e/app-dir/index.test.ts | 3 +++ 3 files changed, 24 insertions(+), 6 deletions(-) create mode 100644 test/e2e/app-dir/app/pages/adapter-hooks/pushed.js diff --git a/test/e2e/app-dir/app/components/router-hooks-fixtures.js b/test/e2e/app-dir/app/components/router-hooks-fixtures.js index fc946354603beb6..465ff3b665ca7be 100644 --- a/test/e2e/app-dir/app/components/router-hooks-fixtures.js +++ b/test/e2e/app-dir/app/components/router-hooks-fixtures.js @@ -1,26 +1,38 @@ -import { useRouter } from 'next/router' -import { usePathname, useSearchParams } from 'next/navigation' +import { useRouter as usePagesRouter } from 'next/router' +import { + usePathname, + useRouter as useAppRouter, + useSearchParams, +} from 'next/navigation' import { useState } from 'react' import { useEffect } from 'react' export const RouterHooksFixtures = () => { - const router = useRouter() + const pagesRouter = usePagesRouter() + const appRouter = useAppRouter() const searchParams = useSearchParams() const pathname = usePathname() const [value, setValue] = useState(null) useEffect(() => { - if (!router.isReady) { + if (!pagesRouter.isReady) { return } setValue(searchParams.get('key')) - }, [router.isReady, searchParams]) + }, [pagesRouter.isReady, searchParams]) + + const onClick = () => { + appRouter.push('/adapter-hooks/pushed') + } return ( -
+
{value}
{pathname}
+
) } diff --git a/test/e2e/app-dir/app/pages/adapter-hooks/pushed.js b/test/e2e/app-dir/app/pages/adapter-hooks/pushed.js new file mode 100644 index 000000000000000..95d5882bfc305a3 --- /dev/null +++ b/test/e2e/app-dir/app/pages/adapter-hooks/pushed.js @@ -0,0 +1,3 @@ +export default function Page() { + return
+} diff --git a/test/e2e/app-dir/index.test.ts b/test/e2e/app-dir/index.test.ts index aafb7238fe1dbe3..cf6821efb3b0d0e 100644 --- a/test/e2e/app-dir/index.test.ts +++ b/test/e2e/app-dir/index.test.ts @@ -1153,6 +1153,9 @@ describe('app dir', () => { expect(await browser.elementById('pathname').text()).toBe( pathname ) + + await browser.elementByCss('button').click() + await browser.waitForElementByCss('#pushed') } finally { await browser.close() } From c449c464dddd888523b9de9103918226c4adbe87 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Wed, 26 Oct 2022 18:02:29 -0700 Subject: [PATCH 05/12] fix: linting --- packages/next/client/components/layout-router.tsx | 1 - packages/next/client/link.tsx | 5 +---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/next/client/components/layout-router.tsx b/packages/next/client/components/layout-router.tsx index 84e6aef9ae0a608..dc8cb0c5139a0cf 100644 --- a/packages/next/client/components/layout-router.tsx +++ b/packages/next/client/components/layout-router.tsx @@ -19,7 +19,6 @@ import { LayoutRouterContext, GlobalLayoutRouterContext, TemplateContext, - AppRouterContext, } from '../../shared/lib/app-router-context' import { fetchServerResponse } from './app-router' import { createInfinitePromise } from './infinite-promise' diff --git a/packages/next/client/link.tsx b/packages/next/client/link.tsx index 040271488556491..3a7e31d9ea33b90 100644 --- a/packages/next/client/link.tsx +++ b/packages/next/client/link.tsx @@ -11,10 +11,7 @@ import { import { formatUrl } from '../shared/lib/router/utils/format-url' import { addLocale } from './add-locale' import { RouterContext } from '../shared/lib/router-context' -import { - AppRouterContext, - AppRouterInstance, -} from '../shared/lib/app-router-context' +import { AppRouterInstance } from '../shared/lib/app-router-context' import { useIntersection } from './use-intersection' import { getDomainLocale } from './get-domain-locale' import { addBasePath } from './add-base-path' From 632eee42360ee90e3319465d751f6a937a5e5970 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 27 Oct 2022 12:52:37 -0700 Subject: [PATCH 06/12] fix: improve link component - remove extra work on some logic paths - support missing router --- packages/next/client/link.tsx | 150 +++++++++++++++++++++------------- 1 file changed, 94 insertions(+), 56 deletions(-) diff --git a/packages/next/client/link.tsx b/packages/next/client/link.tsx index 3a7e31d9ea33b90..206c4baf9cf7bd4 100644 --- a/packages/next/client/link.tsx +++ b/packages/next/client/link.tsx @@ -11,11 +11,13 @@ import { import { formatUrl } from '../shared/lib/router/utils/format-url' import { addLocale } from './add-locale' import { RouterContext } from '../shared/lib/router-context' -import { AppRouterInstance } from '../shared/lib/app-router-context' +import { + AppRouterContext, + AppRouterInstance, +} from '../shared/lib/app-router-context' import { useIntersection } from './use-intersection' import { getDomainLocale } from './get-domain-locale' import { addBasePath } from './add-base-path' -import { useRouter } from './components/navigation' type Url = string | UrlObject type RequiredKeys = { @@ -104,17 +106,35 @@ export type LinkProps = InternalLinkProps type LinkPropsRequired = RequiredKeys type LinkPropsOptional = OptionalKeys -const prefetched: { [cacheKey: string]: boolean } = {} +const prefetched = new Set() function prefetch( router: NextRouter | AppRouterInstance, href: string, as: string, - options?: PrefetchOptions + options?: Omit ): void { - if (typeof window === 'undefined' || !router) return + if (typeof window === 'undefined') { + return + } + + if (!isLocalURL(href)) { + return + } + + // Resolve the locale from the router. + const locale = 'locale' in router ? router.locale : undefined + + const key = href + '%' + as + (locale ? '%' + locale : '') + + // If we've already fetched the key, then don't prefetch it again! + if (prefetched.has(key)) { + return + } + + // Mark this URL as prefetched. + prefetched.add(key) - if (!isLocalURL(href)) return // Prefetch the JSON page if asked (only in the client) // We need to handle a prefetch error here since we may be // loading with priority which can reject but we don't @@ -125,16 +145,6 @@ function prefetch( throw err } }) - - const curLocale = - options && typeof options.locale !== 'undefined' - ? options.locale - : 'locale' in router - ? router.locale - : undefined - - // Join on an invalid URI character - prefetched[href + '%' + as + (curLocale ? '%' + curLocale : '')] = true } function isModifiedEvent(event: React.MouseEvent): boolean { @@ -348,8 +358,8 @@ const Link = React.forwardRef( scroll, locale, onClick, - onMouseEnter, - onTouchStart, + onMouseEnter: onMouseEnterProp, + onTouchStart: onTouchStartProp, legacyBehavior = Boolean(process.env.__NEXT_NEW_LINK_BEHAVIOR) !== true, ...restProps } = props @@ -363,10 +373,10 @@ const Link = React.forwardRef( children = {children} } - const p = prefetchProp !== false + const prefetchEnabled = prefetchProp !== false const pagesRouter = React.useContext(RouterContext) - const appRouter = useRouter() + const appRouter = React.useContext(AppRouterContext) const router = pagesRouter ?? appRouter // We're in the app directory if there is no pages router. @@ -407,7 +417,7 @@ const Link = React.forwardRef( `"onClick" was passed to with \`href\` of \`${hrefProp}\` but "legacyBehavior" was set. The legacy behavior requires onClick be set on the child of next/link` ) } - if (onMouseEnter) { + if (onMouseEnterProp) { console.warn( `"onMouseEnter" was passed to with \`href\` of \`${hrefProp}\` but "legacyBehavior" was set. The legacy behavior requires onMouseEnter be set on the child of next/link` ) @@ -467,18 +477,29 @@ const Link = React.forwardRef( }, [as, childRef, href, resetVisible, setIntersectionRef] ) + + // Prefetch the URL if we haven't already and it's visible. React.useEffect(() => { - const shouldPrefetch = isVisible && p && isLocalURL(href) - const curLocale = - typeof locale !== 'undefined' ? locale : pagesRouter?.locale - const isPrefetched = - prefetched[href + '%' + as + (curLocale ? '%' + curLocale : '')] - if (shouldPrefetch && !isPrefetched) { - prefetch(router, href, as, { - locale: curLocale, - }) + if (!router) { + return + } + + // If we don't need to prefetch the URL, don't do prefetch. + if (!isVisible || !prefetchEnabled) { + return } - }, [as, href, isVisible, locale, p, pagesRouter?.locale, router]) + + // Prefetch the URL. + prefetch(router, href, as) + }, [ + as, + href, + isVisible, + locale, + prefetchEnabled, + pagesRouter?.locale, + router, + ]) const childProps: { onTouchStart: React.TouchEventHandler @@ -500,6 +521,7 @@ const Link = React.forwardRef( if (!legacyBehavior && typeof onClick === 'function') { onClick(e) } + if ( legacyBehavior && child.props && @@ -507,25 +529,33 @@ const Link = React.forwardRef( ) { child.props.onClick(e) } - if (!e.defaultPrevented) { - linkClicked( - e, - router, - href, - as, - replace, - shallow, - scroll, - locale, - isAppRouter, - p - ) + + if (!router) { + return } + + if (e.defaultPrevented) { + return + } + + linkClicked( + e, + router, + href, + as, + replace, + shallow, + scroll, + locale, + isAppRouter, + prefetchEnabled + ) }, onMouseEnter: (e: React.MouseEvent) => { - if (!legacyBehavior && typeof onMouseEnter === 'function') { - onMouseEnter(e) + if (!legacyBehavior && typeof onMouseEnterProp === 'function') { + onMouseEnterProp(e) } + if ( legacyBehavior && child.props && @@ -534,16 +564,20 @@ const Link = React.forwardRef( child.props.onMouseEnter(e) } + if (!router) { + return + } + // Check for not prefetch disabled in page using appRouter - if (!(!p && isAppRouter)) { - if (isLocalURL(href)) { - prefetch(router, href, as, { priority: true }) - } + if (!prefetchEnabled && isAppRouter) { + return } + + prefetch(router, href, as, { priority: true }) }, onTouchStart: (e: React.TouchEvent) => { - if (!legacyBehavior && typeof onTouchStart === 'function') { - onTouchStart(e) + if (!legacyBehavior && typeof onTouchStartProp === 'function') { + onTouchStartProp(e) } if ( @@ -554,12 +588,16 @@ const Link = React.forwardRef( child.props.onTouchStart(e) } + if (!router) { + return + } + // Check for not prefetch disabled in page using appRouter - if (!(!p && isAppRouter)) { - if (isLocalURL(href)) { - prefetch(router, href, as, { priority: true }) - } + if (!prefetchEnabled && isAppRouter) { + return } + + prefetch(router, href, as, { priority: true }) }, } From bd6722279587bd40024a73a23883e6e907cd4124 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 27 Oct 2022 12:56:44 -0700 Subject: [PATCH 07/12] refactor: update `useRouter` from `next/router` types --- .eslintrc.json | 7 +++++-- packages/next/client/router.ts | 6 ++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/.eslintrc.json b/.eslintrc.json index 8eec065efcf790b..21089d04c71aa8f 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -238,7 +238,6 @@ "no-obj-calls": "warn", "no-octal": "warn", "no-octal-escape": "warn", - "no-redeclare": ["warn", { "builtinGlobals": false }], "no-regex-spaces": "warn", "no-restricted-syntax": [ "warn", @@ -330,6 +329,10 @@ "react/style-prop-object": "warn", "react-hooks/rules-of-hooks": "error", // "@typescript-eslint/non-nullable-type-assertion-style": "warn", - "@typescript-eslint/prefer-as-const": "warn" + "@typescript-eslint/prefer-as-const": "warn", + "@typescript-eslint/no-redeclare": [ + "warn", + { "builtinGlobals": false, "ignoreDeclarationMerge": true } + ] } } diff --git a/packages/next/client/router.ts b/packages/next/client/router.ts index beb0efe0c74415b..14664a831f37b3d 100644 --- a/packages/next/client/router.ts +++ b/packages/next/client/router.ts @@ -129,9 +129,11 @@ export default singletonRouter as SingletonRouter // Reexport the withRoute HOC export { default as withRouter } from './with-router' -export function useRouter(): NextRouter { +export function useRouter(throwOnMissing: true): NextRouter +export function useRouter(): NextRouter | null +export function useRouter(throwOnMissing?: boolean) { const router = React.useContext(RouterContext) - if (!router) { + if (!router && throwOnMissing) { throw new Error('invariant expected pages router to be mounted') } From f10773b5076c1a78df4a0fd192144da553745dfc Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 27 Oct 2022 13:06:03 -0700 Subject: [PATCH 08/12] fix: fixed missing boolean --- packages/next/client/route-announcer.tsx | 2 +- packages/next/shared/lib/router/router.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/next/client/route-announcer.tsx b/packages/next/client/route-announcer.tsx index 3b59fd84d96bcbd..cbd26f09ea7bda4 100644 --- a/packages/next/client/route-announcer.tsx +++ b/packages/next/client/route-announcer.tsx @@ -17,7 +17,7 @@ const nextjsRouteAnnouncerStyles: React.CSSProperties = { } export const RouteAnnouncer = () => { - const { asPath } = useRouter() + const { asPath } = useRouter(true) const [routeAnnouncement, setRouteAnnouncement] = React.useState('') // Only announce the path change, but not for the first load because screen diff --git a/packages/next/shared/lib/router/router.ts b/packages/next/shared/lib/router/router.ts index 90c6d63968a49b4..e7d9610913e7d8b 100644 --- a/packages/next/shared/lib/router/router.ts +++ b/packages/next/shared/lib/router/router.ts @@ -122,11 +122,11 @@ function stripOrigin(url: string) { return url.startsWith(origin) ? url.substring(origin.length) : url } -function omit( +function omit( object: T, keys: K[] ): Omit { - const omitted: { [key: string]: any } = {} + const omitted: { [key: string]: unknown } = {} Object.keys(object).forEach((key) => { if (!keys.includes(key as K)) { omitted[key] = object[key] From ba057e07cbfbfca8db4351c68b4a4bee6158f00b Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 27 Oct 2022 16:33:51 -0700 Subject: [PATCH 09/12] fix: linting + test fixes --- packages/next/client/link.tsx | 25 ++++++++++--------- packages/next/shared/lib/loadable.d.ts | 2 +- .../app/components/router-hooks-fixtures.js | 3 +-- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/packages/next/client/link.tsx b/packages/next/client/link.tsx index 206c4baf9cf7bd4..a004ef005d77b55 100644 --- a/packages/next/client/link.tsx +++ b/packages/next/client/link.tsx @@ -78,7 +78,7 @@ type InternalLinkProps = { */ locale?: string | false /** - * Enable legacy link behaviour. + * Enable legacy link behavior. * @defaultValue `false` * @see https://github.com/vercel/next.js/commit/489e65ed98544e69b0afd7e0cfc3f9f6c2b803b7 */ @@ -122,18 +122,21 @@ function prefetch( return } - // Resolve the locale from the router. - const locale = 'locale' in router ? router.locale : undefined + // We should only dedupe requests when experimental.optimisticClientCache is + // disabled. + if (!process.env.__NEXT_OPTIMISTIC_CLIENT_CACHE) { + const locale = 'locale' in router ? router.locale : undefined - const key = href + '%' + as + (locale ? '%' + locale : '') + const key = href + '%' + as + (locale ? '%' + locale : '') - // If we've already fetched the key, then don't prefetch it again! - if (prefetched.has(key)) { - return - } + // If we've already fetched the key, then don't prefetch it again! + if (prefetched.has(key)) { + return + } - // Mark this URL as prefetched. - prefetched.add(key) + // Mark this URL as prefetched. + prefetched.add(key) + } // Prefetch the JSON page if asked (only in the client) // We need to handle a prefetch error here since we may be @@ -568,7 +571,6 @@ const Link = React.forwardRef( return } - // Check for not prefetch disabled in page using appRouter if (!prefetchEnabled && isAppRouter) { return } @@ -592,7 +594,6 @@ const Link = React.forwardRef( return } - // Check for not prefetch disabled in page using appRouter if (!prefetchEnabled && isAppRouter) { return } diff --git a/packages/next/shared/lib/loadable.d.ts b/packages/next/shared/lib/loadable.d.ts index 505fa09be9c3c3f..8ac6e8043379751 100644 --- a/packages/next/shared/lib/loadable.d.ts +++ b/packages/next/shared/lib/loadable.d.ts @@ -9,7 +9,7 @@ declare namespace LoadableExport { } } -// eslint-disable-next-line no-redeclare +// eslint-disable-next-line @typescript-eslint/no-redeclare declare const LoadableExport: LoadableExport.ILoadable export = LoadableExport diff --git a/test/e2e/app-dir/app/components/router-hooks-fixtures.js b/test/e2e/app-dir/app/components/router-hooks-fixtures.js index 465ff3b665ca7be..95f62d6f6fd7f35 100644 --- a/test/e2e/app-dir/app/components/router-hooks-fixtures.js +++ b/test/e2e/app-dir/app/components/router-hooks-fixtures.js @@ -4,8 +4,7 @@ import { useRouter as useAppRouter, useSearchParams, } from 'next/navigation' -import { useState } from 'react' -import { useEffect } from 'react' +import { useState, useEffect } from 'react' export const RouterHooksFixtures = () => { const pagesRouter = usePagesRouter() From 160dba81b640b3c9006cc186b9d04c09e45c6b75 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Fri, 28 Oct 2022 11:07:11 -0700 Subject: [PATCH 10/12] fix: adjust prefetching on hover + touchStart --- packages/next/client/link.tsx | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/packages/next/client/link.tsx b/packages/next/client/link.tsx index a004ef005d77b55..cc818285bcdc807 100644 --- a/packages/next/client/link.tsx +++ b/packages/next/client/link.tsx @@ -5,7 +5,7 @@ import { UrlObject } from 'url' import { isLocalURL, NextRouter, - PrefetchOptions, + PrefetchOptions as RouterPrefetchOptions, resolveHref, } from '../shared/lib/router/router' import { formatUrl } from '../shared/lib/router/utils/format-url' @@ -108,11 +108,19 @@ type LinkPropsOptional = OptionalKeys const prefetched = new Set() +type PrefetchOptions = Omit & { + /** + * bypassPrefetchedCheck will bypass the check to see if the `href` has + * already been fetched. + */ + bypassPrefetchedCheck?: boolean +} + function prefetch( router: NextRouter | AppRouterInstance, href: string, as: string, - options?: Omit + options?: PrefetchOptions ): void { if (typeof window === 'undefined') { return @@ -124,7 +132,7 @@ function prefetch( // We should only dedupe requests when experimental.optimisticClientCache is // disabled. - if (!process.env.__NEXT_OPTIMISTIC_CLIENT_CACHE) { + if (!options?.bypassPrefetchedCheck) { const locale = 'locale' in router ? router.locale : undefined const key = href + '%' + as + (locale ? '%' + locale : '') @@ -575,7 +583,11 @@ const Link = React.forwardRef( return } - prefetch(router, href, as, { priority: true }) + prefetch(router, href, as, { + priority: true, + // @see {https://github.com/vercel/next.js/discussions/40268?sort=top#discussioncomment-3572642} + bypassPrefetchedCheck: true, + }) }, onTouchStart: (e: React.TouchEvent) => { if (!legacyBehavior && typeof onTouchStartProp === 'function') { @@ -598,7 +610,11 @@ const Link = React.forwardRef( return } - prefetch(router, href, as, { priority: true }) + prefetch(router, href, as, { + priority: true, + // @see {https://github.com/vercel/next.js/discussions/40268?sort=top#discussioncomment-3572642} + bypassPrefetchedCheck: true, + }) }, } From ff11ef7d7fdfeb8f784a690e0b3179c570568d2c Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Fri, 28 Oct 2022 11:48:39 -0700 Subject: [PATCH 11/12] fix: restore locale handing --- packages/next/client/link.tsx | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/packages/next/client/link.tsx b/packages/next/client/link.tsx index cc818285bcdc807..6e9156d7f6aa4a7 100644 --- a/packages/next/client/link.tsx +++ b/packages/next/client/link.tsx @@ -108,7 +108,7 @@ type LinkPropsOptional = OptionalKeys const prefetched = new Set() -type PrefetchOptions = Omit & { +type PrefetchOptions = RouterPrefetchOptions & { /** * bypassPrefetchedCheck will bypass the check to see if the `href` has * already been fetched. @@ -120,7 +120,7 @@ function prefetch( router: NextRouter | AppRouterInstance, href: string, as: string, - options?: PrefetchOptions + options: PrefetchOptions ): void { if (typeof window === 'undefined') { return @@ -132,18 +132,25 @@ function prefetch( // We should only dedupe requests when experimental.optimisticClientCache is // disabled. - if (!options?.bypassPrefetchedCheck) { - const locale = 'locale' in router ? router.locale : undefined - - const key = href + '%' + as + (locale ? '%' + locale : '') + if (!options.bypassPrefetchedCheck) { + const locale = + // Let the link's locale prop override the default router locale. + typeof options.locale === 'string' + ? options.locale + : // Otherwise fallback to the router's locale. + 'locale' in router + ? router.locale + : undefined + + const prefetchedKey = href + '%' + as + (locale ? '%' + locale : '') // If we've already fetched the key, then don't prefetch it again! - if (prefetched.has(key)) { + if (prefetched.has(prefetchedKey)) { return } // Mark this URL as prefetched. - prefetched.add(key) + prefetched.add(prefetchedKey) } // Prefetch the JSON page if asked (only in the client) @@ -501,7 +508,7 @@ const Link = React.forwardRef( } // Prefetch the URL. - prefetch(router, href, as) + prefetch(router, href, as, { locale }) }, [ as, href, @@ -584,6 +591,7 @@ const Link = React.forwardRef( } prefetch(router, href, as, { + locale, priority: true, // @see {https://github.com/vercel/next.js/discussions/40268?sort=top#discussioncomment-3572642} bypassPrefetchedCheck: true, @@ -611,6 +619,7 @@ const Link = React.forwardRef( } prefetch(router, href, as, { + locale, priority: true, // @see {https://github.com/vercel/next.js/discussions/40268?sort=top#discussioncomment-3572642} bypassPrefetchedCheck: true, From 879407f8415d982fdda377363189426162ee6e89 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Fri, 28 Oct 2022 12:19:08 -0700 Subject: [PATCH 12/12] fix: fixed tests, narrowed prefetched cache --- packages/next/client/link.tsx | 4 ++-- test/integration/typescript/pages/hello.tsx | 2 +- test/integration/typescript/test/index.test.js | 2 ++ 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/next/client/link.tsx b/packages/next/client/link.tsx index 6e9156d7f6aa4a7..22fe09d5a0d6690 100644 --- a/packages/next/client/link.tsx +++ b/packages/next/client/link.tsx @@ -135,14 +135,14 @@ function prefetch( if (!options.bypassPrefetchedCheck) { const locale = // Let the link's locale prop override the default router locale. - typeof options.locale === 'string' + typeof options.locale !== 'undefined' ? options.locale : // Otherwise fallback to the router's locale. 'locale' in router ? router.locale : undefined - const prefetchedKey = href + '%' + as + (locale ? '%' + locale : '') + const prefetchedKey = href + '%' + as + '%' + locale // If we've already fetched the key, then don't prefetch it again! if (prefetched.has(prefetchedKey)) { diff --git a/test/integration/typescript/pages/hello.tsx b/test/integration/typescript/pages/hello.tsx index acfd061ee410906..1e5c82d05ec5cf9 100644 --- a/test/integration/typescript/pages/hello.tsx +++ b/test/integration/typescript/pages/hello.tsx @@ -31,7 +31,7 @@ class Test2 extends Test { new Test2().show() export default function HelloPage(): JSX.Element { - const router = useRouter() + const router = useRouter(true) console.log(process.browser) console.log(router.pathname) console.log(router.isReady) diff --git a/test/integration/typescript/test/index.test.js b/test/integration/typescript/test/index.test.js index c9ba200286d81d2..3268404f6c7d578 100644 --- a/test/integration/typescript/test/index.test.js +++ b/test/integration/typescript/test/index.test.js @@ -142,6 +142,7 @@ export default function EvilPage(): JSX.Element { try { errorPage.replace('static ', 'static async ') const output = await nextBuild(appDir, [], { stdout: true }) + expect(output.stdout).toMatch(/Compiled successfully/) } finally { errorPage.restore() @@ -153,6 +154,7 @@ export default function EvilPage(): JSX.Element { try { page.replace(/async \(/g, '(') const output = await nextBuild(appDir, [], { stdout: true }) + expect(output.stdout).toMatch(/Compiled successfully/) } finally { page.restore()