Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor: remove isRSCRequestCheck #65587

Merged
merged 1 commit into from
May 29, 2024
Merged

refactor: remove isRSCRequestCheck #65587

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 8 additions & 13 deletions packages/next/src/server/base-server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -844,8 +844,7 @@ export default abstract class Server<
this.handleRequestImpl(req, res, parsedUrl).finally(() => {
if (!span) return

const isRSCRequest = isRSCRequestCheck(req) ?? false

const isRSCRequest = getRequestMeta(req, 'isRSCRequest') ?? false
span.setAttributes({
'http.status_code': res.statusCode,
'next.rsc': isRSCRequest,
Expand Down Expand Up @@ -940,6 +939,7 @@ export default abstract class Server<
)
}

// Update the `x-forwarded-*` headers.
const { originalRequest = null } = isNodeNextRequest(req) ? req : {}
const xForwardedProto = originalRequest?.headers['x-forwarded-proto']
const isHttps = xForwardedProto
Expand Down Expand Up @@ -1831,7 +1831,7 @@ export default abstract class Server<
resolvedPathname: string
): void {
const baseVaryHeader = `${RSC_HEADER}, ${NEXT_ROUTER_STATE_TREE}, ${NEXT_ROUTER_PREFETCH_HEADER}`
const isRSCRequest = isRSCRequestCheck(req)
const isRSCRequest = getRequestMeta(req, 'isRSCRequest') ?? false

let addedNextUrlToVary = false

Expand Down Expand Up @@ -1955,9 +1955,11 @@ export default abstract class Server<
* prefetch request.
*/
const isPrefetchRSCRequest =
(req.headers[NEXT_ROUTER_PREFETCH_HEADER.toLowerCase()] === '1' ||
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is isPrefetchRSCRequest guaranteed to have been set by the time we hit this block?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case yes, the vary header is only called after a render is performed (and we have a resolved pathname, and know it's an app path or not) because it's assigned in the request handler.

getRequestMeta(req, 'isPrefetchRSCRequest')) ??
false
getRequestMeta(req, 'isPrefetchRSCRequest') ?? false

// NOTE: Don't delete headers[RSC] yet, it still needs to be used in renderToHTML later

const isRSCRequest = getRequestMeta(req, 'isRSCRequest') ?? false

// when we are handling a middleware prefetch and it doesn't
// resolve to a static data route we bail early to avoid
Expand Down Expand Up @@ -2000,9 +2002,6 @@ export default abstract class Server<
)
}

// Don't delete headers[RSC] yet, it still needs to be used in renderToHTML later
const isRSCRequest = isRSCRequestCheck(req)

const { routeModule } = components

/**
Expand Down Expand Up @@ -3648,7 +3647,3 @@ export default abstract class Server<
return this.renderError(null, req, res, pathname!, query, setHeaders)
}
}

export function isRSCRequestCheck(req: BaseNextRequest): boolean {
return getRequestMeta(req, 'isRSCRequest') === true
}
7 changes: 5 additions & 2 deletions packages/next/src/server/next-server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ import type {
NextEnabledDirectories,
BaseRequestHandler,
} from './base-server'
import BaseServer, { NoFallbackError, isRSCRequestCheck } from './base-server'
import BaseServer, { NoFallbackError } from './base-server'
import { getMaybePagePath, getPagePath, requireFontManifest } from './require'
import { denormalizePagePath } from '../shared/lib/page-path/denormalize-page-path'
import { normalizePagePath } from '../shared/lib/page-path/normalize-page-path'
Expand Down Expand Up @@ -1130,7 +1130,10 @@ export default class NextNodeServer extends BaseServer<

if (!routeMatch || isMiddlewareRequest) return

const isRSC = isRSCRequestCheck(normalizedReq)
// NOTE: this is only attached after handle has started, this runs
// after the response has been sent, so it should have it set.
const isRSC = getRequestMeta(normalizedReq, 'isRSCRequest')

const reqEnd = Date.now()
const fetchMetrics = normalizedReq.fetchMetrics || []
const reqDuration = reqEnd - reqStart
Expand Down
Loading