Skip to content

Commit f2a7cac

Browse files
ascorbicascorbic
authored
fix: correctly handle ISR for appDir pages (#1855)
* fix: don't use ISR for appDir pages * chore: correct redirects * test: add e2e test * chore: fix test * ci: don't run tests against 14, because appDir requires 16 * chore: snapidoo 🎉 * ci: node 16 * ci: use node 16 * ci: normalise chunk names * ci: typo in test * chore: use repo next version * chore: fix cypress test * test: temporarily remove esm for app alias * chore: disable test failures * chore: move disabled test * chore: use EF to route RSC data reqs * chore: fix test * chore: fix test * chore: fix cypress test * chore: logging * fix: handle trailing slash * chore: wip * chore: handle dynamic isr routes * test: add more cypress tests * fix: handle localised RSC routes * chore: simplify trailing slash handling * fix: correctly handle prerendered dynamic RSC routes * test: fix cypress assertion * fix: correct regex matcher * test: fix cypress * test: stop cypress following redirects
1 parent d82af8f commit f2a7cac

36 files changed

+1090
-144
lines changed

‎.github/workflows/e2e-appdir.yml

-1
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ jobs:
4141
env:
4242
NETLIFY_AUTH_TOKEN: ${{ secrets.NETLIFY_BOT_AUTH_TOKEN }}
4343
NETLIFY_SITE_ID: 1d5a5c76-d445-4ae5-b694-b0d3f2e2c395
44-
NEXT_TEST_VERSION: canary
4544
- uses: actions/upload-artifact@v3
4645
if: ${{ always() }}
4746
name: Upload test results

‎.github/workflows/test.yml

+2-27
Original file line numberDiff line numberDiff line change
@@ -15,32 +15,19 @@ jobs:
1515
strategy:
1616
matrix:
1717
os: [ubuntu-latest, macOS-latest, windows-latest]
18-
node-version: [14, '*']
19-
exclude:
20-
- os: macOS-latest
21-
node-version: 14
22-
- os: windows-latest
23-
node-version: 14
2418
fail-fast: false
2519

2620
steps:
2721
- uses: actions/checkout@v2
2822
- name: Installing with LTS Node.js
2923
uses: actions/setup-node@v2
3024
with:
31-
node-version: 'lts/*'
25+
node-version: 16
3226
check-latest: true
3327
- name: NPM Install
3428
run: npm install
35-
- name: Switching to Node.js ${{ matrix.node-version }} to run tests
36-
uses: actions/setup-node@v2
37-
if: "${{ matrix.node-version != 'lts/*' }}"
38-
with:
39-
node-version: ${{ matrix.node-version }}
40-
check-latest: true
4129
- name: Linting
4230
run: npm run format:ci
43-
if: "${{ matrix.node-version == 'lts/*' }}"
4431
- name: Run tests against next@latest
4532
run: npm test
4633
canary:
@@ -50,12 +37,6 @@ jobs:
5037
strategy:
5138
matrix:
5239
os: [ubuntu-latest, macOS-latest, windows-latest]
53-
node-version: [14, '*']
54-
exclude:
55-
- os: macOS-latest
56-
node-version: 14
57-
- os: windows-latest
58-
node-version: 14
5940
fail-fast: false
6041

6142
if: github.ref_name == 'main'
@@ -64,17 +45,11 @@ jobs:
6445
- name: Installing with LTS Node.js
6546
uses: actions/setup-node@v2
6647
with:
67-
node-version: 'lts/*'
48+
node-version: 16
6849
check-latest: true
6950
- name: NPM Install
7051
run: npm install
7152
- name: Install Next.js Canary
7253
run: npm install -D next@canary --legacy-peer-deps
73-
- name: Switching to Node.js ${{ matrix.node-version }} to run tests
74-
uses: actions/setup-node@v2
75-
if: "${{ matrix.node-version != 'lts/*' }}"
76-
with:
77-
node-version: ${{ matrix.node-version }}
78-
check-latest: true
7954
- name: Run tests against next@canary
8055
run: npm test

‎cypress/integration/default/appdir.spec.ts

+77
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
describe('appDir', () => {
2+
it('renders ISR appdir pages as HTML by default', () => {
3+
cy.request({ url: '/blog/erica/', followRedirect: false }).then((response) => {
4+
expect(response.headers['content-type']).to.match(/^text\/html/)
5+
})
6+
})
7+
8+
it('renders static appdir pages as HTML by default', () => {
9+
cy.request({ url: '/blog/erica/first-post/', followRedirect: false }).then((response) => {
10+
expect(response.headers['content-type']).to.match(/^text\/html/)
11+
})
12+
})
13+
14+
it('renders dynamic appdir pages as HTML by default', () => {
15+
cy.request({ url: '/blog/erica/random-post/', followRedirect: false }).then((response) => {
16+
expect(response.headers['content-type']).to.match(/^text\/html/)
17+
})
18+
})
19+
20+
it('returns RSC data for RSC requests to ISR pages', () => {
21+
cy.request({
22+
url: '/blog/erica/',
23+
headers: {
24+
RSC: '1',
25+
},
26+
followRedirect: false,
27+
}).then((response) => {
28+
expect(response.headers).to.have.property('content-type', 'application/octet-stream')
29+
})
30+
})
31+
32+
it('returns RSC data for RSC requests to static pages', () => {
33+
cy.request({
34+
url: '/blog/erica/first-post/',
35+
headers: {
36+
RSC: '1',
37+
},
38+
followRedirect: false,
39+
}).then((response) => {
40+
expect(response.headers).to.have.property('content-type', 'application/octet-stream')
41+
})
42+
})
43+
44+
it('returns RSC data for RSC requests to dynamic pages', () => {
45+
cy.request({
46+
url: '/blog/erica/random-post/',
47+
headers: {
48+
RSC: '1',
49+
},
50+
followRedirect: false,
51+
}).then((response) => {
52+
expect(response.headers).to.have.property('content-type', 'application/octet-stream')
53+
})
54+
})
55+
56+
it('correctly redirects HTML requests for ISR pages', () => {
57+
cy.request({ url: '/blog/erica', followRedirect: false }).then((response) => {
58+
expect(response.status).to.equal(308)
59+
expect(response.headers).to.have.property('location', '/blog/erica/')
60+
})
61+
})
62+
63+
// This needs trailing slash handling to be fixed
64+
it.skip('correctly redirects HTML requests for static pages', () => {
65+
cy.request({ url: '/blog/erica/first-post', followRedirect: false }).then((response) => {
66+
expect(response.status).to.equal(308)
67+
expect(response.headers).to.have.property('location', '/blog/erica/first-post/')
68+
})
69+
})
70+
71+
it('correctly redirects HTML requests for dynamic pages', () => {
72+
cy.request({ url: '/blog/erica/random-post', followRedirect: false }).then((response) => {
73+
expect(response.status).to.equal(308)
74+
expect(response.headers).to.have.property('location', '/blog/erica/random-post/')
75+
})
76+
})
77+
})

‎demos/default/.vscode/settings.json

+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"typescript.tsdk": "../../node_modules/typescript/lib",
3+
"typescript.enablePromptUseWorkspaceTsdk": true
4+
}

‎demos/default/app/blog/[author]/[slug]/page.tsx

+60
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
import { notFound } from 'next/navigation'
2+
3+
export const revalidate = null
4+
5+
export const dynamicParams = true
6+
7+
export default function Page({ params }) {
8+
if (params.author === 'matt') {
9+
return notFound()
10+
}
11+
return (
12+
<>
13+
<p id="page">/blog/[author]/[slug]</p>
14+
<p id="params">{JSON.stringify(params)}</p>
15+
<p id="date">{Date.now()}</p>
16+
</>
17+
)
18+
}
19+
20+
export function generateStaticParams({ params }: any) {
21+
console.log('/blog/[author]/[slug] generateStaticParams', JSON.stringify(params))
22+
23+
switch (params.author) {
24+
case 'erica': {
25+
return [
26+
{
27+
slug: 'first-post',
28+
},
29+
]
30+
}
31+
case 'sarah': {
32+
return [
33+
{
34+
slug: 'second-post',
35+
},
36+
]
37+
}
38+
case 'nick': {
39+
return [
40+
{
41+
slug: 'first-post',
42+
},
43+
{
44+
slug: 'second-post',
45+
},
46+
]
47+
}
48+
case 'rob': {
49+
return [
50+
{
51+
slug: 'second-post',
52+
},
53+
]
54+
}
55+
56+
default: {
57+
throw new Error(`unexpected author param received ${params.author}`)
58+
}
59+
}
60+
}

‎demos/default/app/blog/[author]/layout.js

+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
export default function Layout({ children, params }) {
2+
return (
3+
<>
4+
<p id="author-layout-params">{JSON.stringify(params)}</p>
5+
{children}
6+
</>
7+
)
8+
}
9+
10+
export function generateStaticParams(params) {
11+
console.log('/blog/[author] generateStaticParams', JSON.stringify(params))
12+
13+
return [{ author: 'nick' }, { author: 'sarah' }, { author: 'rob' }, { author: 'erica' }]
14+
}

‎demos/default/app/blog/[author]/page.js

+39
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import Link from 'next/link'
2+
3+
export default async function Page({ params }) {
4+
await fetch('http://example.com', {
5+
next: { revalidate: 10 },
6+
})
7+
return (
8+
<>
9+
<p id="page">/blog/[author]</p>
10+
<p id="params">{JSON.stringify(params)}</p>
11+
<p id="date">{Date.now()}</p>
12+
<Link href="/blog/erica" id="author-1">
13+
/blog/erica
14+
</Link>
15+
<br />
16+
<Link href="/blog/sarah" id="author-2">
17+
/blog/sarah
18+
</Link>
19+
<br />
20+
<Link href="/blog/nick" id="author-3">
21+
/blog/nick
22+
</Link>
23+
<br />
24+
25+
<Link href="/blog/erica/first-post" id="author-1-post-1">
26+
/blog/erica/first-post
27+
</Link>
28+
<br />
29+
<Link href="/blog/sarah/second-post" id="author-2-post-1">
30+
/blog/sarah/second-post
31+
</Link>
32+
<br />
33+
<Link href="/blog/nick/first-post" id="author-3-post-1">
34+
/blog/nick/first-post
35+
</Link>
36+
<br />
37+
</>
38+
)
39+
}

‎demos/default/app/layout.js

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
export default function Layout({ children }) {
2+
return (
3+
<html lang="en">
4+
<head>
5+
<title>my static blog</title>
6+
</head>
7+
<body>{children}</body>
8+
</html>
9+
)
10+
}

‎demos/default/next.config.js

+1
Original file line numberDiff line numberDiff line change
@@ -84,5 +84,6 @@ module.exports = {
8484
},
8585
experimental: {
8686
optimizeCss: false,
87+
appDir: true,
8788
},
8889
}

‎demos/default/pages/index.js

+6
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,12 @@ const Index = ({ shows, nodeEnv }) => {
158158
<Link href="/rewriteToStatic">Rewrite to static (should show getStaticProps/1)</Link>
159159
</li>
160160
</ul>
161+
<h4>appDir</h4>
162+
<ul>
163+
<li>
164+
<Link href="/blog/erica">app dir page</Link>
165+
</li>
166+
</ul>
161167
<h4>Preview mode</h4>
162168
<p>Preview mode: </p>
163169
<ul>

‎demos/default/tsconfig.json

+9-3
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,20 @@
1111
"moduleResolution": "node",
1212
"resolveJsonModule": true,
1313
"isolatedModules": true,
14-
"jsx": "preserve"
14+
"jsx": "preserve",
15+
"plugins": [
16+
{
17+
"name": "next"
18+
}
19+
]
1520
},
1621
"include": [
1722
"next-env.d.ts",
1823
"**/*.ts",
19-
"**/*.tsx"
24+
"**/*.tsx",
25+
".next/types/**/*.ts"
2026
],
2127
"exclude": [
2228
"node_modules"
2329
]
24-
}
30+
}

0 commit comments

Comments
 (0)