Skip to content

fix: use native path matching for prebundled react conditional #2206

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 11, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions packages/runtime/src/templates/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import {
localizeRoute,
localizeDataRoute,
unlocalizeRoute,
joinPaths,
} from './handlerUtils'

interface NetlifyConfig {
Expand Down Expand Up @@ -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
Expand Down
110 changes: 102 additions & 8 deletions test/templates/server.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,50 @@ jest.mock(
{ virtual: true },
)

jest.mock(
'routes-manifest.json',
() => ({
dynamicRoutes: [
{
page: '/posts/[title]',
regex: '^/posts/([^/]+?)(?:/)?$',
routeKeys: {
nxtPtitle: 'nxtPtitle',
},
namedRegex: '^/posts/(?<nxtPtitle>[^/]+?)(?:/)?$',
},
{
page: '/blog/[author]/[slug]',
regex: '^/blog/([^/]+?)/([^/]+?)(?:/)?$',
routeKeys: {
nxtPauthor: 'nxtPauthor',
nxtPslug: 'nxtPslug',
},
namedRegex: '^/blog/(?<nxtPauthor>[^/]+?)/(?<nxtPslug>[^/]+?)(?:/)?$',
},
],
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 },
)

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
Expand All @@ -76,12 +120,14 @@ beforeAll(() => {
this.buildId = mockBuildId
this.nextConfig = nextOptions.conf
this.netlifyConfig = netlifyConfig
this.renderOpts = { previewProps: {} }
this.appPathsManifest = appPathsManifest
}
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()

Expand All @@ -92,7 +138,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()

Expand All @@ -112,7 +158,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()

Expand All @@ -132,7 +178,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()

Expand All @@ -152,7 +198,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()

Expand All @@ -172,7 +218,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()

Expand All @@ -187,7 +233,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()

Expand All @@ -203,7 +249,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()

Expand All @@ -218,4 +264,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')
})
})