Skip to content

Commit

Permalink
fix: disallow onRenderHtml() to return null/undefined
Browse files Browse the repository at this point in the history
  • Loading branch information
brillout committed Sep 4, 2024
1 parent 8c7c0f4 commit 57a2eba
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 22 deletions.
8 changes: 2 additions & 6 deletions vike/node/runtime/renderPage/createHttpResponse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ type ContentType = HttpResponse['contentType']
type ResponseHeaders = HttpResponse['headers']

async function createHttpResponse(
htmlRender: null | HtmlRender,
htmlRender: HtmlRender,
renderHook: null | RenderHook,
pageContext: {
_pageId: null | string
Expand All @@ -41,11 +41,7 @@ async function createHttpResponse(
_pageConfigs: PageConfigRuntime[]
abortStatusCode?: AbortStatusCode
}
): Promise<HttpResponse | null> {
if (htmlRender === null) {
return null
}

): Promise<HttpResponse> {
let statusCode: StatusCode | undefined = pageContext.abortStatusCode
if (!statusCode) {
const isError = !pageContext._pageId || isErrorPage(pageContext._pageId, pageContext._pageConfigs)
Expand Down
14 changes: 7 additions & 7 deletions vike/node/runtime/renderPage/executeOnRenderHtmlHook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ async function executeOnRenderHtmlHook(
}
): Promise<{
renderHook: RenderHook
htmlRender: null | HtmlRender
htmlRender: HtmlRender
}> {
const { renderHook, hookFn } = getRenderHook(pageContext)
objectAssign(pageContext, { _renderHook: renderHook })
Expand All @@ -62,10 +62,6 @@ async function executeOnRenderHtmlHook(
Object.assign(pageContext, pageContextProvidedByRenderHook)
objectAssign(pageContext, { _pageContextPromise: pageContextPromise })

if (documentHtml === null || documentHtml === undefined) {
return { htmlRender: null, renderHook }
}

const onErrorWhileStreaming = (err: unknown) => {
// Should the stream inject the following?
// ```
Expand Down Expand Up @@ -131,7 +127,7 @@ function getRenderHook(pageContext: PageContextForUserConsumptionServerSide) {
}

function processHookReturnValue(hookReturnValue: unknown, renderHook: RenderHook) {
let documentHtml: null | DocumentHtml = null
let documentHtml: DocumentHtml
let pageContextPromise: PageContextPromise = null
let pageContextProvidedByRenderHook: null | Record<string, unknown> = null
let injectFilter: PreloadFilter = null
Expand Down Expand Up @@ -179,7 +175,11 @@ function processHookReturnValue(hookReturnValue: unknown, renderHook: RenderHook
injectFilter = hookReturnValue.injectFilter
}

if (hookReturnValue.documentHtml) {
assertUsage(
hookReturnValue.documentHtml,
`${errPrefix} returned an object that is missing the ${pc.code('documentHtml')} property.`
)
{
let val = hookReturnValue.documentHtml
const errBegin = `${errPrefix} returned ${pc.cyan('{ documentHtml }')}, but ${pc.cyan('documentHtml')}` as const
if (typeof val === 'string') {
Expand Down
13 changes: 4 additions & 9 deletions vike/node/runtime/renderPage/renderPageAlreadyRouted.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,15 +85,10 @@ async function renderPageAlreadyRouted<

const renderHookResult = await executeOnRenderHtmlHook(pageContext)

if (renderHookResult.htmlRender === null) {
objectAssign(pageContext, { httpResponse: null })
return pageContext
} else {
const { htmlRender, renderHook } = renderHookResult
const httpResponse = await createHttpResponse(htmlRender, renderHook, pageContext)
objectAssign(pageContext, { httpResponse })
return pageContext
}
const { htmlRender, renderHook } = renderHookResult
const httpResponse = await createHttpResponse(htmlRender, renderHook, pageContext)
objectAssign(pageContext, { httpResponse })
return pageContext
}

async function prerenderPage(
Expand Down

0 comments on commit 57a2eba

Please sign in to comment.