Skip to content

Commit

Permalink
Add failing case for location throw (#40445)
Browse files Browse the repository at this point in the history
Found that the cause was that `React.useId()` returns the same value across requests whereas it was being used to create a unique key. On further inspection that code could be removed altogether as `id` was not used client-side and the `rscCache` map is no longer needed as the flight instance is already being created per request so it can live in the context of the request execution.


## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] 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 `pnpm lint`
- [ ] The examples guidelines are followed from [our contributing doc](https://github.com/vercel/next.js/blob/canary/contributing.md#adding-examples)
  • Loading branch information
timneutkens committed Sep 12, 2022
1 parent 3851d90 commit f92a4ce
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 54 deletions.
8 changes: 5 additions & 3 deletions packages/next/client/app-index.tsx
Expand Up @@ -56,17 +56,19 @@ let initialServerDataWriter: ReadableStreamDefaultController | undefined =
let initialServerDataLoaded = false
let initialServerDataFlushed = false

function nextServerDataCallback(seg: [number, string, string]) {
function nextServerDataCallback(
seg: [isBootStrap: 0] | [isNotBootstrap: 1, responsePartial: string]
): void {
if (seg[0] === 0) {
initialServerDataBuffer = []
} else {
if (!initialServerDataBuffer)
throw new Error('Unexpected server data: missing bootstrap script.')

if (initialServerDataWriter) {
initialServerDataWriter.enqueue(encoder.encode(seg[2]))
initialServerDataWriter.enqueue(encoder.encode(seg[1]))
} else {
initialServerDataBuffer.push(seg[2])
initialServerDataBuffer.push(seg[1])
}
}
}
Expand Down
101 changes: 50 additions & 51 deletions packages/next/server/app-render.tsx
Expand Up @@ -54,8 +54,6 @@ function interopDefault(mod: any) {
return mod.default || mod
}

const rscCache = new Map()

// Shadowing check does not work with TypeScript enums
// eslint-disable-next-line no-shadow
const enum RecordStatus {
Expand Down Expand Up @@ -134,57 +132,59 @@ function preloadDataFetchingRecord(
*/
function useFlightResponse(
writable: WritableStream<Uint8Array>,
cachePrefix: string,
req: ReadableStream<Uint8Array>,
serverComponentManifest: any,
flightResponseRef: {
current: ReturnType<typeof createFromReadableStream> | null
},
nonce?: string
) {
const id = cachePrefix + ',' + (React as any).useId()
let entry = rscCache.get(id)
if (!entry) {
const [renderStream, forwardStream] = readableStreamTee(req)
entry = createFromReadableStream(renderStream, {
moduleMap: serverComponentManifest.__ssr_module_mapping__,
})
rscCache.set(id, entry)

let bootstrapped = false
// We only attach CSS chunks to the inlined data.
const forwardReader = forwardStream.getReader()
const writer = writable.getWriter()
const startScriptTag = nonce
? `<script nonce=${JSON.stringify(nonce)}>`
: '<script>'

function process() {
forwardReader.read().then(({ done, value }) => {
if (!bootstrapped) {
bootstrapped = true
writer.write(
encodeText(
`${startScriptTag}(self.__next_s=self.__next_s||[]).push(${htmlEscapeJsonString(
JSON.stringify([0, id])
)})</script>`
)
if (flightResponseRef.current) {
return flightResponseRef.current
}

const [renderStream, forwardStream] = readableStreamTee(req)
flightResponseRef.current = createFromReadableStream(renderStream, {
moduleMap: serverComponentManifest.__ssr_module_mapping__,
})

let bootstrapped = false
// We only attach CSS chunks to the inlined data.
const forwardReader = forwardStream.getReader()
const writer = writable.getWriter()
const startScriptTag = nonce
? `<script nonce=${JSON.stringify(nonce)}>`
: '<script>'

function process() {
forwardReader.read().then(({ done, value }) => {
if (!bootstrapped) {
bootstrapped = true
writer.write(
encodeText(
`${startScriptTag}(self.__next_s=self.__next_s||[]).push(${htmlEscapeJsonString(
JSON.stringify([0])
)})</script>`
)
}
if (done) {
rscCache.delete(id)
writer.close()
} else {
const responsePartial = decodeText(value)
const scripts = `${startScriptTag}(self.__next_s=self.__next_s||[]).push(${htmlEscapeJsonString(
JSON.stringify([1, id, responsePartial])
)})</script>`

writer.write(encodeText(scripts))
process()
}
})
}
process()
)
}
if (done) {
flightResponseRef.current = null
writer.close()
} else {
const responsePartial = decodeText(value)
const scripts = `${startScriptTag}(self.__next_s=self.__next_s||[]).push(${htmlEscapeJsonString(
JSON.stringify([1, responsePartial])
)})</script>`

writer.write(encodeText(scripts))
process()
}
})
}
return entry
process()

return flightResponseRef.current
}

/**
Expand All @@ -200,12 +200,10 @@ function createServerComponentRenderer(
}
},
{
cachePrefix,
transformStream,
serverComponentManifest,
serverContexts,
}: {
cachePrefix: string
transformStream: TransformStream<Uint8Array, Uint8Array>
serverComponentManifest: NonNullable<RenderOpts['serverComponentManifest']>
serverContexts: Array<
Expand Down Expand Up @@ -240,14 +238,16 @@ function createServerComponentRenderer(
return RSCStream
}

const flightResponseRef = { current: null }

const writable = transformStream.writable
return function ServerComponentWrapper() {
const reqStream = createRSCStream()
const response = useFlightResponse(
writable,
cachePrefix,
reqStream,
serverComponentManifest,
flightResponseRef,
nonce
)
return response.readRoot()
Expand Down Expand Up @@ -1110,7 +1110,6 @@ export async function renderToHTMLOrFlight(
},
ComponentMod,
{
cachePrefix: initialCanonicalUrl,
transformStream: serverComponentsInlinedTransformStream,
serverComponentManifest,
serverContexts,
Expand Down
@@ -0,0 +1,3 @@
export default function Loading() {
return <h2>Loading...</h2>
}
15 changes: 15 additions & 0 deletions test/e2e/app-dir/app/app/loading-bug/[categorySlug]/page.server.js
@@ -0,0 +1,15 @@
// @ts-ignore
import { experimental_use as use } from 'react'

const fetchCategory = async (categorySlug) => {
// artificial delay
await new Promise((resolve) => setTimeout(resolve, 3000))

return categorySlug + 'abc'
}

export default function Page({ params }) {
const category = use(fetchCategory(params.categorySlug))

return <div id="category-id">{category}</div>
}
15 changes: 15 additions & 0 deletions test/e2e/app-dir/index.test.ts
Expand Up @@ -1419,6 +1419,21 @@ describe('app dir', () => {
expect(errors).toInclude('Error during SSR')
})
})

describe('known bugs', () => {
it('should not share flight data between requests', async () => {
const fetches = await Promise.all(
[...new Array(5)].map(() =>
renderViaHTTP(next.url, '/loading-bug/electronics')
)
)

for (const text of fetches) {
const $ = cheerio.load(text)
expect($('#category-id').text()).toBe('electronicsabc')
}
})
})
}

describe('without assetPrefix', () => {
Expand Down

0 comments on commit f92a4ce

Please sign in to comment.