-
-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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,schema): allow showing spa loader til after hydration #29776
Changes from all commits
344940a
70cbf80
096b6d6
aed5212
ffe807b
b4a279d
872da31
45a5a54
d2491a0
86db10e
84d1bcb
e1c89be
aa830c5
07cd634
020f212
4b5b8ce
6ca7600
df1d397
ef927bb
24a0253
f4106a7
c5f3d53
86b6be7
29f0663
a7726ad
7ac69b6
0651291
2da2115
db27a07
2f778d2
fcf076d
967c861
b9ae10c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -451,3 +451,24 @@ In this case, the component name would be `MyComponent`, as far as Vue is concer | |
But in order to auto-import it, you would need to use `SomeFolderMyComponent`. | ||
|
||
By setting `experimental.normalizeComponentNames`, these two values match, and Vue will generate a component name that matches the Nuxt pattern for component naming. | ||
|
||
## spaLoadingTemplateLocation | ||
|
||
When rendering a client-only page (with `ssr: false`), we optionally render a loading screen (from `app/spa-loading-template.html`). | ||
|
||
It can be set to `within`, which will render it like this: | ||
|
||
```html | ||
<div id="__nuxt"> | ||
<!-- spa loading template --> | ||
</div> | ||
``` | ||
|
||
Alternatively, you can render the template alongside the Nuxt app root by setting it to `body`: | ||
|
||
```html | ||
<div id="__nuxt"></div> | ||
<!-- spa loading template --> | ||
``` | ||
|
||
This avoids a white flash when hydrating a client-only page. | ||
Comment on lines
+454
to
+474
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π οΈ Refactor suggestion Documentation needs configuration example and related options. While the explanation and HTML examples are clear and helpful, please enhance the documentation by:
export default defineNuxtConfig({
experimental: {
spaLoadingTemplateLocation: 'body' // or 'within'
}
})
This will provide a complete reference for users implementing this feature. |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
<script setup lang="ts"> | ||
await useAsyncData(async () => { | ||
await new Promise((r) => { setTimeout(r, 50) }) | ||
return 42 | ||
}) | ||
</script> | ||
|
||
<template> | ||
<div data-testid="content"> | ||
app content | ||
</div> | ||
</template> | ||
|
||
<style scoped> | ||
|
||
</style> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
<div data-testid="loader">loading...</div> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
export default defineNuxtConfig({ | ||
devtools: { enabled: false }, | ||
spaLoadingTemplate: true, | ||
routeRules: { | ||
'/spa': { ssr: false }, | ||
'/ssr': { ssr: true }, | ||
}, | ||
experimental: { | ||
spaLoadingTemplateLocation: 'within', | ||
}, | ||
compatibilityDate: '2024-06-28', | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
{ | ||
"name": "fixture-spa-loader", | ||
"private": true, | ||
"scripts": { | ||
"dev": "nuxi dev", | ||
"build": "nuxi build", | ||
"start": "nuxi preview" | ||
}, | ||
"dependencies": { | ||
"nuxt": "workspace:*" | ||
}, | ||
"engines": { | ||
"node": "^18.12.0 || ^20.9.0 || >=22.0.0" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
{ | ||
"extends": "./.nuxt/tsconfig.json" | ||
} |
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know how to fix it. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
import { fileURLToPath } from 'node:url' | ||
import { describe, expect, it } from 'vitest' | ||
import { isWindows } from 'std-env' | ||
import { $fetch, getBrowser, setup, url } from '@nuxt/test-utils' | ||
|
||
const isWebpack = | ||
process.env.TEST_BUILDER === 'webpack' || | ||
process.env.TEST_BUILDER === 'rspack' | ||
|
||
const isDev = process.env.TEST_ENV === 'dev' | ||
|
||
await setup({ | ||
rootDir: fileURLToPath(new URL('../fixtures/spa-loader', import.meta.url)), | ||
dev: isDev, | ||
server: true, | ||
browser: true, | ||
setupTimeout: (isWindows ? 360 : 120) * 1000, | ||
nuxtConfig: { | ||
builder: isWebpack ? 'webpack' : 'vite', | ||
spaLoadingTemplate: true, | ||
experimental: { | ||
spaLoadingTemplateLocation: 'within', | ||
}, | ||
}, | ||
}) | ||
|
||
describe('spaLoadingTemplateLocation flag is set to `within`', () => { | ||
it('shoul be render loader inside appTag', async () => { | ||
const html = await $fetch<string>('/spa') | ||
expect(html.replace(/[\n\r]+/g, '')).toContain( | ||
`<div id="__nuxt"><div data-testid="loader">loading...</div></div>`, | ||
) | ||
}) | ||
|
||
it.skipIf(isDev)('spa-loader does not appear while the app is mounting', async () => { | ||
const browser = await getBrowser() | ||
const page = await browser.newPage({}) | ||
await page.goto(url('/spa'), { waitUntil: 'domcontentloaded' }) | ||
|
||
const loader = page.getByTestId('loader') | ||
expect(await loader.isHidden()).toBeTruthy() | ||
|
||
await page.close() | ||
}, 60_000) | ||
}) |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,52 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { fileURLToPath } from 'node:url' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { describe, expect, it } from 'vitest' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { isWindows } from 'std-env' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { getBrowser, setup, url } from '@nuxt/test-utils' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const isWebpack = process.env.TEST_BUILDER === 'webpack' || process.env.TEST_BUILDER === 'rspack' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
await setup({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
rootDir: fileURLToPath(new URL('../fixtures/spa-loader', import.meta.url)), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
dev: process.env.TEST_ENV === 'dev', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
server: true, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
browser: true, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
setupTimeout: (isWindows ? 360 : 120) * 1000, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
nuxtConfig: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
builder: isWebpack ? 'webpack' : 'vite', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
spaLoadingTemplate: true, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
experimental: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
spaLoadingTemplateLocation: 'body', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
describe('spaLoadingTemplateLocation flag is set to `body`', () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
it('should render spa-loader', async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const browser = await getBrowser() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const page = await browser.newPage({}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
await page.goto(url('/spa'), { waitUntil: 'domcontentloaded' }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const loader = page.getByTestId('loader') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π οΈ Refactor suggestion Standardize test IDs across test cases. The test uses inconsistent test IDs: 'loader' vs '__nuxt-spa-loader'. Extract test IDs into constants: const TEST_IDS = {
LOADER: '__nuxt-spa-loader',
CONTENT: 'content'
} as const Then use these constants consistently throughout the tests. Also applies to: 43-43 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
expect(await loader.isVisible()).toBeTruthy() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const content = page.getByTestId('content') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
await content.waitFor({ state: 'visible' }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
expect(await loader.isHidden()).toBeTruthy() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
await page.close() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, 60_000) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+24
to
+36
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π οΈ Refactor suggestion Add error handling and cleanup for browser operations. The browser page navigation and operations should include proper error handling and cleanup. Consider using a try-finally block: it('should render spa-loader', async () => {
const browser = await getBrowser()
const page = await browser.newPage({})
- await page.goto(url('/spa'), { waitUntil: 'domcontentloaded' })
- const loader = page.getByTestId('loader')
- expect(await loader.isVisible()).toBeTruthy()
+ try {
+ await page.goto(url('/spa'), { waitUntil: 'domcontentloaded' })
+ const loader = page.getByTestId('loader')
+ expect(await loader.isVisible()).toBeTruthy()
- const content = page.getByTestId('content')
- await content.waitFor({ state: 'visible' })
- expect(await loader.isHidden()).toBeTruthy()
+ const content = page.getByTestId('content')
+ await content.waitFor({ state: 'visible' })
+ expect(await loader.isHidden()).toBeTruthy()
+ } finally {
+ await page.close()
+ }
- await page.close()
}, 60_000) π Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
it('should render content without spa-loader', async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const browser = await getBrowser() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const page = await browser.newPage({}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
await page.goto(url('/ssr'), { waitUntil: 'domcontentloaded' }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const loader = page.getByTestId('loader') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
expect(await loader.isHidden()).toBeTruthy() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const content = page.getByTestId('content') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
await content.waitFor({ state: 'visible' }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
expect(await loader.isHidden()).toBeTruthy() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. seems this test is failing. maybe we need a slightly different assertion? or modify the implementation so both are not present on the page at the same moment? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a test for the /ssr page. We should not see the SPA loader at any time:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm... the first test is broken... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
await page.close() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, 60_000) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
π οΈ Refactor suggestion
Documentation should cover the complete feature set.
The current documentation focuses on template location but should also explain:
suspense:resolve
app.spaLoaderTag
andapp.spaLoaderAttrs
experimental.spaPreloaderOutside
This aligns with the PR objectives and provides users with comprehensive documentation of all available options.