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.

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

fix(nuxt): return RenderResponse for redirects #20496

Merged
merged 7 commits into from
Apr 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 1 addition & 2 deletions packages/nuxt/src/app/composables/cookie.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const CookieDefaults: CookieOptions<any> = {
encode: val => encodeURIComponent(typeof val === 'string' ? val : JSON.stringify(val))
}

export function useCookie <T = string | null | undefined> (name: string, _opts?: CookieOptions<T>): CookieRef<T> {
export function useCookie<T = string | null | undefined> (name: string, _opts?: CookieOptions<T>): CookieRef<T> {
const opts = { ...CookieDefaults, ..._opts }
const cookies = readRawCookies(opts) || {}

Expand All @@ -53,7 +53,6 @@ export function useCookie <T = string | null | undefined> (name: string, _opts?:
return writeFinalCookieValue()
}
nuxtApp.hooks.hookOnce('app:error', writeAndUnhook)
nuxtApp.hooks.hookOnce('app:redirected', writeAndUnhook)
}

return cookie as CookieRef<T>
Expand Down
29 changes: 19 additions & 10 deletions packages/nuxt/src/app/composables/router.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { getCurrentInstance, inject, onUnmounted } from 'vue'
import type { Ref } from 'vue'
import type { NavigationFailure, NavigationGuard, RouteLocationNormalized, RouteLocationNormalizedLoaded, RouteLocationPathRaw, RouteLocationRaw, Router } from 'vue-router'
import { sendRedirect } from 'h3'
import { sanitizeStatusCode } from 'h3'
import { hasProtocol, joinURL, parseURL } from 'ufo'

import { useNuxtApp, useRuntimeConfig } from '../nuxt'
Expand Down Expand Up @@ -86,7 +86,7 @@ export interface NavigateToOptions {
external?: boolean
}

export const navigateTo = (to: RouteLocationRaw | undefined | null, options?: NavigateToOptions): Promise<void | NavigationFailure | false> | RouteLocationRaw => {
export const navigateTo = (to: RouteLocationRaw | undefined | null, options?: NavigateToOptions): Promise<void | NavigationFailure | false> | false | void | RouteLocationRaw => {
if (!to) {
to = '/'
}
Expand All @@ -111,17 +111,26 @@ export const navigateTo = (to: RouteLocationRaw | undefined | null, options?: Na

if (process.server) {
const nuxtApp = useNuxtApp()
if (nuxtApp.ssrContext && nuxtApp.ssrContext.event) {
if (nuxtApp.ssrContext) {
const fullPath = typeof to === 'string' || isExternal ? toPath : router.resolve(to).fullPath || '/'
const redirectLocation = isExternal ? toPath : joinURL(useRuntimeConfig().app.baseURL, fullPath)
const redirect = () => nuxtApp.callHook('app:redirected')
.then(() => sendRedirect(nuxtApp.ssrContext!.event, redirectLocation, options?.redirectCode || 302))
.then(() => inMiddleware ? /* abort route navigation */ false : undefined)
const location = isExternal ? toPath : joinURL(useRuntimeConfig().app.baseURL, fullPath)

async function redirect () {
// TODO: consider deprecating in favour of `app:rendered` and removing
await nuxtApp.callHook('app:redirected')
const encodedLoc = location.replace(/"/g, '%22')
nuxtApp.ssrContext!._renderResponse = {
statusCode: sanitizeStatusCode(options?.redirectCode || 302, 302),
pi0 marked this conversation as resolved.
Show resolved Hide resolved
body: `<!DOCTYPE html><html><head><meta http-equiv="refresh" content="0; url=${encodedLoc}"></head></html>`,
headers: { location }
}
return inMiddleware ? /* abort route navigation */ false : undefined
}

// We wait to perform the redirect in case any other middleware will intercept the redirect
// and redirect further.
// We wait to perform the redirect last in case any other middleware will intercept the redirect
// and redirect somewhere else instead.
if (!isExternal && inMiddleware) {
router.beforeEach(final => (final.fullPath === fullPath) ? redirect() : undefined)
router.afterEach(final => (final.fullPath === fullPath) ? redirect() : undefined)
return to
}
return redirect()
Expand Down
3 changes: 3 additions & 0 deletions packages/nuxt/src/app/nuxt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { getContext } from 'unctx'
import type { SSRContext } from 'vue-bundle-renderer/runtime'
import type { H3Event } from 'h3'
import type { AppConfig, AppConfigInput, RuntimeConfig } from 'nuxt/schema'
import type { RenderResponse } from 'nitropack'

// eslint-disable-next-line import/no-restricted-paths
import type { NuxtIslandContext } from '../core/runtime/nitro/renderer'
Expand Down Expand Up @@ -61,6 +62,8 @@ export interface NuxtSSRContext extends SSRContext {
renderMeta?: () => Promise<NuxtMeta> | NuxtMeta
islandContext?: NuxtIslandContext
/** @internal */
_renderResponse?: Partial<RenderResponse>
/** @internal */
_payloadReducers: Record<string, (data: any) => any>
}

Expand Down
5 changes: 1 addition & 4 deletions packages/nuxt/src/app/plugins/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import { callWithNuxt, defineNuxtPlugin, useRuntimeConfig } from '../nuxt'
import { clearError, showError } from '../composables/error'
import { navigateTo } from '../composables/router'
import { useState } from '../composables/state'
import { useRequestEvent } from '../composables/ssr'

// @ts-expect-error virtual file
import { globalMiddleware } from '#build/middleware'
Expand Down Expand Up @@ -258,9 +257,7 @@ export default defineNuxtPlugin<{ route: Route, router: Router }>({

await router.replace(initialURL)
if (!isEqual(route.fullPath, initialURL)) {
const event = await callWithNuxt(nuxtApp, useRequestEvent)
const options = { redirectCode: event.node.res.statusCode !== 200 ? event.node.res.statusCode || 302 : 302 }
await callWithNuxt(nuxtApp, navigateTo, [route.fullPath, options])
await callWithNuxt(nuxtApp, navigateTo, [route.fullPath])
}
})

Expand Down
9 changes: 7 additions & 2 deletions packages/nuxt/src/core/runtime/nitro/renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ const ROOT_NODE_REGEX = new RegExp(`^<${appRootTag} id="${appRootId}">([\\s\\S]*

const PRERENDER_NO_SSR_ROUTES = new Set(['/index.html', '/200.html', '/404.html'])

export default defineRenderHandler(async (event) => {
export default defineRenderHandler(async (event): Promise<Partial<RenderResponse>> => {
const nitroApp = useNitroApp()

// Whether we're rendering an error page
Expand Down Expand Up @@ -252,7 +252,12 @@ export default defineRenderHandler(async (event) => {
})
await ssrContext.nuxt?.hooks.callHook('app:rendered', { ssrContext })

if (event.node.res.headersSent || event.node.res.writableEnded) { return }
if (ssrContext._renderResponse) { return ssrContext._renderResponse }

if (event.node.res.headersSent || event.node.res.writableEnded) {
// @ts-expect-error TODO: handle additional cases
return
}

// Handle errors
if (ssrContext.payload?.error && !ssrError) {
Expand Down
2 changes: 1 addition & 1 deletion test/bundle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ describe.skipIf(isWindows || process.env.TEST_BUILDER === 'webpack' || process.e

it('default client bundle size', async () => {
stats.client = await analyzeSizes('**/*.js', publicDir)
expect(roundToKilobytes(stats.client.totalBytes)).toMatchInlineSnapshot('"94.3k"')
expect(roundToKilobytes(stats.client.totalBytes)).toMatchInlineSnapshot('"94.2k"')
expect(stats.client.files.map(f => f.replace(/\..*\.js/, '.js'))).toMatchInlineSnapshot(`
[
"_nuxt/entry.js",
Expand Down
5 changes: 5 additions & 0 deletions test/fixtures/basic/pages/navigate-to-external.vue
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,10 @@
</template>

<script setup>
if (useRoute().path === '/navigate-to-external') {
useNuxtApp().hook('app:rendered', () => {
throw new Error('this should not run')
})
}
await navigateTo('https://example.com/', { external: true, replace: true })
</script>
4 changes: 4 additions & 0 deletions test/fixtures/basic/pages/navigate-to-redirect.vue
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,8 @@ definePageMeta({
return navigateTo({ path: '/' }, { redirectCode: 307 })
}
})
console.log('running setup')
useNuxtApp().hook('app:rendered', () => {
throw new Error('this should not run')
})
</script>
2 changes: 1 addition & 1 deletion test/fixtures/basic/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ describe('middleware', () => {
addRouteMiddleware('example', (to, from) => {
expectTypeOf(to).toEqualTypeOf<RouteLocationNormalizedLoaded>()
expectTypeOf(from).toEqualTypeOf<RouteLocationNormalizedLoaded>()
expectTypeOf(navigateTo).toEqualTypeOf<(to: RouteLocationRaw | null | undefined, options?: NavigateToOptions) => RouteLocationRaw | Promise<void | NavigationFailure | false>>()
expectTypeOf(navigateTo).toEqualTypeOf <(to: RouteLocationRaw | null | undefined, options ?: NavigateToOptions) => RouteLocationRaw | void | false | Promise<void | NavigationFailure | false>>()
navigateTo('/')
abortNavigation()
abortNavigation('error string')
Expand Down