Skip to content

Commit

Permalink
Ensure component load order (#22731)
Browse files Browse the repository at this point in the history
This ensures we load `_document` then `_app` and then the page's component in all cases which matches behavior between the serverless target and the default server target.  Additional tests to ensure this order is followed has been added to prevent regression. 

Fixes: #22732
  • Loading branch information
ijjk authored Mar 3, 2021
1 parent e04d399 commit 1435de1
Show file tree
Hide file tree
Showing 10 changed files with 58 additions and 11 deletions.
3 changes: 2 additions & 1 deletion packages/next/build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -680,7 +680,8 @@ export default async function build(
)
return staticCheckWorkers.isPageStatic(
page,
serverBundle,
distDir,
isLikeServerless,
runtimeEnvConfig,
config.i18n?.locales,
config.i18n?.defaultLocale,
Expand Down
9 changes: 6 additions & 3 deletions packages/next/build/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import { normalizeLocalePath } from '../next-server/lib/i18n/normalize-locale-pa
import * as Log from './output/log'
import opentelemetryApi from '@opentelemetry/api'
import { tracer, traceAsyncFn } from './tracer'
import { loadComponents } from '../next-server/server/load-components'

const fileGzipStats: { [k: string]: Promise<number> } = {}
const fsStatGzip = (file: string) => {
Expand Down Expand Up @@ -713,7 +714,8 @@ export async function buildStaticPaths(

export async function isPageStatic(
page: string,
serverBundle: string,
distDir: string,
serverless: boolean,
runtimeEnvConfig: any,
locales?: string[],
defaultLocale?: string,
Expand Down Expand Up @@ -742,8 +744,9 @@ export async function isPageStatic(
require('../next-server/lib/runtime-config').setConfig(
runtimeEnvConfig
)
const mod = await require(serverBundle)
const Comp = await (mod.default || mod)
const components = await loadComponents(distDir, page, serverless)
const mod = components.ComponentMod
const Comp = mod.default || mod

if (
!Comp ||
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,8 @@ const nextServerlessLoader: webpack.loader.Loader = function () {
}
import { getPageHandler } from 'next/dist/build/webpack/loaders/next-serverless-loader/page-handler'
const documentModule = require("${absoluteDocumentPath}")
const appMod = require('${absoluteAppPath}')
let App = appMod.default || appMod.then && appMod.then(mod => mod.default);
Expand All @@ -163,7 +165,7 @@ const nextServerlessLoader: webpack.loader.Loader = function () {
pageComponent: Component,
pageConfig: config,
appModule: App,
documentModule: require("${absoluteDocumentPath}"),
documentModule: documentModule,
errorModule: require("${absoluteErrorPath}"),
notFoundModule: ${
absolute404Path ? `require("${absolute404Path}")` : undefined
Expand Down
5 changes: 5 additions & 0 deletions packages/next/next-server/server/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import chalk from 'chalk'
import findUp from 'next/dist/compiled/find-up'
import { basename, extname } from 'path'
import * as Log from '../../build/output/log'
import { hasNextSupport } from '../../telemetry/ci-info'
import { CONFIG_FILE } from '../lib/constants'
import { execOnce } from '../lib/utils'
import { defaultConfig, normalizeConfig } from './config-shared'
Expand Down Expand Up @@ -443,6 +444,10 @@ export default async function loadConfig(
)
}

if (hasNextSupport) {
userConfig.target = process.env.NEXT_PRIVATE_TARGET || 'server'
}

return assignDefaults({
configOrigin: CONFIG_FILE,
configFile: path,
Expand Down
11 changes: 6 additions & 5 deletions packages/next/next-server/server/load-components.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export type LoadComponentsReturnType = {
getStaticProps?: GetStaticProps
getStaticPaths?: GetStaticPaths
getServerSideProps?: GetServerSideProps
ComponentMod: any
}

export async function loadComponents(
Expand All @@ -54,14 +55,13 @@ export async function loadComponents(
getStaticProps,
getStaticPaths,
getServerSideProps,
ComponentMod: Component,
} as LoadComponentsReturnType
}

const [DocumentMod, AppMod, ComponentMod] = await Promise.all([
requirePage('/_document', distDir, serverless),
requirePage('/_app', distDir, serverless),
requirePage(pathname, distDir, serverless),
])
const DocumentMod = await requirePage('/_document', distDir, serverless)
const AppMod = await requirePage('/_app', distDir, serverless)
const ComponentMod = await requirePage(pathname, distDir, serverless)

const [
buildManifest,
Expand All @@ -86,6 +86,7 @@ export async function loadComponents(
buildManifest,
reactLoadableManifest,
pageConfig: ComponentMod.config || {},
ComponentMod,
getServerSideProps,
getStaticProps,
getStaticPaths,
Expand Down
13 changes: 13 additions & 0 deletions test/integration/required-server-files/lib/config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
let curConfig

const idk = Math.random()

export default () => {
console.log('returning config', idk, curConfig)
return curConfig
}

export function setConfig(configValue) {
curConfig = configValue
console.log('set config', idk, configValue)
}
2 changes: 2 additions & 0 deletions test/integration/required-server-files/next.config.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
module.exports = {
// ensure incorrect target is overridden by env
target: 'serverless',
rewrites() {
return [
{
Expand Down
7 changes: 7 additions & 0 deletions test/integration/required-server-files/pages/_app.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { setConfig } from '../lib/config'

setConfig({ hello: 'world' })

export default function MyApp({ Component, pageProps }) {
return <Component {...pageProps} />
}
7 changes: 7 additions & 0 deletions test/integration/required-server-files/pages/index.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
import { useRouter } from 'next/router'
import getConfig from '../lib/config'

const localConfig = getConfig()

if (localConfig.hello !== 'world') {
throw new Error('oof import order is wrong, _app comes first')
}

export const getServerSideProps = ({ req }) => {
return {
Expand Down
8 changes: 7 additions & 1 deletion test/integration/required-server-files/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,11 @@ let errors = []
describe('Required Server Files', () => {
beforeAll(async () => {
await fs.remove(join(appDir, '.next'))
await nextBuild(appDir)
await nextBuild(appDir, undefined, {
env: {
NOW_BUILDER: '1',
},
})

buildId = await fs.readFile(join(appDir, '.next/BUILD_ID'), 'utf8')
requiredFilesManifest = await fs.readJSON(
Expand Down Expand Up @@ -88,6 +92,8 @@ describe('Required Server Files', () => {
console.log('checking', file)
expect(await fs.exists(join(appDir, file))).toBe(true)
}

expect(await fs.exists(join(appDir, '.next/server'))).toBe(true)
})

it('should render SSR page correctly', async () => {
Expand Down

0 comments on commit 1435de1

Please sign in to comment.