Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[next] Fix _next/data resolving priority for dynamic routes #8278

Merged
merged 12 commits into from Aug 2, 2022
48 changes: 47 additions & 1 deletion packages/next/src/server-build.ts
Expand Up @@ -58,6 +58,7 @@ const CORRECT_MIDDLEWARE_ORDER_VERSION = 'v12.1.7-canary.29';
const NEXT_DATA_MIDDLEWARE_RESOLVING_VERSION = 'v12.1.7-canary.33';
const EMPTY_ALLOW_QUERY_FOR_PRERENDERED_VERSION = 'v12.2.0';
const CORRECTED_MANIFESTS_VERSION = 'v12.2.0';
const NON_NESTED_MIDDLEWARE_VERSION = 'v12.1.7-canary.9';

export async function serverBuild({
dynamicPages,
Expand Down Expand Up @@ -151,6 +152,10 @@ export async function serverBuild({
nextVersion,
CORRECTED_MANIFESTS_VERSION
);
const isNonNestedMiddleware = semver.gte(
nextVersion,
NON_NESTED_MIDDLEWARE_VERSION
);
let hasStatic500 = !!staticPages[path.join(entryDirectory, '500')];

if (lambdaPageKeys.length === 0) {
Expand Down Expand Up @@ -1054,6 +1059,23 @@ export async function serverBuild({
});
}

// We stopped duplicating matchers for _next/data routes when we added
// x-nextjs-data header resolving but we should still resolve middleware
// when the header isn't present so we augment the source to include that.
// We don't apply this modification for nested middleware > 1 staticRoute
if (isNonNestedMiddleware) {
middleware.staticRoutes.forEach(route => {
if (!route.src?.match(/_next[\\/]{1,}data/)) {
route.src =
`^(\\/_next\\/data\\/${escapedBuildId})?(` +
route.src
?.replace(/\|\^/g, '|')
.replace(/\$$/, ')$')
.replace(/\$/g, '(\\.json)?$');
}
});
}

return {
wildcard: wildcardConfig,
images:
Expand Down Expand Up @@ -1363,7 +1385,30 @@ export async function serverBuild({
...denormalizeNextDataRoute(),

// /_next/data routes for getServerProps/getStaticProps pages
...dataRoutes,
...(isNextDataServerResolving
? // when resolving data routes for middleware we need to include
// all dynamic routes including non-SSG/SSP so that the priority
// is correct
dynamicRoutes
.map(route => {
route = Object.assign({}, route);
route.src = path.posix.join(
'^/',
entryDirectory,
'_next/data/',
escapedBuildId,
route.src.replace(/(^\^|\$$)/g, '') + '.json$'
);

const { pathname } = new URL(route.dest || '/', 'http://n');
ijjk marked this conversation as resolved.
Show resolved Hide resolved

if (prerenders[path.join('./', pathname)]) {
route.dest = `/_next/data/${buildId}${pathname}.json`;
}
return route;
})
.filter(Boolean)
: dataRoutes),

...(!isNextDataServerResolving
? [
Expand Down Expand Up @@ -1395,6 +1440,7 @@ export async function serverBuild({
'x-nextjs-matched-path': '/$1',
},
continue: true,
override: true,
ijjk marked this conversation as resolved.
Show resolved Hide resolved
},
// add a catch-all data route so we don't 404 when getting
// middleware effects
Expand Down
12 changes: 11 additions & 1 deletion packages/next/test/fixtures/00-middleware/middleware.js
Expand Up @@ -2,6 +2,16 @@ import { NextResponse } from 'next/server';

const ALLOWED = ['allowed'];

export const config = {
matcher: [
'/dynamic/:path*',
'/_sites/:path*',
'/:teamId/:slug',
'/:path*',
'/',
],
};

export function middleware(request) {
const url = request.nextUrl;
const pathname = url.pathname;
Expand All @@ -12,7 +22,7 @@ export function middleware(request) {

if (url.pathname === '/redirect-me') {
url.pathname = '/from-middleware';
return NextResponse.redirect(url);
return NextResponse.redirect(url, 307);
}

if (url.pathname === '/next') {
Expand Down
@@ -0,0 +1,3 @@
export default function Page() {
return <p>/[teamId]/[slug]</p>;
}
@@ -0,0 +1,3 @@
export default function Index() {
return <p className="title">static route</p>;
}
58 changes: 56 additions & 2 deletions packages/next/test/fixtures/00-middleware/vercel.json
@@ -1,7 +1,42 @@
{
"version": 2,
"builds": [{ "src": "package.json", "use": "@vercel/next" }],
"builds": [
{
"src": "package.json",
"use": "@vercel/next"
}
],
styfle marked this conversation as resolved.
Show resolved Hide resolved
"probes": [
{
"path": "/_next/data/testing-build-id/dynamic/static.json",
"status": 200,
"headers": {
"x-nextjs-data": 1
},
"responseHeaders": {
"x-matched-path": "/dynamic/static"
}
},
{
"path": "/_next/data/testing-build-id/team-1/slug-1.json",
"status": 200,
"headers": {
"x-nextjs-data": 1
},
"responseHeaders": {
"x-matched-path": "/[teamId]/[slug]"
}
},
{
"path": "/_next/data/testing-build-id/dynamic/id-1.json",
"status": 200,
"headers": {
"x-nextjs-data": 1
},
"responseHeaders": {
"x-matched-path": "/dynamic/[id]"
}
},
{
"path": "/_next/data/testing-build-id/rewrite-to-site.json",
"status": 200,
Expand Down Expand Up @@ -58,9 +93,28 @@
"x-nextjs-data": "1"
},
"responseHeaders": {
"x-nextjs-matched-path": "/dynamic/first"
"x-matched-path": "/dynamic/[id]"
},
"mustContain": "{}"
},
{
"path": "/_next/data/testing-build-id/redirect-me.json",
"status": 307,
"fetchOptions": {
"redirect": "manual"
},
"responseHeaders": {
"Location": "/from-middleware/"
}
},
{
"path": "/_next/data/testing-build-id/_sites/subdomain-1.json",
"status": 200,
"fetchOptions": {
"redirect": "manual"
},
"mustNotContain": "<html>",
"mustContain": "\"site\":\"subdomain-1\""
}
]
}
3 changes: 3 additions & 0 deletions packages/next/test/integration/middleware.test.ts
Expand Up @@ -47,6 +47,9 @@ describe('Middleware simple project', () => {
expect(redirectIndex).toBeLessThan(beforeFilesIndex);
expect(middlewareIndex).toBeLessThan(beforeFilesIndex);
expect(middlewareIndex).toBeLessThan(handleFileSystemIndex);
expect(ctx.buildResult.routes[middlewareIndex].src).toMatch(
/_next[\\/]{1,}data/
);
});

it('generates deterministic code', async () => {
Expand Down