Skip to content

Commit

Permalink
Fix router isReady and react 18 not being detected with no config (ve…
Browse files Browse the repository at this point in the history
…rcel#35762)

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 vercel#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: vercel#35754
x-ref: vercel#35749
  • Loading branch information
ijjk authored Mar 31, 2022
1 parent fe012ff commit d95aed6
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 20 deletions.
18 changes: 10 additions & 8 deletions packages/next/server/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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 || {}
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion packages/next/server/render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -561,7 +561,7 @@ export async function renderToHTML(
defaultAppGetInitialProps &&
!isSSG &&
!getServerSideProps &&
!hasConcurrentFeatures
!isServerComponent

for (const methodName of [
'getStaticProps',
Expand Down
12 changes: 1 addition & 11 deletions packages/next/taskfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
32 changes: 32 additions & 0 deletions test/development/basic/hmr.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
12 changes: 12 additions & 0 deletions test/development/basic/hmr/pages/auto-export-is-ready.js
Original file line number Diff line number Diff line change
@@ -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>
</>
)
}
20 changes: 20 additions & 0 deletions test/development/basic/hmr/pages/gsp-is-ready.js
Original file line number Diff line number Diff line change
@@ -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(),
},
}
}

0 comments on commit d95aed6

Please sign in to comment.