From dbc7c90e5c65af5b4269f2e1cb3bec46f0644c67 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Sun, 27 Aug 2023 01:11:13 +0200 Subject: [PATCH 1/6] refactor: share utils and optimize segments normalization --- .../build/webpack/loaders/next-app-loader.ts | 5 +--- .../src/client/components/match-segments.ts | 4 ++++ .../router-reducer/compute-changed-path.ts | 23 ++++++++----------- 3 files changed, 14 insertions(+), 18 deletions(-) diff --git a/packages/next/src/build/webpack/loaders/next-app-loader.ts b/packages/next/src/build/webpack/loaders/next-app-loader.ts index e0922628c7795..cdb47beab97d2 100644 --- a/packages/next/src/build/webpack/loaders/next-app-loader.ts +++ b/packages/next/src/build/webpack/loaders/next-app-loader.ts @@ -22,6 +22,7 @@ import { AppBundlePathNormalizer } from '../../../server/future/normalizers/buil import { MiddlewareConfig } from '../../analysis/get-page-static-info' import { getFilenameAndExtension } from './next-metadata-route-loader' import { loadEntrypoint } from './next-route-loader/load-entrypoint' +import { isGroupSegment } from '../../../client/components/match-segments' export type AppLoaderOptions = { name: string @@ -75,10 +76,6 @@ export type ComponentsType = { readonly defaultPage?: ModuleReference } -function isGroupSegment(segment: string) { - return segment.startsWith('(') && segment.endsWith(')') -} - async function createAppRouteCode({ name, page, diff --git a/packages/next/src/client/components/match-segments.ts b/packages/next/src/client/components/match-segments.ts index 6fa99c98b299e..0798fb06c4c18 100644 --- a/packages/next/src/client/components/match-segments.ts +++ b/packages/next/src/client/components/match-segments.ts @@ -33,3 +33,7 @@ export const canSegmentBeOverridden = ( return getSegmentParam(existingSegment)?.param === segment[0] } + +export function isGroupSegment(segment: string) { + return segment.startsWith('(') && segment.endsWith(')') +} diff --git a/packages/next/src/client/components/router-reducer/compute-changed-path.ts b/packages/next/src/client/components/router-reducer/compute-changed-path.ts index 763b689083aa6..ba44c1183b797 100644 --- a/packages/next/src/client/components/router-reducer/compute-changed-path.ts +++ b/packages/next/src/client/components/router-reducer/compute-changed-path.ts @@ -1,6 +1,6 @@ import { FlightRouterState, Segment } from '../../../server/app-render/types' import { INTERCEPTION_ROUTE_MARKERS } from '../../../server/future/helpers/interception-routes' -import { matchSegment } from '../match-segments' +import { isGroupSegment, matchSegment } from '../match-segments' const segmentToPathname = (segment: Segment): string => { if (typeof segment === 'string') { @@ -10,13 +10,10 @@ const segmentToPathname = (segment: Segment): string => { return segment[1] } -function normalizePathname(pathname: string): string { +function normalizeSegments(segments: string[]): string { return ( - pathname.split('/').reduce((acc, segment) => { - if ( - segment === '' || - (segment.startsWith('(') && segment.endsWith(')')) - ) { + segments.reduce((acc, segment) => { + if (segment === '' || isGroupSegment(segment)) { return acc } @@ -40,8 +37,7 @@ export function extractPathFromFlightRouterState( if (segment.startsWith('__PAGE__')) return '' - const path = [segment] - + const segments = [segment] const parallelRoutes = flightRouterState[1] ?? {} const childrenPath = parallelRoutes.children @@ -49,7 +45,7 @@ export function extractPathFromFlightRouterState( : undefined if (childrenPath !== undefined) { - path.push(childrenPath) + segments.push(childrenPath) } else { for (const [key, value] of Object.entries(parallelRoutes)) { if (key === 'children') continue @@ -57,13 +53,12 @@ export function extractPathFromFlightRouterState( const childPath = extractPathFromFlightRouterState(value) if (childPath !== undefined) { - path.push(childPath) + segments.push(childPath) } } } - // TODO-APP: optimise this, it's not ideal to join and split - return normalizePathname(path.join('/')) + return normalizeSegments(segments) } function computeChangedPathImpl( @@ -116,5 +111,5 @@ export function computeChangedPath( } // lightweight normalization to remove route groups - return normalizePathname(changedPath) + return normalizeSegments(changedPath.split('/')) } From 01d7adf0f03694907441b414439638d639cb6442 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Sun, 27 Aug 2023 01:26:30 +0200 Subject: [PATCH 2/6] fix --- packages/next/src/client/components/match-segments.ts | 3 ++- .../components/router-reducer/compute-changed-path.ts | 8 ++++++-- .../components/router-reducer/should-hard-navigate.ts | 7 +------ packages/next/src/shared/lib/router/utils/app-paths.ts | 3 ++- 4 files changed, 11 insertions(+), 10 deletions(-) diff --git a/packages/next/src/client/components/match-segments.ts b/packages/next/src/client/components/match-segments.ts index 0798fb06c4c18..b139cc012ff81 100644 --- a/packages/next/src/client/components/match-segments.ts +++ b/packages/next/src/client/components/match-segments.ts @@ -35,5 +35,6 @@ export const canSegmentBeOverridden = ( } export function isGroupSegment(segment: string) { - return segment.startsWith('(') && segment.endsWith(')') + // Use array[0] for performant purpose + return segment[0] === '(' && segment.endsWith(')') } diff --git a/packages/next/src/client/components/router-reducer/compute-changed-path.ts b/packages/next/src/client/components/router-reducer/compute-changed-path.ts index ba44c1183b797..d3c1bd0c5aef1 100644 --- a/packages/next/src/client/components/router-reducer/compute-changed-path.ts +++ b/packages/next/src/client/components/router-reducer/compute-changed-path.ts @@ -2,6 +2,10 @@ import { FlightRouterState, Segment } from '../../../server/app-render/types' import { INTERCEPTION_ROUTE_MARKERS } from '../../../server/future/helpers/interception-routes' import { isGroupSegment, matchSegment } from '../match-segments' +const removeLeadingSlash = (segment: string): string => { + return segment[0] === '/' ? segment.slice(1) : segment +} + const segmentToPathname = (segment: Segment): string => { if (typeof segment === 'string') { return segment @@ -17,7 +21,7 @@ function normalizeSegments(segments: string[]): string { return acc } - return `${acc}/${segment}` + return `${acc}/${removeLeadingSlash(segment)}` }, '') || '/' ) } @@ -52,7 +56,7 @@ export function extractPathFromFlightRouterState( const childPath = extractPathFromFlightRouterState(value) - if (childPath !== undefined) { + if (childPath !== undefined && childPath !== '') { segments.push(childPath) } } diff --git a/packages/next/src/client/components/router-reducer/should-hard-navigate.ts b/packages/next/src/client/components/router-reducer/should-hard-navigate.ts index d5b800882bf4b..bebc95d2bcbab 100644 --- a/packages/next/src/client/components/router-reducer/should-hard-navigate.ts +++ b/packages/next/src/client/components/router-reducer/should-hard-navigate.ts @@ -1,7 +1,6 @@ import type { FlightRouterState, FlightDataPath, - Segment, } from '../../../server/app-render/types' import { matchSegment } from '../match-segments' @@ -11,11 +10,7 @@ export function shouldHardNavigate( flightRouterState: FlightRouterState ): boolean { const [segment, parallelRoutes] = flightRouterState - // TODO-APP: Check if `as` can be replaced. - const [currentSegment, parallelRouteKey] = flightSegmentPath as [ - Segment, - string - ] + const [currentSegment, parallelRouteKey] = flightSegmentPath // Check if current segment matches the existing segment. if (!matchSegment(currentSegment, segment)) { diff --git a/packages/next/src/shared/lib/router/utils/app-paths.ts b/packages/next/src/shared/lib/router/utils/app-paths.ts index 59273bdce4ac4..d5519635c636f 100644 --- a/packages/next/src/shared/lib/router/utils/app-paths.ts +++ b/packages/next/src/shared/lib/router/utils/app-paths.ts @@ -1,3 +1,4 @@ +import { isGroupSegment } from '../../../../client/components/match-segments' import { ensureLeadingSlash } from '../../page-path/ensure-leading-slash' /** @@ -28,7 +29,7 @@ export function normalizeAppPath(route: string) { } // Groups are ignored. - if (segment[0] === '(' && segment.endsWith(')')) { + if (isGroupSegment(segment)) { return pathname } From b9b1c19958e58fb7ecd281885e3e2636b67c5015 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Sun, 27 Aug 2023 02:23:15 +0200 Subject: [PATCH 3/6] fix --- .../client/components/router-reducer/compute-changed-path.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/next/src/client/components/router-reducer/compute-changed-path.ts b/packages/next/src/client/components/router-reducer/compute-changed-path.ts index d3c1bd0c5aef1..12f0baddf7a71 100644 --- a/packages/next/src/client/components/router-reducer/compute-changed-path.ts +++ b/packages/next/src/client/components/router-reducer/compute-changed-path.ts @@ -17,11 +17,12 @@ const segmentToPathname = (segment: Segment): string => { function normalizeSegments(segments: string[]): string { return ( segments.reduce((acc, segment) => { + segment = removeLeadingSlash(segment) if (segment === '' || isGroupSegment(segment)) { return acc } - return `${acc}/${removeLeadingSlash(segment)}` + return `${acc}/${segment}` }, '') || '/' ) } From e0113df9b58f44ed4920d61cabd31723694f20f6 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Sun, 27 Aug 2023 03:34:30 +0200 Subject: [PATCH 4/6] revert --- .../client/components/router-reducer/compute-changed-path.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/next/src/client/components/router-reducer/compute-changed-path.ts b/packages/next/src/client/components/router-reducer/compute-changed-path.ts index 12f0baddf7a71..34b5afb891a08 100644 --- a/packages/next/src/client/components/router-reducer/compute-changed-path.ts +++ b/packages/next/src/client/components/router-reducer/compute-changed-path.ts @@ -57,7 +57,7 @@ export function extractPathFromFlightRouterState( const childPath = extractPathFromFlightRouterState(value) - if (childPath !== undefined && childPath !== '') { + if (childPath !== undefined) { segments.push(childPath) } } From 09458484255d77a747b4d93e43afa1e905832c88 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Sun, 27 Aug 2023 18:59:28 +0200 Subject: [PATCH 5/6] revert --- .../components/router-reducer/should-hard-navigate.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/next/src/client/components/router-reducer/should-hard-navigate.ts b/packages/next/src/client/components/router-reducer/should-hard-navigate.ts index bebc95d2bcbab..d5b800882bf4b 100644 --- a/packages/next/src/client/components/router-reducer/should-hard-navigate.ts +++ b/packages/next/src/client/components/router-reducer/should-hard-navigate.ts @@ -1,6 +1,7 @@ import type { FlightRouterState, FlightDataPath, + Segment, } from '../../../server/app-render/types' import { matchSegment } from '../match-segments' @@ -10,7 +11,11 @@ export function shouldHardNavigate( flightRouterState: FlightRouterState ): boolean { const [segment, parallelRoutes] = flightRouterState - const [currentSegment, parallelRouteKey] = flightSegmentPath + // TODO-APP: Check if `as` can be replaced. + const [currentSegment, parallelRouteKey] = flightSegmentPath as [ + Segment, + string + ] // Check if current segment matches the existing segment. if (!matchSegment(currentSegment, segment)) { From 4f84ce7d4821ab6790fa72c8a1e60f392a6d754f Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Sun, 27 Aug 2023 19:20:04 +0200 Subject: [PATCH 6/6] fix types --- packages/next/src/build/webpack/loaders/next-app-loader.ts | 2 +- packages/next/src/client/components/match-segments.ts | 5 ----- .../client/components/router-reducer/compute-changed-path.ts | 3 ++- packages/next/src/shared/lib/router/utils/app-paths.ts | 2 +- packages/next/src/shared/lib/segment.ts | 4 ++++ 5 files changed, 8 insertions(+), 8 deletions(-) create mode 100644 packages/next/src/shared/lib/segment.ts diff --git a/packages/next/src/build/webpack/loaders/next-app-loader.ts b/packages/next/src/build/webpack/loaders/next-app-loader.ts index a9dd038022d0d..bc97ab73ec41a 100644 --- a/packages/next/src/build/webpack/loaders/next-app-loader.ts +++ b/packages/next/src/build/webpack/loaders/next-app-loader.ts @@ -21,8 +21,8 @@ import { AppPathnameNormalizer } from '../../../server/future/normalizers/built/ import { AppBundlePathNormalizer } from '../../../server/future/normalizers/built/app/app-bundle-path-normalizer' import { MiddlewareConfig } from '../../analysis/get-page-static-info' import { getFilenameAndExtension } from './next-metadata-route-loader' -import { isGroupSegment } from '../../../client/components/match-segments' import { loadEntrypoint } from '../../load-entrypoint' +import { isGroupSegment } from '../../../shared/lib/segment' export type AppLoaderOptions = { name: string diff --git a/packages/next/src/client/components/match-segments.ts b/packages/next/src/client/components/match-segments.ts index b139cc012ff81..6fa99c98b299e 100644 --- a/packages/next/src/client/components/match-segments.ts +++ b/packages/next/src/client/components/match-segments.ts @@ -33,8 +33,3 @@ export const canSegmentBeOverridden = ( return getSegmentParam(existingSegment)?.param === segment[0] } - -export function isGroupSegment(segment: string) { - // Use array[0] for performant purpose - return segment[0] === '(' && segment.endsWith(')') -} diff --git a/packages/next/src/client/components/router-reducer/compute-changed-path.ts b/packages/next/src/client/components/router-reducer/compute-changed-path.ts index 34b5afb891a08..3a80a5a051b8a 100644 --- a/packages/next/src/client/components/router-reducer/compute-changed-path.ts +++ b/packages/next/src/client/components/router-reducer/compute-changed-path.ts @@ -1,6 +1,7 @@ import { FlightRouterState, Segment } from '../../../server/app-render/types' import { INTERCEPTION_ROUTE_MARKERS } from '../../../server/future/helpers/interception-routes' -import { isGroupSegment, matchSegment } from '../match-segments' +import { isGroupSegment } from '../../../shared/lib/segment' +import { matchSegment } from '../match-segments' const removeLeadingSlash = (segment: string): string => { return segment[0] === '/' ? segment.slice(1) : segment diff --git a/packages/next/src/shared/lib/router/utils/app-paths.ts b/packages/next/src/shared/lib/router/utils/app-paths.ts index d5519635c636f..77612edf6d10a 100644 --- a/packages/next/src/shared/lib/router/utils/app-paths.ts +++ b/packages/next/src/shared/lib/router/utils/app-paths.ts @@ -1,5 +1,5 @@ -import { isGroupSegment } from '../../../../client/components/match-segments' import { ensureLeadingSlash } from '../../page-path/ensure-leading-slash' +import { isGroupSegment } from '../../segment' /** * Normalizes an app route so it represents the actual request path. Essentially diff --git a/packages/next/src/shared/lib/segment.ts b/packages/next/src/shared/lib/segment.ts new file mode 100644 index 0000000000000..6167d681ee6e1 --- /dev/null +++ b/packages/next/src/shared/lib/segment.ts @@ -0,0 +1,4 @@ +export function isGroupSegment(segment: string) { + // Use array[0] for performant purpose + return segment[0] === '(' && segment.endsWith(')') +}