From f942d9e89295b98cb48eee3e1fbf074c936a98f9 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Mon, 7 Sep 2020 06:09:02 -0500 Subject: [PATCH] Include additional query values when interpolating href (#16878) Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> --- packages/next/client/page-loader.ts | 2 +- .../next/next-server/lib/router/router.ts | 39 ++++++++++++++++--- .../build-output/test/index.test.js | 4 +- .../dynamic-routing/pages/[name]/index.js | 7 +++- .../dynamic-routing/pages/index.js | 11 ++++++ .../dynamic-routing/test/index.test.js | 36 ++++++++++++++++- 6 files changed, 89 insertions(+), 10 deletions(-) diff --git a/packages/next/client/page-loader.ts b/packages/next/client/page-loader.ts index 700d22d632d80..96c1d7f1199c5 100644 --- a/packages/next/client/page-loader.ts +++ b/packages/next/client/page-loader.ts @@ -216,7 +216,7 @@ export default class PageLoader { const isDynamic: boolean = isDynamicRoute(route) const interpolatedRoute = isDynamic - ? interpolateAs(hrefPathname, asPathname, query) + ? interpolateAs(hrefPathname, asPathname, query).result : '' return isDynamic diff --git a/packages/next/next-server/lib/router/router.ts b/packages/next/next-server/lib/router/router.ts index 7d6f93d47d27e..f77c2519a800c 100644 --- a/packages/next/next-server/lib/router/router.ts +++ b/packages/next/next-server/lib/router/router.ts @@ -98,8 +98,10 @@ export function interpolateAs( query interpolatedRoute = route + const params = Object.keys(dynamicGroups) + if ( - !Object.keys(dynamicGroups).every((param) => { + !params.every((param) => { let value = dynamicMatches[param] || '' const { repeat, optional } = dynamicGroups[param] @@ -129,7 +131,21 @@ export function interpolateAs( // n.b. We ignore this error because we handle warning for this case in // development in the `` component directly. } - return interpolatedRoute + return { + params, + result: interpolatedRoute, + } +} + +function omitParmsFromQuery(query: ParsedUrlQuery, params: string[]) { + const filteredQuery: ParsedUrlQuery = {} + + Object.keys(query).forEach((key) => { + if (!params.includes(key)) { + filteredQuery[key] = query[key] + } + }) + return filteredQuery } /** @@ -157,11 +173,16 @@ export function resolveHref( ) { const query = searchParamsToUrlQuery(finalUrl.searchParams) - interpolatedAs = interpolateAs( + const { result, params } = interpolateAs( finalUrl.pathname, finalUrl.pathname, query ) + interpolatedAs = formatWithValidation({ + pathname: result, + hash: finalUrl.hash, + query: omitParmsFromQuery(query, params), + }) } // if the origin didn't change, it means we received a relative href @@ -608,7 +629,9 @@ export default class Router implements BaseRouter { resolvedAs = delBasePath(resolvedAs) if (isDynamicRoute(route)) { - const { pathname: asPathname } = parseRelativeUrl(resolvedAs) + const parsedAs = parseRelativeUrl(resolvedAs) + const asPathname = parsedAs.pathname + const routeRegex = getRouteRegex(route) const routeMatch = getRouteMatcher(routeRegex)(asPathname) if (!routeMatch) { @@ -632,7 +655,13 @@ export default class Router implements BaseRouter { ) } } else if (route === asPathname) { - as = interpolateAs(route, asPathname, query) + const { result, params } = interpolateAs(route, asPathname, query) + as = formatWithValidation( + Object.assign({}, parsedAs, { + pathname: result, + query: omitParmsFromQuery(query, params), + }) + ) } else { // Merge params into `query`, overwriting any specified in search Object.assign(query, routeMatch) diff --git a/test/integration/build-output/test/index.test.js b/test/integration/build-output/test/index.test.js index 57314c0f64fb8..1ed5de712a17a 100644 --- a/test/integration/build-output/test/index.test.js +++ b/test/integration/build-output/test/index.test.js @@ -95,7 +95,7 @@ describe('Build Output', () => { expect(indexSize.endsWith('B')).toBe(true) // should be no bigger than 60.2 kb - expect(parseFloat(indexFirstLoad) - 60.2).toBeLessThanOrEqual(0) + expect(parseFloat(indexFirstLoad) - 60.3).toBeLessThanOrEqual(0) expect(indexFirstLoad.endsWith('kB')).toBe(true) expect(parseFloat(err404Size) - 3.5).toBeLessThanOrEqual(0) @@ -104,7 +104,7 @@ describe('Build Output', () => { expect(parseFloat(err404FirstLoad) - 63.4).toBeLessThanOrEqual(0) expect(err404FirstLoad.endsWith('kB')).toBe(true) - expect(parseFloat(sharedByAll) - 59.9).toBeLessThanOrEqual(0) + expect(parseFloat(sharedByAll) - 60).toBeLessThanOrEqual(0) expect(sharedByAll.endsWith('kB')).toBe(true) if (_appSize.endsWith('kB')) { diff --git a/test/integration/dynamic-routing/pages/[name]/index.js b/test/integration/dynamic-routing/pages/[name]/index.js index 6a8ff2760623c..8186e5451ed87 100644 --- a/test/integration/dynamic-routing/pages/[name]/index.js +++ b/test/integration/dynamic-routing/pages/[name]/index.js @@ -3,7 +3,12 @@ import { useRouter } from 'next/router' const Page = () => { const router = useRouter() const { query } = router - return

This is {query.name}

+ return ( + <> +

This is {query.name}

+

{JSON.stringify(query)}

+ + ) } Page.getInitialProps = () => ({}) diff --git a/test/integration/dynamic-routing/pages/index.js b/test/integration/dynamic-routing/pages/index.js index c6bf5d0c59eb6..68a2815320ace 100644 --- a/test/integration/dynamic-routing/pages/index.js +++ b/test/integration/dynamic-routing/pages/index.js @@ -31,6 +31,17 @@ const Page = () => { View post 1 (interpolated)
+ + + View post 1 (interpolated additional query) + + +
View post 1 comments diff --git a/test/integration/dynamic-routing/test/index.test.js b/test/integration/dynamic-routing/test/index.test.js index 5427e53c2403b..449d996119999 100644 --- a/test/integration/dynamic-routing/test/index.test.js +++ b/test/integration/dynamic-routing/test/index.test.js @@ -153,7 +153,9 @@ function runTests(dev) { .elementByCss('#view-post-1-interpolated') .getAttribute('href') - expect(url.parse(href).pathname).toBe('/post-1') + const parsedHref = url.parse(href, true) + expect(parsedHref.pathname).toBe('/post-1') + expect(parsedHref.query).toEqual({}) await browser.elementByCss('#view-post-1-interpolated').click() await browser.waitForElementByCss('#asdf') @@ -167,6 +169,38 @@ function runTests(dev) { } }) + it('should navigate to a dynamic page successfully interpolated with additional query values', async () => { + let browser + try { + browser = await webdriver(appPort, '/') + await browser.eval('window.beforeNav = 1') + + const href = await browser + .elementByCss('#view-post-1-interpolated-more-query') + .getAttribute('href') + + const parsedHref = url.parse(href, true) + expect(parsedHref.pathname).toBe('/post-1') + expect(parsedHref.query).toEqual({ another: 'value' }) + + await browser.elementByCss('#view-post-1-interpolated-more-query').click() + await browser.waitForElementByCss('#asdf') + + expect(await browser.eval('window.beforeNav')).toBe(1) + + const text = await browser.elementByCss('#asdf').text() + expect(text).toMatch(/this is.*?post-1/i) + + const query = JSON.parse(await browser.elementByCss('#query').text()) + expect(query).toEqual({ + name: 'post-1', + another: 'value', + }) + } finally { + if (browser) await browser.close() + } + }) + it('should allow calling Router.push on mount successfully', async () => { const browser = await webdriver(appPort, '/post-1/on-mount-redir') try {