Skip to content

Commit

Permalink
fix(vue-app): skip page render early on error or navigation (#20719)
Browse files Browse the repository at this point in the history
  • Loading branch information
rchl committed May 22, 2023
1 parent 5e9942b commit db0e155
Show file tree
Hide file tree
Showing 10 changed files with 202 additions and 12 deletions.
59 changes: 50 additions & 9 deletions packages/vue-app/template/client.js
Expand Up @@ -244,7 +244,7 @@ function resolveComponents (route) {
<% } %>

<% if (features.middleware) { %>
function callMiddleware (Components, context, layout) {
function callMiddleware (Components, context, layout, renderState) {
let midd = <%= devalue(router.middleware) %><%= isTest ? '// eslint-disable-line' : '' %>
let unknownMiddleware = false

Expand Down Expand Up @@ -278,7 +278,7 @@ function callMiddleware (Components, context, layout) {
if (unknownMiddleware) {
return
}
return middlewareSeries(midd, context)
return middlewareSeries(midd, context, renderState)
}
<% } else if (isDev) {
// This is a placeholder function mainly so we dont have to
Expand All @@ -288,7 +288,7 @@ function callMiddleware () {
return Promise.resolve(true)
}
<% } %>
async function render (to, from, next) {
async function render (to, from, next, renderState) {
if (this._routeChanged === false && this._paramChanged === false && this._queryChanged === false) {
return next()
}
Expand Down Expand Up @@ -329,6 +329,12 @@ async function render (to, from, next) {
await setContext(app, {
route: to,
from,
error: (err) => {
if (renderState.aborted) {
return
}
app.nuxt.error.call(this, err)
},
next: _next.bind(this)
})
this._dateLastError = app.nuxt.dateErr
Expand All @@ -342,10 +348,14 @@ async function render (to, from, next) {
if (!Components.length) {
<% if (features.middleware) { %>
// Default layout
await callMiddleware.call(this, Components, app.context)
await callMiddleware.call(this, Components, app.context, undefined, renderState)
if (nextCalled) {
return
}
if (renderState.aborted) {
next(false)
return
}
<% } %>

<% if (features.layouts) { %>
Expand All @@ -359,10 +369,14 @@ async function render (to, from, next) {
<% } %>

<% if (features.middleware) { %>
await callMiddleware.call(this, Components, app.context, layout)
await callMiddleware.call(this, Components, app.context, layout, renderState)
if (nextCalled) {
return
}
if (renderState.aborted) {
next(false)
return
}
<% } %>

// Show error page
Expand All @@ -387,10 +401,14 @@ async function render (to, from, next) {
try {
<% if (features.middleware) { %>
// Call middleware
await callMiddleware.call(this, Components, app.context)
await callMiddleware.call(this, Components, app.context, undefined, renderState)
if (nextCalled) {
return
}
if (renderState.aborted) {
next(false)
return
}
if (app.context._errored) {
return next()
}
Expand All @@ -407,10 +425,14 @@ async function render (to, from, next) {

<% if (features.middleware) { %>
// Call middleware for layout
await callMiddleware.call(this, Components, app.context, layout)
await callMiddleware.call(this, Components, app.context, layout, renderState)
if (nextCalled) {
return
}
if (renderState.aborted) {
next(false)
return
}
if (app.context._errored) {
return next()
}
Expand Down Expand Up @@ -578,10 +600,18 @@ async function render (to, from, next) {
this.$loading.finish()
}
<% } %>
if (renderState.aborted) {
next(false)
return
}
next()
}

} catch (err) {
if (renderState.aborted) {
next(false)
return
}
const error = err || {}
if (error.message === 'ERR_REDIRECT') {
return this.<%= globals.nuxt %>.$emit('routeChanged', to, from, error)
Expand Down Expand Up @@ -908,7 +938,17 @@ async function mountApp (__app) {

// Add beforeEach router hooks
router.beforeEach(loadAsyncComponents.bind(_app))
router.beforeEach(render.bind(_app))

// Each new invocation of render() aborts previous invocation
let renderState = null
const boundRender = render.bind(_app)
router.beforeEach((to, from, next) => {
if (renderState) {
renderState.aborted = true
}
renderState = { aborted: false }
boundRender(to, from, next, renderState)
})

// Fix in static: remove trailing slash to force hydration
// Full static, if server-rendered: hydrate, to allow custom redirect to generated page
Expand Down Expand Up @@ -954,5 +994,6 @@ async function mountApp (__app) {
errorHandler(err)
}
})
})
},
{ aborted: false })
}
10 changes: 7 additions & 3 deletions packages/vue-app/template/utils.js
Expand Up @@ -279,6 +279,10 @@ export async function setContext (app, context) {
app.context.from = fromRouteData
}

if (context.error) {
app.context.error = context.error
}

app.context.next = context.next
app.context._redirected = false
app.context._errored = false
Expand All @@ -287,13 +291,13 @@ export async function setContext (app, context) {
app.context.query = app.context.route.query || {}
}
<% if (features.middleware) { %>
export function middlewareSeries (promises, appContext) {
if (!promises.length || appContext._redirected || appContext._errored) {
export function middlewareSeries (promises, appContext, renderState) {
if (!promises.length || appContext._redirected || appContext._errored || (renderState && renderState.aborted)) {
return Promise.resolve()
}
return promisify(promises[0], appContext)
.then(() => {
return middlewareSeries(promises.slice(1), appContext)
return middlewareSeries(promises.slice(1), appContext, renderState)
})
}
<% } %>
Expand Down
68 changes: 68 additions & 0 deletions test/e2e/basic.navigation.test.js
@@ -0,0 +1,68 @@
import Browser from '../utils/browser'
import { loadFixture, getPort, Nuxt, waitFor } from '../utils'

let port
const browser = new Browser()
const url = route => 'http://localhost:' + port + route

let nuxt = null
let page = null

describe('basic browser navigation', () => {
beforeAll(async () => {
const config = await loadFixture('basic')
nuxt = new Nuxt(config)
await nuxt.ready()

port = await getPort()
await nuxt.server.listen(port, 'localhost')

await browser.start({
// slowMo: 50,
// headless: false
})
})

const TEST_PATHS = [
'asyncData-error',
'asyncData-throw',
'middleware-asyncData-error',
'middleware-asyncData-throw'
]

for (const path of TEST_PATHS) {
test(`Navigate away from /navigation/${path}`, async () => {
page = await browser.page(url('/'))
expect(await page.$text('h1')).toBe('Index page')

await page.nuxt.navigate('/navigation')
expect(await page.$text('h3')).toContain('Click one of the links below')

await page.nuxt.navigate(`/navigation/${path}`, /* waitEnd */false)
await waitFor(nuxt.options.loading.throttle + 100)
const loading = await page.nuxt.loadingData()
expect(loading.show).toBe(true)

await page.nuxt.go(-1)
await page.waitForFunction(
'$nuxt.$loading.$data.show === false'
)
expect(await page.$text('h1')).toBe('Index page')

await waitFor(2000)

expect(await page.$text('h1')).toBe('Index page')
})
}

// Close server and ask nuxt to stop listening to file changes
afterAll(async () => {
await nuxt.close()
})

// Stop browser
afterAll(async () => {
await page.close()
await browser.close()
})
})
3 changes: 3 additions & 0 deletions test/fixtures/basic/middleware/slow.js
@@ -0,0 +1,3 @@
export default async function () {
await new Promise(resolve => setTimeout(resolve, 2000))
}
3 changes: 3 additions & 0 deletions test/fixtures/basic/pages/index.vue
Expand Up @@ -2,5 +2,8 @@
<div>
<h1>Index page</h1>
<img src="~/assets/test.svg">
<nuxt-link to="navigation">
Continue to /navigation
</nuxt-link>
</div>
</template>
27 changes: 27 additions & 0 deletions test/fixtures/basic/pages/navigation.vue
@@ -0,0 +1,27 @@
<template>
<div>
<h3>Click one of the links below and navigate back in history before the page has fully loaded. Pass if no error page shown.</h3>
<ul>
<li v-for="test in tests" :key="test">
<nuxt-link :to="'navigation/' + test">
{{ test }}
</nuxt-link>
</li>
</ul>
</div>
</template>

<script>
export default {
computed: {
tests () {
return [
'asyncData-error',
'asyncData-throw',
'middleware-asyncData-error',
'middleware-asyncData-throw'
]
}
}
}
</script>
11 changes: 11 additions & 0 deletions test/fixtures/basic/pages/navigation/asyncData-error.vue
@@ -0,0 +1,11 @@
<template>
<h1>Async Data Error</h1>
</template>
<script>
export default {
async asyncData ({ error }) {
await new Promise(resolve => setTimeout(resolve, 2000))
error({ statusCode: 401, message: 'Not allowed here' })
}
}
</script>
11 changes: 11 additions & 0 deletions test/fixtures/basic/pages/navigation/asyncData-throw.vue
@@ -0,0 +1,11 @@
<template>
<h1>Async Throw Error</h1>
</template>
<script>
export default {
async asyncData () {
await new Promise(resolve => setTimeout(resolve, 2000))
throw new Error('crash')
}
}
</script>
@@ -0,0 +1,11 @@
<template>
<h1>Async Data Error</h1>
</template>
<script>
export default {
middleware: ['slow'],
asyncData ({ error, route }) {
error({ statusCode: 401, message: 'Not allowed here' })
}
}
</script>
@@ -0,0 +1,11 @@
<template>
<h1>Async Throw Error</h1>
</template>
<script>
export default {
middleware: ['slow'],
asyncData () {
throw new Error('crash')
}
}
</script>

0 comments on commit db0e155

Please sign in to comment.