Skip to content

Commit

Permalink
Use renderToStaticMarkup to render documentHTML (#36213)
Browse files Browse the repository at this point in the history
There wasn't a strong reason to choose `renderToStream` over `renderToStaticMarkup` for the document wrapper. But due to problems like #35870, we can switch back to the static renderer for now.

Fixes #35870.

## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have 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 helpful link attached, see `contributing.md`

## Documentation / Examples

- [ ] Make sure the linting passes by running `yarn lint`


Co-authored-by: JJ Kasper <22380829+ijjk@users.noreply.github.com>
  • Loading branch information
shuding and ijjk committed Apr 19, 2022
1 parent 0e2fd92 commit 90d3478
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 13 deletions.
4 changes: 3 additions & 1 deletion packages/next/server/node-web-streams-helper.ts
@@ -1,3 +1,5 @@
import { nonNullable } from '../lib/non-nullable'

export function readableStreamTee<T = any>(
readable: ReadableStream<T>
): [ReadableStream<T>, ReadableStream<T>] {
Expand Down Expand Up @@ -172,7 +174,7 @@ export async function continueFromInitialStream({
suffixUnclosed != null ? createPrefixStream(suffixUnclosed) : null,
dataStream ? createInlineDataStream(dataStream) : null,
suffixUnclosed != null ? createSuffixStream(closeTag) : null,
].filter(Boolean) as any
].filter(nonNullable)

return transforms.reduce(
(readable, transform) => readable.pipeThrough(transform),
Expand Down
13 changes: 1 addition & 12 deletions packages/next/server/render.tsx
Expand Up @@ -72,7 +72,6 @@ import {
streamToString,
chainStreams,
createBufferedTransformStream,
renderToStream,
renderToInitialStream,
continueFromInitialStream,
} from './node-web-streams-helper'
Expand Down Expand Up @@ -1628,17 +1627,7 @@ export async function renderToHTML(
</AmpStateContext.Provider>
)

let documentHTML: string
if (hasConcurrentFeatures) {
const documentStream = await renderToStream({
ReactDOMServer,
element: document,
generateStaticHTML: true,
})
documentHTML = await streamToString(documentStream)
} else {
documentHTML = ReactDOMServer.renderToStaticMarkup(document)
}
const documentHTML = ReactDOMServer.renderToStaticMarkup(document)

if (process.env.NODE_ENV !== 'production') {
const nonRenderedComponents = []
Expand Down
Expand Up @@ -10,6 +10,7 @@ export default function Index({ header }) {
<div>
<Head>
<meta name="rsc-title" content="index" />
<title>hello, {envVar}</title>
</Head>
<h1>{`component:index.server`}</h1>
<div>{'env:' + envVar}</div>
Expand Down
Expand Up @@ -10,6 +10,11 @@ export default async function basic(context, { env }) {
expect(pathNotFoundHTML).toContain('custom-404-page')
})

it('should render title correctly', async () => {
const res = await renderViaHTTP(context.appPort, '/')
expect(res).toContain('<title>hello, env_var_test</title>')
})

it('should support api routes', async () => {
const res = await renderViaHTTP(context.appPort, '/api/ping')
expect(res).toContain('pong')
Expand Down

0 comments on commit 90d3478

Please sign in to comment.