Fix router isReady and react 18 not being detected with no config (#3…

…5762) This fixes `router.isReady` being incorrect in dev mode due to the `isAutoExport` field being false from `hasConcurrentFeatures` being flagged similar to the static 404 in #35749. While investigating this I also noticed we aren't properly detecting react 18 when no `next.config.js` is present. ## Bug - [x] Related issues linked using `fixes #number` - [x] Integration tests added - [x] Errors have helpful link attached, see `contributing.md` Fixes: #35754 x-ref: #35749
vercel · Mar 31, 2022 · d95aed6 · d95aed6
1 parent fe012ff
commit d95aed6
Show file tree

Hide file tree

Showing 6 changed files with 76 additions and 20 deletions.
diff --git a/packages/next/server/config.ts b/packages/next/server/config.ts
@@ -188,6 +188,13 @@ function assignDefaults(userConfig: { [key: string]: any }) {
     }
   }
 
+  const hasReactRoot = shouldUseReactRoot()
+  if (hasReactRoot) {
+    // users might not have the `experimental` key in their config
+    result.experimental = result.experimental || {}
+    result.experimental.reactRoot = true
+  }
+
   if (result?.images) {
     const images: ImageConfig = result.images
 
@@ -681,13 +688,6 @@ export default async function loadConfig(
       )
     }
 
-    const hasReactRoot = shouldUseReactRoot()
-    if (hasReactRoot) {
-      // users might not have the `experimental` key in their config
-      userConfig.experimental = userConfig.experimental || {}
-      userConfig.experimental.reactRoot = true
-    }
-
     if (userConfig.amp?.canonicalBase) {
       const { canonicalBase } = userConfig.amp || ({} as any)
       userConfig.amp = userConfig.amp || {}
@@ -727,7 +727,9 @@ export default async function loadConfig(
     }
   }
 
-  const completeConfig = defaultConfig as NextConfigComplete
+  // always call assignDefaults to ensure settings like
+  // reactRoot can be updated correctly even with no next.config.js
+  const completeConfig = assignDefaults(defaultConfig) as NextConfigComplete
   completeConfig.configFileName = configFileName
   setHttpAgentOptions(completeConfig.httpAgentOptions)
   return completeConfig

diff --git a/packages/next/server/render.tsx b/packages/next/server/render.tsx
@@ -561,7 +561,7 @@ export async function renderToHTML(
     defaultAppGetInitialProps &&
     !isSSG &&
     !getServerSideProps &&
-    !hasConcurrentFeatures
+    !isServerComponent
 
   for (const methodName of [
     'getStaticProps',

diff --git a/packages/next/taskfile.js b/packages/next/taskfile.js
@@ -1861,18 +1861,8 @@ export async function pages_document(task, opts) {
     .target('dist/pages')
 }
 
-export async function pages_document_server(task, opts) {
-  await task
-    .source('pages/_document-concurrent.tsx')
-    .swc('client', { dev: opts.dev, keepImportAssertions: true })
-    .target('dist/pages')
-}
-
 export async function pages(task, opts) {
-  await task.parallel(
-    ['pages_app', 'pages_error', 'pages_document', 'pages_document_server'],
-    opts
-  )
+  await task.parallel(['pages_app', 'pages_error', 'pages_document'], opts)
 }
 
 export async function telemetry(task, opts) {

diff --git a/test/development/basic/hmr.test.ts b/test/development/basic/hmr.test.ts
@@ -30,6 +30,38 @@ describe('basic HMR', () => {
   })
   afterAll(() => next.destroy())
 
+  it('should have correct router.isReady for auto-export page', async () => {
+    let browser = await webdriver(next.url, '/auto-export-is-ready')
+
+    expect(await browser.elementByCss('#ready').text()).toBe('yes')
+    expect(JSON.parse(await browser.elementByCss('#query').text())).toEqual({})
+
+    browser = await webdriver(next.url, '/auto-export-is-ready?hello=world')
+
+    await check(async () => {
+      return browser.elementByCss('#ready').text()
+    }, 'yes')
+    expect(JSON.parse(await browser.elementByCss('#query').text())).toEqual({
+      hello: 'world',
+    })
+  })
+
+  it('should have correct router.isReady for getStaticProps page', async () => {
+    let browser = await webdriver(next.url, '/gsp-is-ready')
+
+    expect(await browser.elementByCss('#ready').text()).toBe('yes')
+    expect(JSON.parse(await browser.elementByCss('#query').text())).toEqual({})
+
+    browser = await webdriver(next.url, '/gsp-is-ready?hello=world')
+
+    await check(async () => {
+      return browser.elementByCss('#ready').text()
+    }, 'yes')
+    expect(JSON.parse(await browser.elementByCss('#query').text())).toEqual({
+      hello: 'world',
+    })
+  })
+
   describe('Hot Module Reloading', () => {
     describe('delete a page and add it back', () => {
       it('should load the page properly', async () => {

diff --git a/test/development/basic/hmr/pages/auto-export-is-ready.js b/test/development/basic/hmr/pages/auto-export-is-ready.js
@@ -0,0 +1,12 @@
+import { useRouter } from 'next/router'
+
+export default function Page(props) {
+  const router = useRouter()
+  return (
+    <>
+      <p>auto-export router.isReady</p>
+      <p id="query">{JSON.stringify(router.query)}</p>
+      <p id="ready">{router.isReady ? 'yes' : 'no'}</p>
+    </>
+  )
+}
diff --git a/test/development/basic/hmr/pages/gsp-is-ready.js b/test/development/basic/hmr/pages/gsp-is-ready.js
@@ -0,0 +1,20 @@
+import { useRouter } from 'next/router'
+
+export default function Page(props) {
+  const router = useRouter()
+  return (
+    <>
+      <p>getStaticProps router.isReady</p>
+      <p id="query">{JSON.stringify(router.query)}</p>
+      <p id="ready">{router.isReady ? 'yes' : 'no'}</p>
+    </>
+  )
+}
+
+export function getStaticProps() {
+  return {
+    props: {
+      now: Date.now(),
+    },
+  }
+}