From 3840c08652e0a5f5889e88fa214ed3c27305ab31 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 20 Oct 2022 16:06:37 -0600 Subject: [PATCH 1/4] fix: handle case of useSearchParams when accessed from pages --- packages/next/client/components/hooks-client-context.ts | 2 +- packages/next/client/components/navigation.ts | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/next/client/components/hooks-client-context.ts b/packages/next/client/components/hooks-client-context.ts index 3fdc5395a7cf..598d52256acf 100644 --- a/packages/next/client/components/hooks-client-context.ts +++ b/packages/next/client/components/hooks-client-context.ts @@ -1,6 +1,6 @@ import { createContext } from 'react' -export const SearchParamsContext = createContext(null as any) +export const SearchParamsContext = createContext(null) export const PathnameContext = createContext(null as any) export const ParamsContext = createContext(null as any) export const LayoutSegmentsContext = createContext(null as any) diff --git a/packages/next/client/components/navigation.ts b/packages/next/client/components/navigation.ts index efca12318331..2fadaebbef0b 100644 --- a/packages/next/client/components/navigation.ts +++ b/packages/next/client/components/navigation.ts @@ -76,6 +76,11 @@ class ReadonlyURLSearchParams { export function useSearchParams() { const searchParams = useContext(SearchParamsContext) const readonlySearchParams = useMemo(() => { + if (!searchParams) { + // TODO-APP: consider throwing an error or adapting this to support pages router + return null + } + return new ReadonlyURLSearchParams(searchParams) }, [searchParams]) return readonlySearchParams From db9f5077345ddedf956ab4d758a2799b5b60b458 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 20 Oct 2022 16:07:13 -0600 Subject: [PATCH 2/4] fix: correct invalid TS types for router-dependant hooks --- packages/next/client/components/hooks-client-context.ts | 2 +- packages/next/client/components/navigation.ts | 8 ++++++-- packages/next/client/router.ts | 2 +- packages/next/shared/lib/app-router-context.ts | 4 ++-- packages/next/shared/lib/router-context.ts | 2 +- 5 files changed, 11 insertions(+), 7 deletions(-) diff --git a/packages/next/client/components/hooks-client-context.ts b/packages/next/client/components/hooks-client-context.ts index 598d52256acf..b8daa2eeec22 100644 --- a/packages/next/client/components/hooks-client-context.ts +++ b/packages/next/client/components/hooks-client-context.ts @@ -1,7 +1,7 @@ import { createContext } from 'react' export const SearchParamsContext = createContext(null) -export const PathnameContext = createContext(null as any) +export const PathnameContext = createContext(null) export const ParamsContext = createContext(null as any) export const LayoutSegmentsContext = createContext(null as any) diff --git a/packages/next/client/components/navigation.ts b/packages/next/client/components/navigation.ts index 2fadaebbef0b..c4b833ba52f7 100644 --- a/packages/next/client/components/navigation.ts +++ b/packages/next/client/components/navigation.ts @@ -90,7 +90,10 @@ export function useSearchParams() { /** * Get the router methods. For example router.push('/dashboard') */ -export function useRouter(): import('../../shared/lib/app-router-context').AppRouterInstance { +export function useRouter(): + | import('../../shared/lib/app-router-context').AppRouterInstance + | null { + // TODO-APP: consider throwing an error or adapting this to support pages router return useContext(AppRouterContext) } @@ -102,7 +105,8 @@ export function useRouter(): import('../../shared/lib/app-router-context').AppRo /** * Get the current pathname. For example usePathname() on /dashboard?foo=bar would return "/dashboard" */ -export function usePathname(): string { +export function usePathname(): string | null { + // TODO-APP: consider throwing an error or adapting this to support pages router return useContext(PathnameContext) } diff --git a/packages/next/client/router.ts b/packages/next/client/router.ts index f739b291209b..c26bba6624be 100644 --- a/packages/next/client/router.ts +++ b/packages/next/client/router.ts @@ -129,7 +129,7 @@ export default singletonRouter as SingletonRouter // Reexport the withRoute HOC export { default as withRouter } from './with-router' -export function useRouter(): NextRouter { +export function useRouter(): NextRouter | null { return React.useContext(RouterContext) } diff --git a/packages/next/shared/lib/app-router-context.ts b/packages/next/shared/lib/app-router-context.ts index d0a295d764fa..c7da1fdeff00 100644 --- a/packages/next/shared/lib/app-router-context.ts +++ b/packages/next/shared/lib/app-router-context.ts @@ -57,8 +57,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 d00df65cfeb5..4a9d9a575b89 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' From 24e6de8d1d3ad0e22176a4821893d44baab71700 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Fri, 21 Oct 2022 14:13:40 -0600 Subject: [PATCH 3/4] feat: backwards compatible router-dep hook support --- .../next/client/components/app-navigation.ts | 21 +++++ .../next/client/components/hybrid-router.ts | 16 ++++ .../next/client/components/layout-router.tsx | 6 +- packages/next/client/components/navigation.ts | 81 ++++++++++++++++--- .../react-dev-overlay/hot-reloader-client.tsx | 6 +- packages/next/client/link.tsx | 36 +++------ packages/next/client/route-announcer.tsx | 7 +- packages/next/shared/lib/router/router.ts | 11 ++- 8 files changed, 138 insertions(+), 46 deletions(-) create mode 100644 packages/next/client/components/app-navigation.ts create mode 100644 packages/next/client/components/hybrid-router.ts diff --git a/packages/next/client/components/app-navigation.ts b/packages/next/client/components/app-navigation.ts new file mode 100644 index 000000000000..bf001b2c1794 --- /dev/null +++ b/packages/next/client/components/app-navigation.ts @@ -0,0 +1,21 @@ +import { useContext } from 'react' +import { + AppRouterContext, + AppRouterInstance, +} from '../../shared/lib/app-router-context' + +/** + * useAppRouter will get the AppRouterInstance on the context if it's mounted. + * If it is not mounted, it will throw an error. This method should only be used + * when you expect only to have the app router mounted (not pages router). + * + * @returns the app router instance + */ +export function useAppRouter(): AppRouterInstance { + const router = useContext(AppRouterContext) + if (!router) { + throw new Error('invariant expected app router to be mounted') + } + + return router +} diff --git a/packages/next/client/components/hybrid-router.ts b/packages/next/client/components/hybrid-router.ts new file mode 100644 index 000000000000..023430f0bf71 --- /dev/null +++ b/packages/next/client/components/hybrid-router.ts @@ -0,0 +1,16 @@ +import { AppRouterInstance } from '../../shared/lib/app-router-context' +import { NextRouter } from '../router' + +export const HYBRID_ROUTER_TYPE = Symbol('HYBRID_ROUTER_TYPE') + +type MaskedHybridRouter = { + // Store the router type on the router via this private symbol. + [HYBRID_ROUTER_TYPE]: T +} & Router & + // Add partial fields of the other router so it's type-compatible with it, but + // it will show those extra fields as undefined. + Partial> + +export type HybridRouter = + | MaskedHybridRouter<'app', AppRouterInstance, NextRouter> + | MaskedHybridRouter<'pages', NextRouter, AppRouterInstance> diff --git a/packages/next/client/components/layout-router.tsx b/packages/next/client/components/layout-router.tsx index 698b2e7ef8c4..768738f3b39f 100644 --- a/packages/next/client/components/layout-router.tsx +++ b/packages/next/client/components/layout-router.tsx @@ -25,12 +25,12 @@ import { LayoutRouterContext, GlobalLayoutRouterContext, TemplateContext, - AppRouterContext, } from '../../shared/lib/app-router-context' import { fetchServerResponse } from './app-router' import { createInfinitePromise } from './infinite-promise' import { ErrorBoundary } from './error-boundary' import { matchSegment } from './match-segments' +import { useAppRouter } from './app-navigation' /** * Add refetch marker to router state at the point of the current layout segment. @@ -279,7 +279,7 @@ interface RedirectBoundaryProps { } function HandleRedirect({ redirect }: { redirect: string }) { - const router = useContext(AppRouterContext) + const router = useAppRouter() useEffect(() => { router.replace(redirect, {}) @@ -316,7 +316,7 @@ class RedirectErrorBoundary extends React.Component< } function RedirectBoundary({ children }: { children: React.ReactNode }) { - const router = useContext(AppRouterContext) + const router = useAppRouter() return ( {children} ) diff --git a/packages/next/client/components/navigation.ts b/packages/next/client/components/navigation.ts index c4b833ba52f7..4b67806c9d42 100644 --- a/packages/next/client/components/navigation.ts +++ b/packages/next/client/components/navigation.ts @@ -12,6 +12,9 @@ import { AppRouterContext, LayoutRouterContext, } from '../../shared/lib/app-router-context' +import { RouterContext } from '../../shared/lib/router-context' +import type { ParsedUrlQuery } from 'querystring' +import { HybridRouter, HYBRID_ROUTER_TYPE } from './hybrid-router' export { ServerInsertedHTMLContext, @@ -69,20 +72,54 @@ class ReadonlyURLSearchParams { } } +/** + * parsedURLQueryToURLSearchParams converts a parsed url query to a url search + * params object. + * + * @param query parsed url query + * @returns url search params object + */ +function parsedURLQueryToURLSearchParams( + query: ParsedUrlQuery +): URLSearchParams { + return new URLSearchParams( + Object.keys(query).reduce<[string, string][]>((acc, name) => { + const value = query[name] + if (Array.isArray(value)) { + acc.push(...value.map<[string, string]>((v) => [name, v])) + } else { + acc.push([name, value]) + } + + return acc + }, []) + ) +} + /** * Get a read-only URLSearchParams object. For example searchParams.get('foo') would return 'bar' when ?foo=bar * Learn more about URLSearchParams here: https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams */ export function useSearchParams() { + const router = useContext(RouterContext) const searchParams = useContext(SearchParamsContext) + const readonlySearchParams = useMemo(() => { - if (!searchParams) { - // TODO-APP: consider throwing an error or adapting this to support pages router - return null + // To support migration from pages to app, this adds a workaround that'll + // support the pages router here too. + if (router) { + return new ReadonlyURLSearchParams( + parsedURLQueryToURLSearchParams(router.query) + ) + } + + if (searchParams) { + return new ReadonlyURLSearchParams(searchParams) } - return new ReadonlyURLSearchParams(searchParams) - }, [searchParams]) + throw new Error('invariant at least one router was expected') + }, [router, searchParams]) + return readonlySearchParams } @@ -90,11 +127,21 @@ export function useSearchParams() { /** * Get the router methods. For example router.push('/dashboard') */ -export function useRouter(): - | import('../../shared/lib/app-router-context').AppRouterInstance - | null { - // TODO-APP: consider throwing an error or adapting this to support pages router - return useContext(AppRouterContext) +export function useRouter(): HybridRouter { + const router = useContext(RouterContext) + const appRouter = useContext(AppRouterContext) + + return useMemo(() => { + if (router) { + return { [HYBRID_ROUTER_TYPE]: 'pages', ...router } + } + + if (appRouter) { + return { [HYBRID_ROUTER_TYPE]: 'app', ...appRouter } + } + + throw new Error('invariant at least one router was expected') + }, [router, appRouter]) } // 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,8 +153,18 @@ export function useRouter(): * Get the current pathname. For example usePathname() on /dashboard?foo=bar would return "/dashboard" */ export function usePathname(): string | null { - // TODO-APP: consider throwing an error or adapting this to support pages router - return useContext(PathnameContext) + const router = useContext(RouterContext) + const pathname = useContext(PathnameContext) + + if (router) { + if (router.isReady) { + return router.asPath + } + + return null + } + + return pathname } // TODO-APP: define what should be provided through context. diff --git a/packages/next/client/components/react-dev-overlay/hot-reloader-client.tsx b/packages/next/client/components/react-dev-overlay/hot-reloader-client.tsx index 750fb7da79ac..0515c9591400 100644 --- a/packages/next/client/components/react-dev-overlay/hot-reloader-client.tsx +++ b/packages/next/client/components/react-dev-overlay/hot-reloader-client.tsx @@ -9,7 +9,6 @@ import React, { } from 'react' import stripAnsi from 'next/dist/compiled/strip-ansi' import formatWebpackMessages from '../../dev/error-overlay/format-webpack-messages' -import { useRouter } from '../navigation' import { errorOverlayReducer } from './internal/error-overlay-reducer' import { ACTION_BUILD_OK, @@ -26,6 +25,7 @@ import { useWebsocket, useWebsocketPing, } from './internal/helpers/use-websocket' +import { useAppRouter } from '../app-navigation' interface Dispatcher { onBuildOk(): void @@ -174,7 +174,7 @@ function tryApplyUpdates( function processMessage( e: any, sendMessage: any, - router: ReturnType, + router: ReturnType, dispatcher: Dispatcher ) { const obj = JSON.parse(e.data) @@ -442,7 +442,7 @@ export default function HotReload({ useWebsocketPing(webSocketRef) const sendMessage = useSendMessage(webSocketRef) - const router = useRouter() + const router = useAppRouter() useEffect(() => { const handler = (event: MessageEvent) => { if ( diff --git a/packages/next/client/link.tsx b/packages/next/client/link.tsx index 5f89822900d4..4df8c8af18e2 100644 --- a/packages/next/client/link.tsx +++ b/packages/next/client/link.tsx @@ -4,19 +4,15 @@ import React from 'react' import { UrlObject } from 'url' import { isLocalURL, - NextRouter, PrefetchOptions, resolveHref, } from '../shared/lib/router/router' import { addLocale } from './add-locale' -import { RouterContext } from '../shared/lib/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' +import { HybridRouter, HYBRID_ROUTER_TYPE } from './components/hybrid-router' type Url = string | UrlObject type RequiredKeys = { @@ -108,7 +104,7 @@ type LinkPropsOptional = OptionalKeys const prefetched: { [cacheKey: string]: boolean } = {} function prefetch( - router: NextRouter, + router: HybridRouter, href: string, as: string, options?: PrefetchOptions @@ -148,7 +144,7 @@ function isModifiedEvent(event: React.MouseEvent): boolean { function linkClicked( e: React.MouseEvent, - router: NextRouter | AppRouterInstance, + router: HybridRouter, href: string, as: string, replace?: boolean, @@ -171,19 +167,14 @@ function linkClicked( e.preventDefault() const navigate = () => { - // If the router is an NextRouter instance it will have `beforePopState` - if ('beforePopState' in router) { + if (router[HYBRID_ROUTER_TYPE] === 'pages') { router[replace ? 'replace' : 'push'](href, as, { shallow, locale, 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, }) } @@ -357,13 +348,8 @@ 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 router = useRouter() + const isAppRouter = router[HYBRID_ROUTER_TYPE] === 'app' const { href, as } = React.useMemo(() => { const [resolvedHref, resolvedAs] = resolveHref(router, hrefProp, true) @@ -487,7 +473,7 @@ const Link = React.forwardRef( shallow, scroll, locale, - Boolean(appRouter), + isAppRouter, p ) } @@ -505,7 +491,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 }) } @@ -525,7 +511,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 }) } diff --git a/packages/next/client/route-announcer.tsx b/packages/next/client/route-announcer.tsx index 3b59fd84d96b..6d9b14a538cd 100644 --- a/packages/next/client/route-announcer.tsx +++ b/packages/next/client/route-announcer.tsx @@ -17,7 +17,12 @@ const nextjsRouteAnnouncerStyles: React.CSSProperties = { } export const RouteAnnouncer = () => { - const { asPath } = useRouter() + const router = useRouter() + if (!router) { + throw new Error('invariant expected pages router to be mounted') + } + + const { asPath } = router 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 ec609730f24c..d7ea5a842576 100644 --- a/packages/next/shared/lib/router/router.ts +++ b/packages/next/shared/lib/router/router.ts @@ -48,6 +48,7 @@ import { getNextPathnameInfo } from './utils/get-next-pathname-info' import { formatNextPathnameInfo } from './utils/format-next-pathname-info' import { compareRouterStates } from './utils/compare-states' import { isBot } from './utils/is-bot' +import { AppRouterInstance } from '../app-router-context' declare global { interface Window { @@ -220,7 +221,7 @@ export function interpolateAs( * Preserves absolute urls. */ export function resolveHref( - router: NextRouter, + router: NextRouter | AppRouterInstance, href: Url, resolveAs?: boolean ): string { @@ -252,7 +253,13 @@ export function resolveHref( try { base = new URL( - urlAsString.startsWith('#') ? router.asPath : router.pathname, + // TODO-APP: investigate if this is the intended beheviour + 'asPath' in router + ? urlAsString.startsWith('#') + ? router.asPath + : router.pathname + : // Emulate the fallback in the catch below + '/', 'http://n' ) } catch (_) { From 8e1b75475e632d71c4d333570fa444ecb750555d Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Sun, 23 Oct 2022 14:16:21 -0600 Subject: [PATCH 4/4] fix: handle case when router query is empty (not ready) --- packages/next/client/components/navigation.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/next/client/components/navigation.ts b/packages/next/client/components/navigation.ts index 4b67806c9d42..75e80f28a000 100644 --- a/packages/next/client/components/navigation.ts +++ b/packages/next/client/components/navigation.ts @@ -108,6 +108,10 @@ export function useSearchParams() { // To support migration from pages to app, this adds a workaround that'll // support the pages router here too. if (router) { + if (!router.isReady) { + return new ReadonlyURLSearchParams(new URLSearchParams()) + } + return new ReadonlyURLSearchParams( parsedURLQueryToURLSearchParams(router.query) )