Skip to content

Commit

Permalink
Fix page static info extractor for app dir (#42001)
Browse files Browse the repository at this point in the history
We currently only fallback to the global `runtime` option if the page
actually **needs** a runtime (not statically optimizable). That happens
for SSG (ISR) and SSR for pages/. But for app/, we will always need a
`runtime` to render server components.

Also in this PR, I improved the tests to actually **test** the runtime
to ensure it has `globalThis.EdgeRuntime` so it's not running in the
Node.js runtime
([ref](https://edge-runtime.vercel.app/features/available-apis#addressing-the-runtime)).

## Bug

- [ ] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have a helpful link attached, see `contributing.md`

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the
feature request has been accepted for implementation before opening a
PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have a helpful link attached, see `contributing.md`

## Documentation / Examples

- [ ] Make sure the linting passes by running `pnpm lint`
- [ ] The "examples guidelines" are followed from [our contributing
doc](https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md)

Co-authored-by: JJ Kasper <jj@jjsweb.site>
  • Loading branch information
shuding and ijjk committed Oct 27, 2022
1 parent 5d2e278 commit c8983a6
Show file tree
Hide file tree
Showing 16 changed files with 129 additions and 6 deletions.
7 changes: 5 additions & 2 deletions packages/next/build/analysis/get-page-static-info.ts
Expand Up @@ -273,8 +273,9 @@ export async function getPageStaticInfo(params: {
pageFilePath: string
isDev?: boolean
page?: string
pageType?: 'pages' | 'app'
}): Promise<PageStaticInfo> {
const { isDev, pageFilePath, nextConfig, page } = params
const { isDev, pageFilePath, nextConfig, page, pageType } = params

const fileContent = (await tryToReadFile(pageFilePath, !isDev)) || ''
if (
Expand Down Expand Up @@ -322,10 +323,12 @@ export async function getPageStaticInfo(params: {
}
}

const requiresServerRuntime = ssr || ssg || pageType === 'app'

resolvedRuntime =
SERVER_RUNTIME.edge === resolvedRuntime
? SERVER_RUNTIME.edge
: ssr || ssg
: requiresServerRuntime
? resolvedRuntime || nextConfig.experimental?.runtime
: undefined

Expand Down
1 change: 1 addition & 0 deletions packages/next/build/entries.ts
Expand Up @@ -374,6 +374,7 @@ export async function createEntrypoints(params: CreateEntrypointsParams) {
pageFilePath,
isDev,
page,
pageType: isInsideAppDir ? 'app' : 'pages',
})

const isServerComponent =
Expand Down
1 change: 1 addition & 0 deletions packages/next/build/index.ts
Expand Up @@ -1339,6 +1339,7 @@ export default async function build(
pagePath
),
nextConfig: config,
pageType,
})
: undefined

Expand Down
1 change: 1 addition & 0 deletions packages/next/server/dev/hot-reloader.ts
Expand Up @@ -600,6 +600,7 @@ export default class HotReloader {
pageFilePath: entryData.absolutePagePath,
nextConfig: this.config,
isDev: true,
pageType: isAppPath ? 'app' : 'pages',
})
: {}
const isServerComponent =
Expand Down
1 change: 1 addition & 0 deletions packages/next/server/dev/next-dev-server.ts
Expand Up @@ -381,6 +381,7 @@ export default class DevServer extends Server {
nextConfig: this.nextConfig,
page: rootFile,
isDev: true,
pageType: isAppPath ? 'app' : 'pages',
})

if (isMiddlewareFile(rootFile)) {
Expand Down
1 change: 1 addition & 0 deletions packages/next/server/dev/on-demand-entry-handler.ts
Expand Up @@ -630,6 +630,7 @@ export function onDemandEntryHandler({
pageFilePath: pagePathData.absolutePagePath,
nextConfig,
isDev: true,
pageType: isInsideAppDir ? 'app' : 'pages',
})

const added = new Map<CompilerNameValues, ReturnType<typeof addEntry>>()
Expand Down
32 changes: 32 additions & 0 deletions test/e2e/app-dir/app-edge-global.test.ts
@@ -0,0 +1,32 @@
import { createNext, FileRef } from 'e2e-utils'
import { NextInstance } from 'test/lib/next-modes/base'
import { renderViaHTTP } from 'next-test-utils'
import path from 'path'

describe('app-dir global edge configuration', () => {
if ((global as any).isNextDeploy) {
it('should skip next deploy for now', () => {})
return
}

let next: NextInstance

beforeAll(async () => {
next = await createNext({
files: new FileRef(path.join(__dirname, 'app-edge-global')),
dependencies: {
react: 'latest',
'react-dom': 'latest',
typescript: 'latest',
'@types/react': 'latest',
'@types/node': 'latest',
},
})
})
afterAll(() => next.destroy())

it('should handle edge only routes', async () => {
const appHtml = await renderViaHTTP(next.url, '/app-edge')
expect(appHtml).toContain('<p>Edge!</p>')
})
})
1 change: 1 addition & 0 deletions test/e2e/app-dir/app-edge-global/.gitignore
@@ -0,0 +1 @@
.vscode
11 changes: 11 additions & 0 deletions test/e2e/app-dir/app-edge-global/app/app-edge/layout.tsx
@@ -0,0 +1,11 @@
'use client'

// TODO-APP: support typing for useSelectedLayoutSegment
// @ts-ignore
import { useSelectedLayoutSegments } from 'next/navigation'

export default function Layout({ children }: { children: React.ReactNode }) {
// useSelectedLayoutSegment should not be thrown
useSelectedLayoutSegments()
return children
}
6 changes: 6 additions & 0 deletions test/e2e/app-dir/app-edge-global/app/app-edge/page.tsx
@@ -0,0 +1,6 @@
export default function Page() {
if ('EdgeRuntime' in globalThis) {
return <p>Edge!</p>
}
return <p>Node!</p>
}
8 changes: 8 additions & 0 deletions test/e2e/app-dir/app-edge-global/app/layout.tsx
@@ -0,0 +1,8 @@
export default function Root({ children }: { children: React.ReactNode }) {
return (
<html>
<head></head>
<body>{children}</body>
</html>
)
}
6 changes: 6 additions & 0 deletions test/e2e/app-dir/app-edge-global/next.config.js
@@ -0,0 +1,6 @@
module.exports = {
experimental: {
appDir: true,
runtime: 'experimental-edge',
},
}
29 changes: 29 additions & 0 deletions test/e2e/app-dir/app-edge-global/tsconfig.json
@@ -0,0 +1,29 @@
{
"compilerOptions": {
"target": "ES6",
"lib": ["dom", "dom.iterable", "esnext"],
"allowJs": true,
"skipLibCheck": true,
"strict": true,
"forceConsistentCasingInFileNames": true,
"noEmit": true,
"esModuleInterop": true,
"module": "esnext",
"moduleResolution": "node",
"resolveJsonModule": true,
"isolatedModules": true,
"jsx": "preserve",
"incremental": true,
"baseUrl": ".",
"paths": {
"@/ui/*": ["ui/*"]
},
"plugins": [
{
"name": "next"
}
]
},
"include": ["next-env.d.ts", "**/*.ts", "**/*.tsx", ".next/types/**/*.ts"],
"exclude": ["node_modules"]
}
6 changes: 3 additions & 3 deletions test/e2e/app-dir/app-edge.test.ts
Expand Up @@ -27,7 +27,7 @@ describe('app-dir edge SSR', () => {

it('should handle edge only routes', async () => {
const appHtml = await renderViaHTTP(next.url, '/app-edge')
expect(appHtml).toContain('<p>app-edge-ssr</p>')
expect(appHtml).toContain('<p>Edge!</p>')

const pageHtml = await renderViaHTTP(next.url, '/pages-edge')
expect(pageHtml).toContain('<p>pages-edge-ssr</p>')
Expand All @@ -39,7 +39,7 @@ describe('app-dir edge SSR', () => {
const content = await next.readFile(pageFile)

// Update rendered content
const updatedContent = content.replace('app-edge-ssr', 'edge-hmr')
const updatedContent = content.replace('Edge!', 'edge-hmr')
await next.patchFile(pageFile, updatedContent)
await check(async () => {
const html = await renderViaHTTP(next.url, '/app-edge')
Expand All @@ -51,7 +51,7 @@ describe('app-dir edge SSR', () => {
await check(async () => {
const html = await renderViaHTTP(next.url, '/app-edge')
return html
}, /app-edge-ssr/)
}, /Edge!/)
})
}
})
6 changes: 5 additions & 1 deletion test/e2e/app-dir/app-edge/app/app-edge/page.tsx
@@ -1,4 +1,8 @@
export default function Page() {
return <p>app-edge-ssr</p>
if ('EdgeRuntime' in globalThis) {
return <p>Edge!</p>
}
return <p>Node!</p>
}

export const runtime = 'experimental-edge'
18 changes: 18 additions & 0 deletions test/unit/parse-page-static-info.test.ts
Expand Up @@ -14,6 +14,7 @@ describe('parse page static info', () => {
const { runtime, ssr, ssg } = await getPageStaticInfo({
pageFilePath: join(fixtureDir, 'page-runtime/nodejs-ssr.js'),
nextConfig: createNextConfig(),
pageType: 'pages',
})
expect(runtime).toBe('nodejs')
expect(ssr).toBe(true)
Expand All @@ -24,6 +25,7 @@ describe('parse page static info', () => {
const { runtime, ssr, ssg } = await getPageStaticInfo({
pageFilePath: join(fixtureDir, 'page-runtime/nodejs.js'),
nextConfig: createNextConfig(),
pageType: 'pages',
})
expect(runtime).toBe(undefined)
expect(ssr).toBe(false)
Expand All @@ -34,6 +36,7 @@ describe('parse page static info', () => {
const { runtime } = await getPageStaticInfo({
pageFilePath: join(fixtureDir, 'page-runtime/edge.js'),
nextConfig: createNextConfig(),
pageType: 'pages',
})
expect(runtime).toBe('experimental-edge')
})
Expand All @@ -42,6 +45,7 @@ describe('parse page static info', () => {
const { runtime } = await getPageStaticInfo({
pageFilePath: join(fixtureDir, 'page-runtime/static.js'),
nextConfig: createNextConfig(),
pageType: 'pages',
})
expect(runtime).toBe(undefined)
})
Expand All @@ -50,6 +54,7 @@ describe('parse page static info', () => {
const { ssr, ssg } = await getPageStaticInfo({
pageFilePath: join(fixtureDir, 'page-runtime/ssr-variable-gssp.js'),
nextConfig: createNextConfig(),
pageType: 'pages',
})
expect(ssr).toBe(true)
expect(ssg).toBe(false)
Expand All @@ -61,6 +66,7 @@ describe('fallback to the global runtime configuration', () => {
const { runtime, ssr, ssg } = await getPageStaticInfo({
pageFilePath: join(fixtureDir, 'page-runtime/fallback-with-gsp.js'),
nextConfig: createNextConfig('experimental-edge'),
pageType: 'pages',
})
expect(runtime).toBe('experimental-edge')
expect(ssr).toBe(false)
Expand All @@ -71,9 +77,21 @@ describe('fallback to the global runtime configuration', () => {
const { runtime, ssr, ssg } = await getPageStaticInfo({
pageFilePath: join(fixtureDir, 'page-runtime/fallback-re-export-gsp.js'),
nextConfig: createNextConfig('experimental-edge'),
pageType: 'pages',
})
expect(runtime).toBe('experimental-edge')
expect(ssr).toBe(false)
expect(ssg).toBe(true)
})

it('should always fallback to the global runtime for app', async () => {
const { runtime, ssr, ssg } = await getPageStaticInfo({
pageFilePath: join(fixtureDir, 'page-runtime/static.js'),
nextConfig: createNextConfig('experimental-edge'),
pageType: 'app',
})
expect(runtime).toBe('experimental-edge')
expect(ssr).toBe(false)
expect(ssg).toBe(false)
})
})

0 comments on commit c8983a6

Please sign in to comment.