Skip to content

Commit

Permalink
Move handling of navigation to pages from new router (#41001)
Browse files Browse the repository at this point in the history
Ensures the static generation case is covered. Checks
application/octet-stream client-side, if it's not does a full page
navigation.

<!--
Thanks for opening a PR! Your contribution is much appreciated.
To make sure your PR is handled as smoothly as possible we request that
you follow the checklist sections below.
Choose the right checklist for the change that you're making:
-->

## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Errors have a helpful link attached, see `contributing.md`

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the
feature request has been accepted for implementation before opening a
PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have a helpful link attached, see `contributing.md`

## Documentation / Examples

- [ ] Make sure the linting passes by running `pnpm lint`
- [ ] The "examples guidelines" are followed from [our contributing
doc](https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md)
  • Loading branch information
timneutkens committed Sep 29, 2022
1 parent 335e918 commit c5d2c2d
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 40 deletions.
8 changes: 8 additions & 0 deletions packages/next/client/components/app-router.client.tsx
Expand Up @@ -65,6 +65,14 @@ export async function fetchServerResponse(
? urlToUrlWithoutFlightMarker(res.url)
: undefined

const isFlightResponse =
res.headers.get('content-type') === 'application/octet-stream'

// If fetch returns something different than flight response handle it like a mpa navigation
if (!isFlightResponse) {
return [res.url, undefined]
}

// Handle the `fetch` readable stream that can be unwrapped by `React.use`.
const flightData: FlightData = await createFromFetch(Promise.resolve(res))
return [flightData, canonicalUrl]
Expand Down
1 change: 0 additions & 1 deletion packages/next/export/worker.ts
Expand Up @@ -407,7 +407,6 @@ export default async function exportPage({
page,
query,
curRenderOpts as any,
false,
true
)
const html = result?.toUnchunkedString()
Expand Down
29 changes: 1 addition & 28 deletions packages/next/server/app-render.tsx
Expand Up @@ -6,7 +6,7 @@ import type { ServerRuntime } from '../types'
// @ts-ignore
import React, { experimental_use as use } from 'react'

import { ParsedUrlQuery, stringify as stringifyQuery } from 'querystring'
import { ParsedUrlQuery } from 'querystring'
import { createFromReadableStream } from 'next/dist/compiled/react-server-dom-webpack'
import { NextParsedUrlQuery } from './request-meta'
import RenderResult from './render-result'
Expand Down Expand Up @@ -609,7 +609,6 @@ export async function renderToHTMLOrFlight(
pathname: string,
query: NextParsedUrlQuery,
renderOpts: RenderOpts,
isPagesDir: boolean,
isStaticGeneration: boolean = false
): Promise<RenderResult | null> {
const isFlight = req.headers.__rsc__ !== undefined
Expand Down Expand Up @@ -638,32 +637,6 @@ export async function renderToHTMLOrFlight(
ComponentMod,
} = renderOpts

// Handle client-side navigation to pages directory
// This is handled before
if (isFlight && isPagesDir) {
stripInternalQueries(query)
const search = stringifyQuery(query)

// For pages dir, there is only the SSR pass and we don't have the bundled
// React subset. Here we directly import the flight renderer with the
// unbundled React.
// TODO-APP: Is it possible to hard code the flight response here instead of
// rendering it?
const ReactServerDOMWebpack = require('next/dist/compiled/react-server-dom-webpack/writer.browser.server')

// Empty so that the client-side router will do a full page navigation.
const flightData: FlightData = pathname + (search ? `?${search}` : '')
return new FlightRenderResult(
ReactServerDOMWebpack.renderToReadableStream(
flightData,
serverComponentManifest,
{
onError: flightDataRendererErrorHandler,
}
).pipeThrough(createBufferedTransformStream())
)
}

patchFetch(ComponentMod)

const staticGenerationAsyncStorage = ComponentMod.staticGenerationAsyncStorage
Expand Down
9 changes: 2 additions & 7 deletions packages/next/server/next-server.ts
Expand Up @@ -826,18 +826,13 @@ export default class NextNodeServer extends BaseServer {
renderOpts.serverCSSManifest = this.serverCSSManifest
renderOpts.fontLoaderManifest = this.fontLoaderManifest

if (
this.nextConfig.experimental.appDir &&
(renderOpts.isAppPath || req.headers.__rsc__)
) {
const isPagesDir = !renderOpts.isAppPath
if (this.nextConfig.experimental.appDir && renderOpts.isAppPath) {
return appRenderToHTMLOrFlight(
req.originalRequest,
res.originalResponse,
pathname,
query,
renderOpts,
isPagesDir
renderOpts
)
}

Expand Down
11 changes: 7 additions & 4 deletions test/e2e/app-dir/index.test.ts
Expand Up @@ -524,18 +524,21 @@ describe('app dir', () => {
}
})

// TODO-APP: should enable when implemented
it.skip('should allow linking from app page to pages page', async () => {
it('should allow linking from app page to pages page', async () => {
const browser = await webdriver(next.url, '/pages-linking')

try {
// Click the link.
await browser.elementById('app-link').click()
await browser.waitForElementByCss('#pages-link')
expect(await browser.waitForElementByCss('#pages-link').text()).toBe(
'To App Page'
)

// Click the other link.
await browser.elementById('pages-link').click()
await browser.waitForElementByCss('#app-link')
expect(await browser.waitForElementByCss('#app-link').text()).toBe(
'To Pages Page'
)
} finally {
await browser.close()
}
Expand Down

0 comments on commit c5d2c2d

Please sign in to comment.