Skip to content

Commit

Permalink
Always defer close tags to be the suffix of body stream (#50056)
Browse files Browse the repository at this point in the history
Currently the suffix logic is for pages so nothing happens in app dir. This PR changes it to **always** apply the `createSuffixStream(closeTag)` transform, to defer `</body></html>` to the end of the stream.
fix #48372
fix NEXT-1200
  • Loading branch information
shuding committed May 19, 2023
1 parent f0d0811 commit 26ea9c6
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 2 deletions.
37 changes: 35 additions & 2 deletions packages/next/src/server/stream-utils/node-web-streams-helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,25 @@ export function createInlineDataStream(
export function createSuffixStream(
suffix: string
): TransformStream<Uint8Array, Uint8Array> {
let foundSuffix = false
const textDecoder = new TextDecoder()

// Remove suffix from the stream, and enqueue it back in flush
return new TransformStream({
transform(chunk, controller) {
if (!suffix || foundSuffix) {
return controller.enqueue(chunk)
}

const content = decodeText(chunk, textDecoder)
if (content.endsWith(suffix)) {
foundSuffix = true
const contentWithoutSuffix = content.slice(0, -suffix.length)
controller.enqueue(encodeText(contentWithoutSuffix))
} else {
controller.enqueue(chunk)
}
},
flush(controller) {
if (suffix) {
controller.enqueue(encodeText(suffix))
Expand Down Expand Up @@ -355,7 +373,6 @@ export async function continueFromInitialStream(
serverInsertedHTMLToHead,
validateRootLayout,
}: {
suffix?: string
dataStream?: ReadableStream<Uint8Array>
generateStaticHTML: boolean
getServerInsertedHTML?: () => Promise<string>
Expand All @@ -364,23 +381,38 @@ export async function continueFromInitialStream(
assetPrefix?: string
getTree: () => FlightRouterState
}
// Suffix to inject after the buffered data, but before the close tags.
suffix?: string
}
): Promise<ReadableStream<Uint8Array>> {
const closeTag = '</body></html>'

// Suffix itself might contain close tags at the end, so we need to split it.
const suffixUnclosed = suffix ? suffix.split(closeTag)[0] : null

if (generateStaticHTML) {
await renderStream.allReady
}

const transforms: Array<TransformStream<Uint8Array, Uint8Array>> = [
// Buffer everything to avoid flushing too frequently
createBufferedTransformStream(),

// Insert generated tags to head
getServerInsertedHTML && !serverInsertedHTMLToHead
? createInsertedHTMLStream(getServerInsertedHTML)
: null,

// Insert suffix content
suffixUnclosed != null ? createDeferredSuffixStream(suffixUnclosed) : null,

// Insert the flight data stream
dataStream ? createInlineDataStream(dataStream) : null,
suffixUnclosed != null ? createSuffixStream(closeTag) : null,

// Close tags should always be deferred to the end
createSuffixStream(closeTag),

// Special head insertions
createHeadInsertionTransformStream(async () => {
// TODO-APP: Insert server side html to end of head in app layout rendering, to avoid
// hydration errors. Remove this once it's ready to be handled by react itself.
Expand All @@ -390,6 +422,7 @@ export async function continueFromInitialStream(
: ''
return serverInsertedHTML
}),

validateRootLayout
? createRootLayoutValidatorStream(
validateRootLayout.assetPrefix,
Expand Down
9 changes: 9 additions & 0 deletions test/e2e/app-dir/app/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,15 @@ createNextDescribe(
expect(html).toContain('hello from app/dashboard')
})

it('should ensure the </body></html> suffix is at the end of the stream', async () => {
const html = await next.render('/dashboard')

// It must end with the suffix and not contain it anywhere else.
const suffix = '</body></html>'
expect(html).toEndWith(suffix)
expect(html.slice(0, -suffix.length)).not.toContain(suffix)
})

if (!isNextDeploy) {
it('should serve /index as separate page', async () => {
const stderr = []
Expand Down

0 comments on commit 26ea9c6

Please sign in to comment.