From 3e59eb73ce9100bcdfd2417c2c2c941b7585d070 Mon Sep 17 00:00:00 2001 From: Gerald Monaco Date: Wed, 2 Feb 2022 20:22:07 +0000 Subject: [PATCH 1/3] Make router state read-only --- packages/next/shared/lib/router/router.ts | 205 +++++++++++++--------- 1 file changed, 125 insertions(+), 80 deletions(-) diff --git a/packages/next/shared/lib/router/router.ts b/packages/next/shared/lib/router/router.ts index d828f29d9774..e3430c8a335f 100644 --- a/packages/next/shared/lib/router/router.ts +++ b/packages/next/shared/lib/router/router.ts @@ -596,10 +596,6 @@ interface NextDataCache { } export default class Router implements BaseRouter { - route: string - pathname: string - query: ParsedUrlQuery - asPath: string basePath: string /** @@ -620,17 +616,24 @@ export default class Router implements BaseRouter { events: MittEmitter _wrapApp: (App: AppComponent) => any isSsr: boolean - isFallback: boolean _inFlightRoute?: string _shallow?: boolean - locale?: string locales?: string[] defaultLocale?: string domainLocales?: DomainLocale[] - isReady: boolean - isPreview: boolean isLocaleDomain: boolean + private state: Readonly<{ + route: string + pathname: string + query: ParsedUrlQuery + asPath: string + locale: string | undefined + isFallback: boolean + isReady: boolean + isPreview: boolean + }> + private _idx: number = 0 static events: MittEmitter = mitt() @@ -670,7 +673,7 @@ export default class Router implements BaseRouter { } ) { // represents the current component key - this.route = removePathTrailingSlash(pathname) + const route = removePathTrailingSlash(pathname) // set up the component cache (by route keys) this.components = {} @@ -678,7 +681,7 @@ export default class Router implements BaseRouter { // Otherwise, this cause issues when when going back and // come again to the errored page. if (pathname !== '/_error') { - this.components[this.route] = { + this.components[route] = { Component, initial: true, props: initialProps, @@ -701,14 +704,11 @@ export default class Router implements BaseRouter { this.events = Router.events this.pageLoader = pageLoader - this.pathname = pathname - this.query = query // if auto prerendered and dynamic route wait to update asPath // until after mount to prevent hydration mismatch const autoExportDynamic = isDynamicRoute(pathname) && self.__NEXT_DATA__.autoExport - this.asPath = autoExportDynamic ? pathname : as this.basePath = basePath this.sub = subscription this.clc = null @@ -716,22 +716,9 @@ export default class Router implements BaseRouter { // make sure to ignore extra popState in safari on navigating // back from external site this.isSsr = true - - this.isFallback = isFallback - - this.isReady = !!( - self.__NEXT_DATA__.gssp || - self.__NEXT_DATA__.gip || - (self.__NEXT_DATA__.appGip && !self.__NEXT_DATA__.gsp) || - (!autoExportDynamic && - !self.location.search && - !process.env.__NEXT_HAS_REWRITES) - ) - this.isPreview = !!isPreview this.isLocaleDomain = false if (process.env.__NEXT_I18N_SUPPORT) { - this.locale = locale this.locales = locales this.defaultLocale = defaultLocale this.domainLocales = domainLocales @@ -741,6 +728,24 @@ export default class Router implements BaseRouter { ) } + this.state = { + route, + pathname, + query, + asPath: autoExportDynamic ? pathname : as, + isPreview: !!isPreview, + locale: process.env.__NEXT_I18N_SUPPORT ? locale : undefined, + isFallback, + isReady: !!( + self.__NEXT_DATA__.gssp || + self.__NEXT_DATA__.gip || + (self.__NEXT_DATA__.appGip && !self.__NEXT_DATA__.gsp) || + (!autoExportDynamic && + !self.location.search && + !process.env.__NEXT_HAS_REWRITES) + ), + } + if (typeof window !== 'undefined') { // make sure "as" doesn't start with double slashes or else it can // throw an error as it's considered invalid @@ -913,22 +918,26 @@ export default class Router implements BaseRouter { (options as any)._shouldResolveHref || pathNoQueryHash(url) === pathNoQueryHash(as) + const nextState = { + ...this.state, + } + // for static pages with query params in the URL we delay // marking the router ready until after the query is updated if ((options as any)._h) { - this.isReady = true + nextState.isReady = true } - const prevLocale = this.locale + const prevLocale = nextState.locale if (process.env.__NEXT_I18N_SUPPORT) { - this.locale = + nextState.locale = options.locale === false ? this.defaultLocale - : options.locale || this.locale + : options.locale || nextState.locale if (typeof options.locale === 'undefined') { - options.locale = this.locale + options.locale = nextState.locale } const parsedAs = parseRelativeUrl(hasBasePath(as) ? delBasePath(as) : as) @@ -938,7 +947,7 @@ export default class Router implements BaseRouter { ) if (localePathResult.detectedLocale) { - this.locale = localePathResult.detectedLocale + nextState.locale = localePathResult.detectedLocale parsedAs.pathname = addBasePath(parsedAs.pathname) as = formatWithValidation(parsedAs) url = addBasePath( @@ -954,8 +963,8 @@ export default class Router implements BaseRouter { // moves this on its own due to the return if (process.env.__NEXT_I18N_SUPPORT) { // if the locale isn't configured hard navigate to show 404 page - if (!this.locales?.includes(this.locale!)) { - parsedAs.pathname = addLocale(parsedAs.pathname, this.locale) + if (!this.locales?.includes(nextState.locale!)) { + parsedAs.pathname = addLocale(parsedAs.pathname, nextState.locale) window.location.href = formatWithValidation(parsedAs) // this was previously a return but was removed in favor // of better dead code elimination with regenerator runtime @@ -966,7 +975,7 @@ export default class Router implements BaseRouter { const detectedDomain = detectDomainLocale( this.domainLocales, undefined, - this.locale + nextState.locale ) // we need to wrap this in the env check again since regenerator runtime @@ -985,9 +994,9 @@ export default class Router implements BaseRouter { detectedDomain.domain }${addBasePath( `${ - this.locale === detectedDomain.defaultLocale + nextState.locale === detectedDomain.defaultLocale ? '' - : `/${this.locale}` + : `/${nextState.locale}` }${asNoBasePath === '/' ? '' : asNoBasePath}` || '/' )}` // this was previously a return but was removed in favor @@ -1025,11 +1034,11 @@ export default class Router implements BaseRouter { ) const cleanedAs = delLocale( hasBasePath(as) ? delBasePath(as) : as, - this.locale + nextState.locale ) this._inFlightRoute = as - let localeChange = prevLocale !== this.locale + let localeChange = prevLocale !== nextState.locale // If the url change is only related to a hash change // We should not proceed. We should only change the state. @@ -1042,12 +1051,12 @@ export default class Router implements BaseRouter { this.onlyAHashChange(cleanedAs) && !localeChange ) { - this.asPath = cleanedAs + nextState.asPath = cleanedAs Router.events.emit('hashChangeStart', as, routeProps) // TODO: do we need the resolved href when only a hash change? this.changeState(method, url, as, options) this.scrollToHash(cleanedAs) - this.notify(this.components[this.route], null) + this.set(nextState, this.components[nextState.route], null) Router.events.emit('hashChangeComplete', as, routeProps) return true } @@ -1097,7 +1106,7 @@ export default class Router implements BaseRouter { if (process.env.__NEXT_HAS_REWRITES && as.startsWith('/')) { const rewritesResult = resolveRewrites( - addBasePath(addLocale(cleanedAs, this.locale)), + addBasePath(addLocale(cleanedAs, nextState.locale)), pages, rewrites, query, @@ -1141,7 +1150,7 @@ export default class Router implements BaseRouter { return false } - resolvedAs = delLocale(delBasePath(resolvedAs), this.locale) + resolvedAs = delLocale(delBasePath(resolvedAs), nextState.locale) /** * If the route update was triggered for client-side hydration and @@ -1158,6 +1167,8 @@ export default class Router implements BaseRouter { pages, pathname, query, + locale: nextState.locale, + isPreview: nextState.isPreview, }) if (effect.type === 'rewrite') { @@ -1244,7 +1255,9 @@ export default class Router implements BaseRouter { query, as, resolvedAs, - routeProps + routeProps, + nextState.locale, + nextState.isPreview ) let { error, props, __N_SSG, __N_SSP } = routeInfo @@ -1278,7 +1291,7 @@ export default class Router implements BaseRouter { return new Promise(() => {}) } - this.isPreview = !!props.__N_PREVIEW + nextState.isPreview = !!props.__N_PREVIEW // handle SSG data 404 if (props.notFound === SSG_DATA_NOT_FOUND) { @@ -1297,7 +1310,9 @@ export default class Router implements BaseRouter { query, as, resolvedAs, - { shallow: false } + { shallow: false }, + nextState.locale, + nextState.isPreview ) } } @@ -1317,15 +1332,19 @@ export default class Router implements BaseRouter { } // shallow routing is only allowed for same page URL changes. - const isValidShallowRoute = options.shallow && this.route === route + const isValidShallowRoute = options.shallow && nextState.route === route const shouldScroll = options.scroll ?? !isValidShallowRoute const resetScroll = shouldScroll ? { x: 0, y: 0 } : null + await this.set( - route, - pathname!, - query, - cleanedAs, + { + ...nextState, + route, + pathname, + query, + asPath: cleanedAs, + }, routeInfo, forcedScroll ?? resetScroll ).catch((e) => { @@ -1339,8 +1358,8 @@ export default class Router implements BaseRouter { } if (process.env.__NEXT_I18N_SUPPORT) { - if (this.locale) { - document.documentElement.lang = this.locale + if (nextState.locale) { + document.documentElement.lang = nextState.locale } } Router.events.emit('routeChangeComplete', as, routeProps) @@ -1474,7 +1493,9 @@ export default class Router implements BaseRouter { query: any, as: string, resolvedAs: string, - routeProps: RouteProperties + routeProps: RouteProperties, + locale: string | undefined, + isPreview: boolean ): Promise { try { const existingRouteInfo: PrivateRouteInfo | undefined = @@ -1522,7 +1543,7 @@ export default class Router implements BaseRouter { asPath: resolvedAs, ssg: __N_SSG, rsc: __N_RSC, - locale: this.locale, + locale, }) } @@ -1533,7 +1554,7 @@ export default class Router implements BaseRouter { this.isSsr, false, __N_SSG ? this.sdc : this.sdr, - !!__N_SSG && !this.isPreview + !!__N_SSG && !isPreview ) : this.getInitialProps( Component, @@ -1542,7 +1563,7 @@ export default class Router implements BaseRouter { pathname, query, asPath: as, - locale: this.locale, + locale, locales: this.locales, defaultLocale: this.defaultLocale, } as any @@ -1573,21 +1594,18 @@ export default class Router implements BaseRouter { } } - set( - route: string, - pathname: string, - query: ParsedUrlQuery, - as: string, + private set( + state: typeof this.state, data: PrivateRouteInfo, resetScroll: { x: number; y: number } | null ): Promise { - this.isFallback = false + this.state = state - this.route = route - this.pathname = pathname - this.query = query - this.asPath = as - return this.notify(data, resetScroll) + return this.sub( + data, + this.components['/_app'].Component as AppComponent, + resetScroll + ) } /** @@ -1728,6 +1746,8 @@ export default class Router implements BaseRouter { pages, pathname, query, + locale: this.locale, + isPreview: this.isPreview, }) if (effects.type === 'rewrite') { @@ -1833,10 +1853,12 @@ export default class Router implements BaseRouter { pages: string[] pathname: string query: ParsedUrlQuery + locale: string | undefined + isPreview: boolean }): Promise { const cleanedAs = delLocale( hasBasePath(options.as) ? delBasePath(options.as) : options.as, - this.locale + options.locale ) const fns = await this.pageLoader.getMiddlewareList() @@ -1851,6 +1873,7 @@ export default class Router implements BaseRouter { const preflight = await this._getPreflightData({ preflightHref: options.as, shouldCache: options.cache, + isPreview: options.isPreview, }) if (preflight.rewrite?.startsWith('/')) { @@ -1936,13 +1959,14 @@ export default class Router implements BaseRouter { _getPreflightData(params: { preflightHref: string shouldCache?: boolean + isPreview: boolean }): Promise { - const { preflightHref, shouldCache = false } = params + const { preflightHref, shouldCache = false, isPreview } = params const { href: cacheKey } = new URL(preflightHref, window.location.href) if ( process.env.NODE_ENV === 'production' && - !this.isPreview && + !isPreview && shouldCache && this.sde[cacheKey] ) { @@ -2008,14 +2032,35 @@ export default class Router implements BaseRouter { } } - notify( - data: PrivateRouteInfo, - resetScroll: { x: number; y: number } | null - ): Promise { - return this.sub( - data, - this.components['/_app'].Component as AppComponent, - resetScroll - ) + get route(): string { + return this.state.route + } + + get pathname(): string { + return this.state.pathname + } + + get query(): ParsedUrlQuery { + return this.state.query + } + + get asPath(): string { + return this.state.asPath + } + + get locale(): string | undefined { + return this.state.locale + } + + get isFallback(): boolean { + return this.state.isFallback + } + + get isReady(): boolean { + return this.state.isReady + } + + get isPreview(): boolean { + return this.state.isPreview } } From 3b783c4f9ba772aa2ef646546cff4badfc4ade78 Mon Sep 17 00:00:00 2001 From: Gerald Monaco Date: Wed, 2 Feb 2022 20:50:19 +0000 Subject: [PATCH 2/3] Remove isReady from state --- packages/next/shared/lib/router/router.ts | 24 ++++++++++------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/packages/next/shared/lib/router/router.ts b/packages/next/shared/lib/router/router.ts index e3430c8a335f..13037fc1cb9a 100644 --- a/packages/next/shared/lib/router/router.ts +++ b/packages/next/shared/lib/router/router.ts @@ -621,6 +621,7 @@ export default class Router implements BaseRouter { locales?: string[] defaultLocale?: string domainLocales?: DomainLocale[] + isReady: boolean isLocaleDomain: boolean private state: Readonly<{ @@ -630,7 +631,6 @@ export default class Router implements BaseRouter { asPath: string locale: string | undefined isFallback: boolean - isReady: boolean isPreview: boolean }> @@ -717,6 +717,14 @@ export default class Router implements BaseRouter { // back from external site this.isSsr = true this.isLocaleDomain = false + this.isReady = !!( + self.__NEXT_DATA__.gssp || + self.__NEXT_DATA__.gip || + (self.__NEXT_DATA__.appGip && !self.__NEXT_DATA__.gsp) || + (!autoExportDynamic && + !self.location.search && + !process.env.__NEXT_HAS_REWRITES) + ) if (process.env.__NEXT_I18N_SUPPORT) { this.locales = locales @@ -736,14 +744,6 @@ export default class Router implements BaseRouter { isPreview: !!isPreview, locale: process.env.__NEXT_I18N_SUPPORT ? locale : undefined, isFallback, - isReady: !!( - self.__NEXT_DATA__.gssp || - self.__NEXT_DATA__.gip || - (self.__NEXT_DATA__.appGip && !self.__NEXT_DATA__.gsp) || - (!autoExportDynamic && - !self.location.search && - !process.env.__NEXT_HAS_REWRITES) - ), } if (typeof window !== 'undefined') { @@ -925,7 +925,7 @@ export default class Router implements BaseRouter { // for static pages with query params in the URL we delay // marking the router ready until after the query is updated if ((options as any)._h) { - nextState.isReady = true + this.isReady = true } const prevLocale = nextState.locale @@ -2056,10 +2056,6 @@ export default class Router implements BaseRouter { return this.state.isFallback } - get isReady(): boolean { - return this.state.isReady - } - get isPreview(): boolean { return this.state.isPreview } From 7553e1a68d28be1f35da4d9fec0aa547e181f43e Mon Sep 17 00:00:00 2001 From: Gerald Monaco Date: Wed, 2 Feb 2022 20:58:07 +0000 Subject: [PATCH 3/3] Correctly unset isFallback --- packages/next/shared/lib/router/router.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/next/shared/lib/router/router.ts b/packages/next/shared/lib/router/router.ts index 13037fc1cb9a..6ba086cccb14 100644 --- a/packages/next/shared/lib/router/router.ts +++ b/packages/next/shared/lib/router/router.ts @@ -1344,6 +1344,7 @@ export default class Router implements BaseRouter { pathname, query, asPath: cleanedAs, + isFallback: false, }, routeInfo, forcedScroll ?? resetScroll