From 2a0aefe81b81386d453cbde8679b17a4feee29d6 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Fri, 18 Feb 2022 11:47:22 +0100 Subject: [PATCH 1/4] Leverage existing component checking waring for streaming --- .../next-middleware-ssr-loader/index.ts | 4 -- packages/next/server/base-server.ts | 3 +- packages/next/server/render.tsx | 4 +- test/integration/react-18/app/next.config.js | 1 - test/integration/react-18/test/index.test.js | 66 ++++++++++++------- 5 files changed, 48 insertions(+), 30 deletions(-) diff --git a/packages/next/build/webpack/loaders/next-middleware-ssr-loader/index.ts b/packages/next/build/webpack/loaders/next-middleware-ssr-loader/index.ts index d457fcaa72d3668..870a6b901693a23 100644 --- a/packages/next/build/webpack/loaders/next-middleware-ssr-loader/index.ts +++ b/packages/next/build/webpack/loaders/next-middleware-ssr-loader/index.ts @@ -39,10 +39,6 @@ export default async function middlewareSSRLoader(this: any) { const reactLoadableManifest = self.__REACT_LOADABLE_MANIFEST const rscManifest = self.__RSC_MANIFEST - if (typeof pageMod.default !== 'function') { - throw new Error('Your page must export a \`default\` component') - } - // Set server context self.__server_context = { page: ${JSON.stringify(page)}, diff --git a/packages/next/server/base-server.ts b/packages/next/server/base-server.ts index 77160619770fe7e..ecd833fbcac5553 100644 --- a/packages/next/server/base-server.ts +++ b/packages/next/server/base-server.ts @@ -1122,7 +1122,8 @@ export default abstract class Server { const isSSG = !!components.getStaticProps const hasServerProps = !!components.getServerSideProps const hasStaticPaths = !!components.getStaticPaths - const hasGetInitialProps = !!(components.Component as any).getInitialProps + const hasGetInitialProps = !!(components.ComponentMod as any) + .getInitialProps // Toggle whether or not this is a Data request const isDataReq = !!query._nextDataReq && (isSSG || hasServerProps) diff --git a/packages/next/server/render.tsx b/packages/next/server/render.tsx index d18e1f336b006ee..ce5b2a5d9a42794 100644 --- a/packages/next/server/render.tsx +++ b/packages/next/server/render.tsx @@ -545,7 +545,7 @@ export async function renderToHTML( const defaultAppGetInitialProps = App.getInitialProps === (App as any).origGetInitialProps - const hasPageGetInitialProps = !!(Component as any).getInitialProps + const hasPageGetInitialProps = !!(Component as any)?.getInitialProps const pageIsDynamic = isDynamicRoute(pathname) @@ -561,7 +561,7 @@ export async function renderToHTML( 'getServerSideProps', 'getStaticPaths', ]) { - if ((Component as any)[methodName]) { + if ((Component as any)?.[methodName]) { throw new Error( `page ${pathname} ${methodName} ${GSSP_COMPONENT_MEMBER_ERROR}` ) diff --git a/test/integration/react-18/app/next.config.js b/test/integration/react-18/app/next.config.js index 270a344a62435f5..30e690a6231c054 100644 --- a/test/integration/react-18/app/next.config.js +++ b/test/integration/react-18/app/next.config.js @@ -3,7 +3,6 @@ const withReact18 = require('../test/with-react-18') module.exports = withReact18({ // reactStrictMode: true, experimental: { - reactRoot: true, // runtime: 'edge', }, images: { diff --git a/test/integration/react-18/test/index.test.js b/test/integration/react-18/test/index.test.js index 8d68baaef1912b7..341785a9214d6cf 100644 --- a/test/integration/react-18/test/index.test.js +++ b/test/integration/react-18/test/index.test.js @@ -11,11 +11,14 @@ import { nextStart, renderViaHTTP, fetchViaHTTP, + hasRedbox, + getRedboxHeader, } from 'next-test-utils' import blocking from './blocking' import concurrent from './concurrent' import basics from './basics' import strictMode from './strict-mode' +import webdriver from 'next-webdriver' // overrides react and react-dom to v18 const nodeArgs = ['-r', join(__dirname, 'require-hook.js')] @@ -23,6 +26,7 @@ const appDir = join(__dirname, '../app') const nextConfig = new File(join(appDir, 'next.config.js')) const dynamicHello = new File(join(appDir, 'components/dynamic-hello.js')) const unwrappedPage = new File(join(appDir, 'pages/suspense/unwrapped.js')) +const invalidPage = new File(join(appDir, 'pages/invalid.js')) const USING_CREATE_ROOT = 'Using the createRoot API for React' @@ -141,19 +145,9 @@ describe('Blocking mode', () => { }) function runTestsAgainstRuntime(runtime) { - describe(`Concurrent mode in the ${runtime} runtime`, () => { - beforeAll(async () => { - nextConfig.replace("// runtime: 'edge'", `runtime: '${runtime}'`) - dynamicHello.replace('suspense = false', `suspense = true`) - // `noSSR` mode will be ignored by suspense - dynamicHello.replace('let ssr', `let ssr = false`) - }) - afterAll(async () => { - nextConfig.restore() - dynamicHello.restore() - }) - - runTests(`runtime is set to '${runtime}'`, (context) => { + runTests( + `Concurrent mode in the ${runtime} runtime`, + (context, env) => { concurrent(context, (p, q) => renderViaHTTP(context.appPort, p, q)) it('should stream to users', async () => { @@ -189,17 +183,44 @@ function runTestsAgainstRuntime(runtime) { ) expect(res.headers.get('etag')).toBeDefined() }) - }) - }) + + if (env === 'dev') { + it('should recover after undefined exported as default', async () => { + invalidPage.write(`export const value = 1`) + const browser = await webdriver(context.appPort, '/invalid') + + expect(await hasRedbox(browser)).toBe(true) + expect(await getRedboxHeader(browser)).toMatch( + `Error: The default export is not a React Component in page: "/invalid"` + ) + + invalidPage.delete() + }) + } + }, + { + beforeAll: () => { + nextConfig.replace("// runtime: 'edge'", `runtime: '${runtime}'`) + dynamicHello.replace('suspense = false', `suspense = true`) + // `noSSR` mode will be ignored by suspense + dynamicHello.replace('let ssr', `let ssr = false`) + }, + afterAll: () => { + nextConfig.restore() + dynamicHello.restore() + }, + } + ) } -function runTest(mode, name, fn) { +function runTest(env, name, fn, options) { const context = { appDir } - describe(`${name} (${mode})`, () => { + describe(`${name} (${env})`, () => { beforeAll(async () => { context.appPort = await findPort() context.stderr = '' - if (mode === 'dev') { + options?.beforeAll(env) + if (env === 'dev') { context.server = await launchApp(context.appDir, context.appPort, { nodeArgs, onStderr(msg) { @@ -217,16 +238,17 @@ function runTest(mode, name, fn) { } }) afterAll(async () => { + options?.afterAll(env) await killApp(context.server) }) - fn(context) + fn(context, env) }) } runTestsAgainstRuntime('edge') runTestsAgainstRuntime('nodejs') -function runTests(name, fn) { - runTest('dev', name, fn) - runTest('prod', name, fn) +function runTests(name, fn, options) { + runTest('dev', name, fn, options) + runTest('prod', name, fn, options) } From 383f44824ac6360a5c19755946d149bb73b3bb59 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Fri, 18 Feb 2022 12:58:50 +0100 Subject: [PATCH 2/4] revert test case reactRoot --- test/integration/react-18/app/next.config.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/integration/react-18/app/next.config.js b/test/integration/react-18/app/next.config.js index 30e690a6231c054..270a344a62435f5 100644 --- a/test/integration/react-18/app/next.config.js +++ b/test/integration/react-18/app/next.config.js @@ -3,6 +3,7 @@ const withReact18 = require('../test/with-react-18') module.exports = withReact18({ // reactStrictMode: true, experimental: { + reactRoot: true, // runtime: 'edge', }, images: { From c88fff1c81b16a20aaca778f557da88756f0d4ee Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Fri, 18 Feb 2022 14:18:19 +0100 Subject: [PATCH 3/4] fix typing and test --- packages/next/server/base-server.ts | 5 ++- packages/next/server/load-components.ts | 8 +++-- test/integration/react-18/test/index.test.js | 35 +++++++++++--------- 3 files changed, 27 insertions(+), 21 deletions(-) diff --git a/packages/next/server/base-server.ts b/packages/next/server/base-server.ts index ecd833fbcac5553..51b7f7bfa173539 100644 --- a/packages/next/server/base-server.ts +++ b/packages/next/server/base-server.ts @@ -12,7 +12,7 @@ import type { Rewrite } from '../lib/load-custom-routes' import type { RenderOpts, RenderOptsPartial } from './render' import type { ResponseCacheEntry, ResponseCacheValue } from './response-cache' import type { UrlWithParsedQuery } from 'url' -import type { CacheFs } from '../shared/lib/utils' +import type { CacheFs, NextComponentType } from '../shared/lib/utils' import type { PreviewData } from 'next/types' import type { PagesManifest } from '../build/webpack/plugins/pages-manifest-plugin' import type { BaseNextRequest, BaseNextResponse } from './base-http' @@ -1122,8 +1122,7 @@ export default abstract class Server { const isSSG = !!components.getStaticProps const hasServerProps = !!components.getServerSideProps const hasStaticPaths = !!components.getStaticPaths - const hasGetInitialProps = !!(components.ComponentMod as any) - .getInitialProps + const hasGetInitialProps = !!components.Component?.getInitialProps // Toggle whether or not this is a Data request const isDataReq = !!query._nextDataReq && (isSSG || hasServerProps) diff --git a/packages/next/server/load-components.ts b/packages/next/server/load-components.ts index e4c86fee8bcb33e..b4d4b02a0ac0d9d 100644 --- a/packages/next/server/load-components.ts +++ b/packages/next/server/load-components.ts @@ -1,3 +1,8 @@ +import type { + AppType, + DocumentType, + NextComponentType, +} from '../shared/lib/utils' import { BUILD_MANIFEST, REACT_LOADABLE_MANIFEST, @@ -5,7 +10,6 @@ import { import { join } from 'path' import { requirePage } from './require' import { BuildManifest } from './get-page-files' -import { AppType, DocumentType } from '../shared/lib/utils' import { interopDefault } from '../lib/interop-default' import { PageConfig, @@ -22,7 +26,7 @@ export type ManifestItem = { export type ReactLoadableManifest = { [moduleId: string]: ManifestItem } export type LoadComponentsReturnType = { - Component: React.ComponentType + Component: NextComponentType pageConfig: PageConfig buildManifest: BuildManifest reactLoadableManifest: ReactLoadableManifest diff --git a/test/integration/react-18/test/index.test.js b/test/integration/react-18/test/index.test.js index 341785a9214d6cf..2660a9fd20b1e13 100644 --- a/test/integration/react-18/test/index.test.js +++ b/test/integration/react-18/test/index.test.js @@ -150,6 +150,17 @@ function runTestsAgainstRuntime(runtime) { (context, env) => { concurrent(context, (p, q) => renderViaHTTP(context.appPort, p, q)) + if (env === 'dev') { + it('should recover after undefined exported as default', async () => { + const browser = await webdriver(context.appPort, '/invalid') + + expect(await hasRedbox(browser)).toBe(true) + expect(await getRedboxHeader(browser)).toMatch( + `Error: The default export is not a React Component in page: "/invalid"` + ) + }) + } + it('should stream to users', async () => { const res = await fetchViaHTTP(context.appPort, '/ssr') expect(res.headers.get('etag')).toBeNull() @@ -183,29 +194,21 @@ function runTestsAgainstRuntime(runtime) { ) expect(res.headers.get('etag')).toBeDefined() }) - - if (env === 'dev') { - it('should recover after undefined exported as default', async () => { - invalidPage.write(`export const value = 1`) - const browser = await webdriver(context.appPort, '/invalid') - - expect(await hasRedbox(browser)).toBe(true) - expect(await getRedboxHeader(browser)).toMatch( - `Error: The default export is not a React Component in page: "/invalid"` - ) - - invalidPage.delete() - }) - } }, { - beforeAll: () => { + beforeAll: (env) => { + if (env === 'dev') { + invalidPage.write(`export const value = 1`) + } nextConfig.replace("// runtime: 'edge'", `runtime: '${runtime}'`) dynamicHello.replace('suspense = false', `suspense = true`) // `noSSR` mode will be ignored by suspense dynamicHello.replace('let ssr', `let ssr = false`) }, - afterAll: () => { + afterAll: (env) => { + if (env === 'dev') { + invalidPage.delete() + } nextConfig.restore() dynamicHello.restore() }, From 45bc8a20c74b0e01048521a035050484b037e50d Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Fri, 18 Feb 2022 15:34:00 +0100 Subject: [PATCH 4/4] fix lint --- packages/next/server/base-server.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/next/server/base-server.ts b/packages/next/server/base-server.ts index 51b7f7bfa173539..19e62dcc11f23bc 100644 --- a/packages/next/server/base-server.ts +++ b/packages/next/server/base-server.ts @@ -12,7 +12,7 @@ import type { Rewrite } from '../lib/load-custom-routes' import type { RenderOpts, RenderOptsPartial } from './render' import type { ResponseCacheEntry, ResponseCacheValue } from './response-cache' import type { UrlWithParsedQuery } from 'url' -import type { CacheFs, NextComponentType } from '../shared/lib/utils' +import type { CacheFs } from '../shared/lib/utils' import type { PreviewData } from 'next/types' import type { PagesManifest } from '../build/webpack/plugins/pages-manifest-plugin' import type { BaseNextRequest, BaseNextResponse } from './base-http'