From 9b415b55c826b209d52f3b6e2c2ecb4505126b5c Mon Sep 17 00:00:00 2001 From: Rob Stanford Date: Mon, 10 Jul 2023 09:45:52 +0100 Subject: [PATCH 1/3] fix: use native path matching for app routes --- packages/runtime/src/templates/server.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/runtime/src/templates/server.ts b/packages/runtime/src/templates/server.ts index 3372106d1d..2adfe366bc 100644 --- a/packages/runtime/src/templates/server.ts +++ b/packages/runtime/src/templates/server.ts @@ -12,7 +12,6 @@ import { localizeRoute, localizeDataRoute, unlocalizeRoute, - joinPaths, } from './handlerUtils' interface NetlifyConfig { @@ -78,12 +77,11 @@ const getNetlifyNextServer = (NextServer: NextServerType) => { // doing what they do in https://github.com/vercel/vercel/blob/1663db7ca34d3dd99b57994f801fb30b72fbd2f3/packages/next/src/server-build.ts#L576-L580 private netlifyPrebundleReact(path: string) { const routesManifest = this.getRoutesManifest?.() - const appPathsManifest = this.getAppPathsManifest?.() + const appPathsRoutes = this.getAppPathRoutes?.() const routes = routesManifest && [...routesManifest.staticRoutes, ...routesManifest.dynamicRoutes] - const matchedRoute = routes?.find((route) => new RegExp(route.regex).test(path.split('?')[0])) - const isAppRoute = - appPathsManifest && matchedRoute ? appPathsManifest[joinPaths(matchedRoute.page, 'page')] : false + const matchedRoute = routes?.find((route) => new RegExp(route.regex).test(new URL(path, 'http://n').pathname)) + const isAppRoute = appPathsRoutes && matchedRoute ? appPathsRoutes[matchedRoute.page] : false if (isAppRoute) { // app routes should use prebundled React From 7073377986940df5357866661f11acd45d94bf54 Mon Sep 17 00:00:00 2001 From: Rob Stanford Date: Mon, 10 Jul 2023 09:46:27 +0100 Subject: [PATCH 2/3] chore: re-enable custom server tests and add react tests --- test/templates/server.spec.ts | 115 +++++++++++++++++++++++++++++++--- 1 file changed, 107 insertions(+), 8 deletions(-) diff --git a/test/templates/server.spec.ts b/test/templates/server.spec.ts index 8296a6f01d..994adce304 100644 --- a/test/templates/server.spec.ts +++ b/test/templates/server.spec.ts @@ -65,6 +65,54 @@ jest.mock( { virtual: true }, ) +jest.mock( + 'server/app-paths-manifest.json', + () => ({ + '/blog/(test)/[author]/[slug]/page': 'app/blog/[author]/[slug]/page.js', + }), + { virtual: true }, +) + +jest.mock( + 'routes-manifest.json', + () => ({ + dynamicRoutes: [ + { + page: '/posts/[title]', + regex: '^/posts/([^/]+?)(?:/)?$', + routeKeys: { + nxtPtitle: 'nxtPtitle', + }, + namedRegex: '^/posts/(?[^/]+?)(?:/)?$', + }, + { + page: '/blog/[author]/[slug]', + regex: '^/blog/([^/]+?)/([^/]+?)(?:/)?$', + routeKeys: { + nxtPauthor: 'nxtPauthor', + nxtPslug: 'nxtPslug', + }, + namedRegex: '^/blog/(?[^/]+?)/(?[^/]+?)(?:/)?$', + }, + ], + staticRoutes: [ + { + page: '/non-i18n/with-revalidate', + regex: '^/non-i18n/with-revalidate(?:/)?$', + routeKeys: {}, + namedRegex: '^/non-i18n/with-revalidate(?:/)?$', + }, + { + page: '/i18n/with-revalidate', + regex: '^/i18n/with-revalidate(?:/)?$', + routeKeys: {}, + namedRegex: '^/i18n/with-revalidate(?:/)?$', + }, + ], + }), + { virtual: true }, +) + let NetlifyNextServer: NetlifyNextServerType beforeAll(() => { const NextServer: NextServerType = require(getServerFile(__dirname, false)).default @@ -76,12 +124,15 @@ beforeAll(() => { this.buildId = mockBuildId this.nextConfig = nextOptions.conf this.netlifyConfig = netlifyConfig + this.renderOpts = { previewProps: {} } + this.hasAppDir = Boolean(this.nextConfig?.experimental?.appDir) + this.appPathsManifest = this.getAppPathsManifest() } Object.setPrototypeOf(NetlifyNextServer, MockNetlifyNextServerConstructor) }) describe('the netlify next server', () => { - it.skip('does not revalidate a request without an `x-prerender-revalidate` header', async () => { + it('does not revalidate a request without an `x-prerender-revalidate` header', async () => { const netlifyNextServer = new NetlifyNextServer({ conf: {} }, { ...mockTokenConfig }) const requestHandler = netlifyNextServer.getRequestHandler() @@ -92,7 +143,7 @@ describe('the netlify next server', () => { expect(mockedApiFetch).not.toHaveBeenCalled() }) - it.skip('revalidates a static non-i18n route with an `x-prerender-revalidate` header', async () => { + it('revalidates a static non-i18n route with an `x-prerender-revalidate` header', async () => { const netlifyNextServer = new NetlifyNextServer({ conf: {} }, { ...mockTokenConfig }) const requestHandler = netlifyNextServer.getRequestHandler() @@ -112,7 +163,7 @@ describe('the netlify next server', () => { ) }) - it.skip('revalidates a static i18n route with an `x-prerender-revalidate` header', async () => { + it('revalidates a static i18n route with an `x-prerender-revalidate` header', async () => { const netlifyNextServer = new NetlifyNextServer({ conf: { ...mocki18nConfig } }, { ...mockTokenConfig }) const requestHandler = netlifyNextServer.getRequestHandler() @@ -132,7 +183,7 @@ describe('the netlify next server', () => { ) }) - it.skip('revalidates a dynamic non-i18n route with an `x-prerender-revalidate` header', async () => { + it('revalidates a dynamic non-i18n route with an `x-prerender-revalidate` header', async () => { const netlifyNextServer = new NetlifyNextServer({ conf: {} }, { ...mockTokenConfig }) const requestHandler = netlifyNextServer.getRequestHandler() @@ -152,7 +203,7 @@ describe('the netlify next server', () => { ) }) - it.skip('revalidates a dynamic i18n route with an `x-prerender-revalidate` header', async () => { + it('revalidates a dynamic i18n route with an `x-prerender-revalidate` header', async () => { const netlifyNextServer = new NetlifyNextServer({ conf: { ...mocki18nConfig } }, { ...mockTokenConfig }) const requestHandler = netlifyNextServer.getRequestHandler() @@ -172,7 +223,7 @@ describe('the netlify next server', () => { ) }) - it.skip('throws an error when route is not found in the manifest', async () => { + it('throws an error when route is not found in the manifest', async () => { const netlifyNextServer = new NetlifyNextServer({ conf: {} }, mockTokenConfig) const requestHandler = netlifyNextServer.getRequestHandler() @@ -187,7 +238,7 @@ describe('the netlify next server', () => { ) }) - it.skip('throws an error when paths are not found by the API', async () => { + it('throws an error when paths are not found by the API', async () => { const netlifyNextServer = new NetlifyNextServer({ conf: {} }, mockTokenConfig) const requestHandler = netlifyNextServer.getRequestHandler() @@ -203,7 +254,7 @@ describe('the netlify next server', () => { ) }) - it.skip('throws an error when the revalidate API is unreachable', async () => { + it('throws an error when the revalidate API is unreachable', async () => { const netlifyNextServer = new NetlifyNextServer({ conf: {} }, mockTokenConfig) const requestHandler = netlifyNextServer.getRequestHandler() @@ -218,4 +269,52 @@ describe('the netlify next server', () => { 'Unable to connect', ) }) + + it('resolves react as normal for pages routes', async () => { + const netlifyNextServer = new NetlifyNextServer({ conf: {} }, {}) + const requestHandler = netlifyNextServer.getRequestHandler() + + const { req: mockReq, res: mockRes } = createRequestResponseMocks({ + url: '/posts/hello', + }) + + // @ts-expect-error - Types are incorrect for `MockedResponse` + await requestHandler(new NodeNextRequest(mockReq), new NodeNextResponse(mockRes)) + + // eslint-disable-next-line no-underscore-dangle + expect(process.env.__NEXT_PRIVATE_PREBUNDLED_REACT).toBe('') + }) + + it('resolves the prebundled react version for app routes', async () => { + const netlifyNextServer = new NetlifyNextServer({ conf: { experimental: { appDir: true } } }, {}) + const requestHandler = netlifyNextServer.getRequestHandler() + + const { req: mockReq, res: mockRes } = createRequestResponseMocks({ + url: '/blog/rob/hello', + }) + + // @ts-expect-error - Types are incorrect for `MockedResponse` + await requestHandler(new NodeNextRequest(mockReq), new NodeNextResponse(mockRes)) + + // eslint-disable-next-line no-underscore-dangle + expect(process.env.__NEXT_PRIVATE_PREBUNDLED_REACT).toBe('next') + }) + + it('resolves the experimental prebundled react version for app routes with server actions', async () => { + const netlifyNextServer = new NetlifyNextServer( + { conf: { experimental: { appDir: true, serverActions: true } } }, + {}, + ) + const requestHandler = netlifyNextServer.getRequestHandler() + + const { req: mockReq, res: mockRes } = createRequestResponseMocks({ + url: '/blog/rob/hello', + }) + + // @ts-expect-error - Types are incorrect for `MockedResponse` + await requestHandler(new NodeNextRequest(mockReq), new NodeNextResponse(mockRes)) + + // eslint-disable-next-line no-underscore-dangle + expect(process.env.__NEXT_PRIVATE_PREBUNDLED_REACT).toBe('experimental') + }) }) From a51534ffcf9c6147166c631835f6b65f92bd7ee0 Mon Sep 17 00:00:00 2001 From: Rob Stanford Date: Mon, 10 Jul 2023 10:22:44 +0100 Subject: [PATCH 3/3] chore: update tests to work on windows --- test/templates/server.spec.ts | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/test/templates/server.spec.ts b/test/templates/server.spec.ts index 994adce304..fa1d3c5868 100644 --- a/test/templates/server.spec.ts +++ b/test/templates/server.spec.ts @@ -65,14 +65,6 @@ jest.mock( { virtual: true }, ) -jest.mock( - 'server/app-paths-manifest.json', - () => ({ - '/blog/(test)/[author]/[slug]/page': 'app/blog/[author]/[slug]/page.js', - }), - { virtual: true }, -) - jest.mock( 'routes-manifest.json', () => ({ @@ -113,6 +105,10 @@ jest.mock( { virtual: true }, ) +const appPathsManifest = { + '/blog/(test)/[author]/[slug]/page': 'app/blog/[author]/[slug]/page.js', +} + let NetlifyNextServer: NetlifyNextServerType beforeAll(() => { const NextServer: NextServerType = require(getServerFile(__dirname, false)).default @@ -125,8 +121,7 @@ beforeAll(() => { this.nextConfig = nextOptions.conf this.netlifyConfig = netlifyConfig this.renderOpts = { previewProps: {} } - this.hasAppDir = Boolean(this.nextConfig?.experimental?.appDir) - this.appPathsManifest = this.getAppPathsManifest() + this.appPathsManifest = appPathsManifest } Object.setPrototypeOf(NetlifyNextServer, MockNetlifyNextServerConstructor) })