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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix uncaught error in getInitialProps when runtime is set to nodejs #34228

Merged
merged 9 commits into from
Feb 11, 2022
Merged
Show file tree
Hide file tree
Changes from all 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
23 changes: 18 additions & 5 deletions packages/next/build/webpack/loaders/next-flight-server-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,12 @@ async function parseImportsInfo(
}
break
}
case 'ExportDefaultExpression':
const exp = node.expression
if (exp.type === 'Identifier') {
defaultExportName = exp.value
}
break
default:
break
}
Expand Down Expand Up @@ -157,11 +163,18 @@ export default async function transformSource(
*/

const noop = `export const __rsc_noop__=()=>{${imports.join(';')}}`
const defaultExportNoop = isClientCompilation
? `export default function ${defaultExportName}(){}\n${defaultExportName}.__next_rsc__=1;`
: defaultExportName
? `${defaultExportName}.__next_rsc__=1;${defaultExportName}.__webpack_require__=__webpack_require__;`
: ''

let defaultExportNoop = ''
if (isClientCompilation) {
defaultExportNoop = `export default function ${
defaultExportName || 'ServerComponent'
}(){}\n${defaultExportName || 'ServerComponent'}.__next_rsc__=1;`
} else {
if (defaultExportName) {
// It's required to have the default export for pages. For other components, it's fine to leave it as is.
defaultExportNoop = `${defaultExportName}.__next_rsc__=1;${defaultExportName}.__webpack_require__=__webpack_require__;`
}
}

const transformed = transformedSource + '\n' + noop + '\n' + defaultExportNoop

Expand Down
7 changes: 6 additions & 1 deletion packages/next/server/render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -467,9 +467,14 @@ export async function renderToHTML(

const hasConcurrentFeatures = !!runtime

const isServerComponent = !!serverComponentManifest && hasConcurrentFeatures
const OriginalComponent = renderOpts.Component

// We don't need to opt-into the flight inlining logic if the page isn't a RSC.
const isServerComponent =
!!serverComponentManifest &&
hasConcurrentFeatures &&
(OriginalComponent as any).__next_rsc__

let Component: React.ComponentType<{}> | ((props: any) => JSX.Element) =
renderOpts.Component
let serverComponentsInlinedTransformStream: TransformStream<any, any> | null =
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import webdriver from 'next-webdriver'
import { renderViaHTTP } from 'next-test-utils'

export default async function basic(context) {
export default async function basic(context, { env }) {
it('should render 404 error correctly', async () => {
const path404HTML = await renderViaHTTP(context.appPort, '/404')
const pathNotFoundHTML = await renderViaHTTP(context.appPort, '/not-found')
Expand Down Expand Up @@ -35,4 +35,15 @@ export default async function basic(context) {

expect(hydrationContent).toBe('custom-404-pagenext_streaming_data')
})

it('should render 500 error correctly', async () => {
const path500HTML = await renderViaHTTP(context.appPort, '/err')

if (env === 'dev') {
// In dev mode it should show the error popup.
expect(path500HTML).toContain('Error: oops')
} else {
expect(path500HTML).toContain('custom-500-page')
}
})
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,31 +4,28 @@ import { join } from 'path'
import fs from 'fs-extra'
import webdriver from 'next-webdriver'

import {
File,
fetchViaHTTP,
findPort,
killApp,
renderViaHTTP,
} from 'next-test-utils'
import { fetchViaHTTP, findPort, killApp, renderViaHTTP } from 'next-test-utils'

import { nextBuild, nextStart, nextDev } from './utils'
import {
nextBuild,
nextStart,
nextDev,
appDir,
nativeModuleTestAppDir,
distDir,
documentPage,
appPage,
appServerPage,
error500Page,
nextConfig,
} from './utils'

import css from './css'
import rsc from './rsc'
import streaming from './streaming'
import basic from './basic'
import functions from './functions'

const appDir = join(__dirname, '../app')
const nativeModuleTestAppDir = join(__dirname, '../unsupported-native-module')
const distDir = join(__dirname, '../app/.next')
const documentPage = new File(join(appDir, 'pages/_document.jsx'))
const appPage = new File(join(appDir, 'pages/_app.js'))
const appServerPage = new File(join(appDir, 'pages/_app.server.js'))
const error500Page = new File(join(appDir, 'pages/500.js'))
const nextConfig = new File(join(appDir, 'next.config.js'))

const documentWithGip = `
import { Html, Head, Main, NextScript } from 'next/document'

Expand Down Expand Up @@ -153,14 +150,9 @@ describe('Edge runtime - prod', () => {
expect(html).toContain('foo.client')
})

it('should render 500 error correctly', async () => {
const path500HTML = await renderViaHTTP(context.appPort, '/err')
expect(path500HTML).toContain('custom-500-page')
})

basic(context)
rsc(context, 'edge')
basic(context, { env: 'prod' })
streaming(context)
rsc(context, { runtime: 'edge', env: 'prod' })
})

describe('Edge runtime - dev', () => {
Expand All @@ -185,35 +177,16 @@ describe('Edge runtime - dev', () => {
expect(content).toMatchInlineSnapshot('"foo.client"')
})

it('should not bundle external imports into client builds for RSC', async () => {
const html = await renderViaHTTP(context.appPort, '/external-imports')
expect(html).toContain('date:')

const distServerDir = join(distDir, 'static', 'chunks', 'pages')
const bundle = fs
.readFileSync(join(distServerDir, 'external-imports.js'))
.toString()

expect(bundle).not.toContain('moment')
})

it('should render 500 error correctly', async () => {
const path500HTML = await renderViaHTTP(context.appPort, '/err')

// In dev mode it should show the error popup.
expect(path500HTML).toContain('Error: oops')
})

basic(context)
rsc(context, 'edge')
basic(context, { env: 'dev' })
streaming(context)
rsc(context, { runtime: 'edge', env: 'dev' })
})

const nodejsRuntimeBasicSuite = {
runTests: (context, env) => {
basic(context)
basic(context, { env })
streaming(context)
rsc(context, 'nodejs')
rsc(context, { runtime: 'nodejs' })

if (env === 'prod') {
it('should generate middleware SSR manifests for Node.js', async () => {
Expand Down Expand Up @@ -241,8 +214,14 @@ const nodejsRuntimeBasicSuite = {
})
}
},
beforeAll: () => nextConfig.replace("runtime: 'edge'", "runtime: 'nodejs'"),
afterAll: () => nextConfig.restore(),
beforeAll: () => {
error500Page.write(page500)
nextConfig.replace("runtime: 'edge'", "runtime: 'nodejs'")
},
afterAll: () => {
error500Page.delete()
nextConfig.restore()
},
}

const customAppPageSuite = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,17 @@
import webdriver from 'next-webdriver'
import cheerio from 'cheerio'
import { renderViaHTTP, check } from 'next-test-utils'
import { join } from 'path'
import fs from 'fs-extra'

import { distDir } from './utils'

function getNodeBySelector(html, selector) {
const $ = cheerio.load(html)
return $(selector)
}

export default function (context, runtime) {
export default function (context, { runtime, env }) {
it('should render server components correctly', async () => {
const homeHTML = await renderViaHTTP(context.appPort, '/', null, {
headers: {
Expand Down Expand Up @@ -72,6 +76,22 @@ export default function (context, runtime) {
})
}

// For prod build, the directory contains the build ID so it's not deterministic.
huozhi marked this conversation as resolved.
Show resolved Hide resolved
// Only enable it for dev for now.
if (env === 'dev') {
it('should not bundle external imports into client builds for RSC', async () => {
const html = await renderViaHTTP(context.appPort, '/external-imports')
expect(html).toContain('date:')

const distServerDir = join(distDir, 'static', 'chunks', 'pages')
const bundle = fs
.readFileSync(join(distServerDir, 'external-imports.js'))
.toString()

expect(bundle).not.toContain('moment')
})
}

it('should handle multiple named exports correctly', async () => {
const clientExportsHTML = await renderViaHTTP(
context.appPort,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,25 @@
import { join } from 'path'
import {
File,
launchApp,
nextBuild as _nextBuild,
nextStart as _nextStart,
} from 'next-test-utils'

const nodeArgs = ['-r', join(__dirname, '../../react-18/test/require-hook.js')]

export const appDir = join(__dirname, '../app')
export const nativeModuleTestAppDir = join(
__dirname,
'../unsupported-native-module'
)
export const distDir = join(__dirname, '../app/.next')
export const documentPage = new File(join(appDir, 'pages/_document.jsx'))
export const appPage = new File(join(appDir, 'pages/_app.js'))
export const appServerPage = new File(join(appDir, 'pages/_app.server.js'))
export const error500Page = new File(join(appDir, 'pages/500.js'))
export const nextConfig = new File(join(appDir, 'next.config.js'))

export async function nextBuild(dir, options) {
return await _nextBuild(dir, [], {
...options,
Expand Down