From 937124ba5db0e1716283b70462681008a335c5f8 Mon Sep 17 00:00:00 2001 From: William Kerrigan Date: Thu, 15 Aug 2019 22:18:59 +1200 Subject: [PATCH 1/3] Fix dynamic APIs with query params If you define a dynamic API such as ``` pages/ api/ [id].js ``` This api becomes available at `/api/[id]`. If you send a request with a query parameter the value of `req.query.id` will include the query string as well as the path parameter. E.g. the request to `/api/2?test=123` will result in `req.query` being ```javascript { id: "2?test=123", test: "123", } ``` instead of ```javascript { id: "2", test: "123", } ``` --- .../lib/router/utils/route-regex.ts | 4 +-- .../pages/api/dynamic/[path]/index.js | 3 ++ .../serverless/pages/api/posts/[id].js | 4 +-- .../integration/serverless/test/index.test.js | 24 ++++++++++++-- test/unit/route-regex.test.js | 33 +++++++++++++++++++ 5 files changed, 62 insertions(+), 6 deletions(-) create mode 100644 test/integration/serverless/pages/api/dynamic/[path]/index.js create mode 100644 test/unit/route-regex.test.js diff --git a/packages/next-server/lib/router/utils/route-regex.ts b/packages/next-server/lib/router/utils/route-regex.ts index 2c071d43c18fb..59b2c7086efc0 100644 --- a/packages/next-server/lib/router/utils/route-regex.ts +++ b/packages/next-server/lib/router/utils/route-regex.ts @@ -18,12 +18,12 @@ export function getRouteRegex( // Un-escape key .replace(/\\([|\\{}()[\]^$+*?.-])/g, '$1') ] = groupIndex++), - '/([^/]+?)' + '/([^/?]+?)' ) ) return { - re: new RegExp('^' + parameterizedRoute + '(?:/)?$', 'i'), + re: new RegExp('^' + parameterizedRoute + '(?:/)?(?:\\?.*)?$', 'i'), groups, } } diff --git a/test/integration/serverless/pages/api/dynamic/[path]/index.js b/test/integration/serverless/pages/api/dynamic/[path]/index.js new file mode 100644 index 0000000000000..a85b40be7a075 --- /dev/null +++ b/test/integration/serverless/pages/api/dynamic/[path]/index.js @@ -0,0 +1,3 @@ +export default ({ query }, res) => { + res.json(query) +} diff --git a/test/integration/serverless/pages/api/posts/[id].js b/test/integration/serverless/pages/api/posts/[id].js index 490c95a79f881..a85b40be7a075 100644 --- a/test/integration/serverless/pages/api/posts/[id].js +++ b/test/integration/serverless/pages/api/posts/[id].js @@ -1,3 +1,3 @@ -export default (req, res) => { - res.json({ post: req.query.id }) +export default ({ query }, res) => { + res.json(query) } diff --git a/test/integration/serverless/test/index.test.js b/test/integration/serverless/test/index.test.js index a07043f189177..3f6643e10813d 100644 --- a/test/integration/serverless/test/index.test.js +++ b/test/integration/serverless/test/index.test.js @@ -121,8 +121,28 @@ describe('Serverless', () => { it('should reply on dynamic API request successfully', async () => { const result = await renderViaHTTP(appPort, '/api/posts/post-1') - const { post } = JSON.parse(result) - expect(post).toBe('post-1') + const { id } = JSON.parse(result) + expect(id).toBe('post-1') + }) + + it('should reply on dynamic API request successfully with query parameters', async () => { + const result = await renderViaHTTP(appPort, '/api/posts/post-1?param=val') + const { id, param } = JSON.parse(result) + expect(id).toBe('post-1') + expect(param).toBe('val') + }) + + it('should reply on dynamic API index request successfully', async () => { + const result = await renderViaHTTP(appPort, '/api/dynamic/post-1') + const { path } = JSON.parse(result) + expect(path).toBe('post-1') + }) + + it('should reply on dynamic API index request successfully with query parameters', async () => { + const result = await renderViaHTTP(appPort, '/api/dynamic/post-1?param=val') + const { path, param } = JSON.parse(result) + expect(path).toBe('post-1') + expect(param).toBe('val') }) it('should 404 on API request with trailing slash', async () => { diff --git a/test/unit/route-regex.test.js b/test/unit/route-regex.test.js new file mode 100644 index 0000000000000..e907dea450c14 --- /dev/null +++ b/test/unit/route-regex.test.js @@ -0,0 +1,33 @@ +/* eslint-env jest */ +import { getRouteRegex } from 'next-server/dist/lib/router/utils/route-regex' + +describe('getRouteRegex', () => { + it('should render static route with param', async () => { + const { re, groups } = getRouteRegex('/api/test') + const matches = '/api/test?param=val'.match(re) + + expect(matches).toHaveLength(1) + expect(matches[0]).toEqual('/api/test?param=val') + expect(groups).toEqual({}) + }) + + it('should render dynamic route', async () => { + const { re, groups } = getRouteRegex('/api/[test]') + const matches = '/api/abc'.match(re) + + expect(matches).toHaveLength(2) + expect(matches[0]).toEqual('/api/abc') + expect(matches[1]).toEqual('abc') + expect(groups).toEqual({ test: 1 }) + }) + + it('should render dynamic route with param', async () => { + const { re, groups } = getRouteRegex('/api/[test]') + const matches = '/api/abc?param=val'.match(re) + + expect(matches).toHaveLength(2) + expect(matches[0]).toEqual('/api/abc?param=val') + expect(matches[1]).toEqual('abc') + expect(groups).toEqual({ test: 1 }) + }) +}) From d9cc30fcd991436c2cd5d7ba4c73738726bff23c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Huvar?= Date: Mon, 19 Aug 2019 18:14:51 +0200 Subject: [PATCH 2/3] Fix url parse in serverless loader --- .../lib/router/utils/route-regex.ts | 4 +-- .../webpack/loaders/next-serverless-loader.ts | 3 +- .../api-support/test/index.test.js | 17 ++++++++-- .../production/pages/api/[post]/index.js | 3 ++ .../integration/production/test/index.test.js | 8 +++++ test/unit/route-regex.test.js | 33 ------------------- 6 files changed, 29 insertions(+), 39 deletions(-) create mode 100644 test/integration/production/pages/api/[post]/index.js delete mode 100644 test/unit/route-regex.test.js diff --git a/packages/next-server/lib/router/utils/route-regex.ts b/packages/next-server/lib/router/utils/route-regex.ts index 59b2c7086efc0..2c071d43c18fb 100644 --- a/packages/next-server/lib/router/utils/route-regex.ts +++ b/packages/next-server/lib/router/utils/route-regex.ts @@ -18,12 +18,12 @@ export function getRouteRegex( // Un-escape key .replace(/\\([|\\{}()[\]^$+*?.-])/g, '$1') ] = groupIndex++), - '/([^/?]+?)' + '/([^/]+?)' ) ) return { - re: new RegExp('^' + parameterizedRoute + '(?:/)?(?:\\?.*)?$', 'i'), + re: new RegExp('^' + parameterizedRoute + '(?:/)?$', 'i'), groups, } } diff --git a/packages/next/build/webpack/loaders/next-serverless-loader.ts b/packages/next/build/webpack/loaders/next-serverless-loader.ts index 2e3f377d00e66..7c2a7fd5848be 100644 --- a/packages/next/build/webpack/loaders/next-serverless-loader.ts +++ b/packages/next/build/webpack/loaders/next-serverless-loader.ts @@ -50,12 +50,13 @@ const nextServerlessLoader: loader.Loader = function() { ` : `` } + import { parse } from 'url' import { apiResolver } from 'next-server/dist/server/api-utils' export default (req, res) => { const params = ${ isDynamicRoute(page) - ? `getRouteMatcher(getRouteRegex('${page}'))(req.url)` + ? `getRouteMatcher(getRouteRegex('${page}'))(parse(req.url).pathname)` : `{}` } const resolver = require('${absolutePagePath}') diff --git a/test/integration/api-support/test/index.test.js b/test/integration/api-support/test/index.test.js index 2ae6cad48f63b..7d40486508f01 100644 --- a/test/integration/api-support/test/index.test.js +++ b/test/integration/api-support/test/index.test.js @@ -220,6 +220,17 @@ function runTests (serverless = false) { expect(data).toEqual({ post: 'post-1' }) }) + it('should work with dynamic params and search string', async () => { + const data = await fetchViaHTTP( + appPort, + '/api/post-1?val=1', + null, + {} + ).then(res => res.ok && res.json()) + + expect(data).toEqual({ val: '1', post: 'post-1' }) + }) + it('should prioritize a non-dynamic page', async () => { const data = await fetchViaHTTP( appPort, @@ -262,15 +273,15 @@ function runTests (serverless = false) { const port = await findPort() const resolver = require(join( appDir, - '.next/serverless/pages/api/blog.js' + '.next/serverless/pages/api/[post].js' )).default const server = createServer(resolver).listen(port) - const res = await fetchViaHTTP(port, '/api/nextjs') + const res = await fetchViaHTTP(port, '/api/post-1?val=1') const json = await res.json() server.close() - expect(json).toEqual([{ title: 'Cool Post!' }]) + expect(json).toEqual({ val: '1', post: 'post-1' }) } else { expect( existsSync(join(appDir, '.next/server/pages-manifest.json'), 'utf8') diff --git a/test/integration/production/pages/api/[post]/index.js b/test/integration/production/pages/api/[post]/index.js new file mode 100644 index 0000000000000..70b2ec0fbc121 --- /dev/null +++ b/test/integration/production/pages/api/[post]/index.js @@ -0,0 +1,3 @@ +export default ({ query }, res) => { + res.status(200).json(query) +} diff --git a/test/integration/production/test/index.test.js b/test/integration/production/test/index.test.js index 85a5211acd07c..9cbbb50e1bd16 100644 --- a/test/integration/production/test/index.test.js +++ b/test/integration/production/test/index.test.js @@ -233,6 +233,14 @@ describe('Production Usage', () => { const body = await res.text() expect(body).toEqual('API hello works') }) + + it('should work with dynamic params and search string', async () => { + const url = `http://localhost:${appPort}/api/post-1?val=1` + const res = await fetch(url) + const body = await res.json() + + expect(body).toEqual({ val: '1', post: 'post-1' }) + }) }) describe('With navigation', () => { diff --git a/test/unit/route-regex.test.js b/test/unit/route-regex.test.js deleted file mode 100644 index e907dea450c14..0000000000000 --- a/test/unit/route-regex.test.js +++ /dev/null @@ -1,33 +0,0 @@ -/* eslint-env jest */ -import { getRouteRegex } from 'next-server/dist/lib/router/utils/route-regex' - -describe('getRouteRegex', () => { - it('should render static route with param', async () => { - const { re, groups } = getRouteRegex('/api/test') - const matches = '/api/test?param=val'.match(re) - - expect(matches).toHaveLength(1) - expect(matches[0]).toEqual('/api/test?param=val') - expect(groups).toEqual({}) - }) - - it('should render dynamic route', async () => { - const { re, groups } = getRouteRegex('/api/[test]') - const matches = '/api/abc'.match(re) - - expect(matches).toHaveLength(2) - expect(matches[0]).toEqual('/api/abc') - expect(matches[1]).toEqual('abc') - expect(groups).toEqual({ test: 1 }) - }) - - it('should render dynamic route with param', async () => { - const { re, groups } = getRouteRegex('/api/[test]') - const matches = '/api/abc?param=val'.match(re) - - expect(matches).toHaveLength(2) - expect(matches[0]).toEqual('/api/abc?param=val') - expect(matches[1]).toEqual('abc') - expect(groups).toEqual({ test: 1 }) - }) -}) From c81192548eba5b9b2b894078a720ab24ef7731a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Huvar?= Date: Mon, 19 Aug 2019 19:03:37 +0200 Subject: [PATCH 3/3] Add serverless test --- .../api-support/test/index.test.js | 25 ++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/test/integration/api-support/test/index.test.js b/test/integration/api-support/test/index.test.js index 7d40486508f01..2969c4a9c275c 100644 --- a/test/integration/api-support/test/index.test.js +++ b/test/integration/api-support/test/index.test.js @@ -231,6 +231,25 @@ function runTests (serverless = false) { expect(data).toEqual({ val: '1', post: 'post-1' }) }) + if (serverless) { + it('should work with dynamic params and search string like lambda', async () => { + await nextBuild(appDir, []) + + const port = await findPort() + const resolver = require(join( + appDir, + '.next/serverless/pages/api/[post].js' + )).default + + const server = createServer(resolver).listen(port) + const res = await fetchViaHTTP(port, '/api/post-1?val=1') + const json = await res.json() + server.close() + + expect(json).toEqual({ val: '1', post: 'post-1' }) + }) + } + it('should prioritize a non-dynamic page', async () => { const data = await fetchViaHTTP( appPort, @@ -273,15 +292,15 @@ function runTests (serverless = false) { const port = await findPort() const resolver = require(join( appDir, - '.next/serverless/pages/api/[post].js' + '.next/serverless/pages/api/blog.js' )).default const server = createServer(resolver).listen(port) - const res = await fetchViaHTTP(port, '/api/post-1?val=1') + const res = await fetchViaHTTP(port, '/api/nextjs') const json = await res.json() server.close() - expect(json).toEqual({ val: '1', post: 'post-1' }) + expect(json).toEqual([{ title: 'Cool Post!' }]) } else { expect( existsSync(join(appDir, '.next/server/pages-manifest.json'), 'utf8')