Skip to content
This repository has been archived by the owner on Apr 6, 2023. It is now read-only.

perf(vite): remove duplicate css links from rendered page when inlined #7264

Merged
merged 2 commits into from
Sep 7, 2022
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions packages/vite/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import type { OutputOptions } from 'rollup'
import { cacheDirPlugin } from './plugins/cache-dir'
import { wpfs } from './utils/wpfs'
import type { ViteBuildContext, ViteOptions } from './vite'
import { writeManifest } from './manifest'
import { devStyleSSRPlugin } from './plugins/dev-ssr-css'
import { viteNodePlugin } from './vite-node'

Expand Down Expand Up @@ -140,6 +139,4 @@ export async function buildClient (ctx: ViteBuildContext) {
await ctx.nuxt.callHook('build:resources', wpfs)
logger.info(`Client built in ${Date.now() - start}ms`)
}

await writeManifest(ctx)
}
40 changes: 29 additions & 11 deletions packages/vite/src/plugins/ssr-styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,12 @@ import { isCSS } from '../utils'

interface SSRStylePluginOptions {
srcDir: string
chunksWithInlinedCSS: Set<string>
shouldInline?: (id?: string) => boolean
}

export function ssrStylesPlugin (options: SSRStylePluginOptions): Plugin {
const cssMap: Record<string, string[]> = {}
const cssMap: Record<string, { files: string[], inBundle: boolean }> = {}
const idRefMap: Record<string, string> = {}
const globalStyles = new Set<string>()

Expand All @@ -24,7 +25,9 @@ export function ssrStylesPlugin (options: SSRStylePluginOptions): Plugin {
generateBundle (outputOptions) {
const emitted: Record<string, string> = {}
for (const file in cssMap) {
if (!cssMap[file].length) { continue }
const { files, inBundle } = cssMap[file]
// File has been tree-shaken out of build (or there are no styles to inline)
if (!files.length || !inBundle) { continue }

const base = typeof outputOptions.assetFileNames === 'string'
? outputOptions.assetFileNames
Expand All @@ -38,14 +41,19 @@ export function ssrStylesPlugin (options: SSRStylePluginOptions): Plugin {
type: 'asset',
name: `${filename(file)}-styles.mjs`,
source: [
...cssMap[file].map((css, i) => `import style_${i} from './${relative(dirname(base), this.getFileName(css))}';`),
`export default [${cssMap[file].map((_, i) => `style_${i}`).join(', ')}]`
...files.map((css, i) => `import style_${i} from './${relative(dirname(base), this.getFileName(css))}';`),
`export default [${files.map((_, i) => `style_${i}`).join(', ')}]`
].join('\n')
})
}

const globalStylesArray = Array.from(globalStyles).map(css => idRefMap[css] && this.getFileName(idRefMap[css])).filter(Boolean)

for (const key in emitted) {
// Track the chunks we are inlining CSS for so we can omit including links to the .css files
options.chunksWithInlinedCSS.add(key)
}

this.emitFile({
type: 'asset',
fileName: 'styles.mjs',
Expand All @@ -61,11 +69,21 @@ export function ssrStylesPlugin (options: SSRStylePluginOptions): Plugin {
})
},
renderChunk (_code, chunk) {
if (!chunk.isEntry) { return null }
// Entry
for (const mod in chunk.modules) {
if (isCSS(mod) && !mod.includes('&used')) {
globalStyles.add(relativeToSrcDir(mod))
if (!chunk.facadeModuleId) { return null }
const id = relativeToSrcDir(chunk.facadeModuleId)
for (const file in chunk.modules) {
const relativePath = relativeToSrcDir(file)
if (relativePath in cssMap) {
cssMap[relativePath].inBundle = cssMap[relativePath].inBundle ?? !!id
}
}

if (chunk.isEntry) {
// Entry
for (const mod in chunk.modules) {
if (isCSS(mod) && !mod.includes('&used')) {
globalStyles.add(relativeToSrcDir(mod))
}
}
}
return null
Expand All @@ -77,7 +95,7 @@ export function ssrStylesPlugin (options: SSRStylePluginOptions): Plugin {
if (options.shouldInline && !options.shouldInline(id)) { return }

const relativeId = relativeToSrcDir(id)
cssMap[relativeId] = cssMap[relativeId] || []
cssMap[relativeId] = cssMap[relativeId] || { files: [] }

let styleCtr = 0
for (const i of findStaticImports(code)) {
Expand All @@ -94,7 +112,7 @@ export function ssrStylesPlugin (options: SSRStylePluginOptions): Plugin {
})

idRefMap[relativeToSrcDir(resolved.id)] = ref
cssMap[relativeId].push(ref)
cssMap[relativeId].files.push(ref)
}
}
}
Expand Down
29 changes: 29 additions & 0 deletions packages/vite/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { wpfs } from './utils/wpfs'
import { cacheDirPlugin } from './plugins/cache-dir'
import { initViteNodeServer } from './vite-node'
import { ssrStylesPlugin } from './plugins/ssr-styles'
import { writeManifest } from './manifest'

export async function buildServer (ctx: ViteBuildContext) {
const useAsyncEntry = ctx.nuxt.options.experimental.asyncEntry ||
Expand Down Expand Up @@ -113,12 +114,35 @@ export async function buildServer (ctx: ViteBuildContext) {
} as ViteOptions)

if (ctx.nuxt.options.experimental.inlineSSRStyles) {
const chunksWithInlinedCSS = new Set<string>()
serverConfig.plugins!.push(ssrStylesPlugin({
srcDir: ctx.nuxt.options.srcDir,
chunksWithInlinedCSS,
shouldInline: typeof ctx.nuxt.options.experimental.inlineSSRStyles === 'function'
? ctx.nuxt.options.experimental.inlineSSRStyles
: undefined
}))

// Remove CSS entries for files that will have inlined styles
ctx.nuxt.hook('build:manifest', (manifest) => {
for (const key in manifest) {
const entry = manifest[key]
const shouldRemoveCSS = chunksWithInlinedCSS.has(key)
Copy link
Member

Choose a reason for hiding this comment

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

What will happen for a component with ssr inline syle enabled when rendering in hybrid/spa mode?

Copy link
Member Author

Choose a reason for hiding this comment

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

Rendering in hybrid/spa mode would not be adding a <link> for the CSS that is specific to the page anyway (as ssrContext.modules would be empty). So there would be no change from these lines (L127-133).

However, there would be a change to the entry CSS, which would be prefetched and then loaded on client-side rather than preloaded. (This would have a negative performance impact.)

-   <link rel="preload" as="style" href="/_nuxt/entry.6e3e3577.css">
-   <link rel="stylesheet" href="/_nuxt/entry.6e3e3577.css">
+   <link rel="prefetch stylesheet" href="/_nuxt/entry.6e3e3577.css">

Note that this also has a negative impact for client-only components for SSR pages. We could disable L135-R143, and though we would have double-loaded entry CSS values, we would not have any negative perf impact for client-side/or hybrid spa:

https://github.com/nuxt/framework/pull/7264/files#diff-9659ec6cc10298d2e39de216bea87573d29efdb58b9fd2e8fc9fc4404719bcaaR135-R143

if (shouldRemoveCSS) {
entry.css = []
}
// Add entry CSS as prefetch (non-blocking)
if (entry.isEntry) {
manifest[key + '-css'] = {
file: '',
css: entry.css
}
entry.css = []
entry.dynamicImports = entry.dynamicImports || []
entry.dynamicImports.push(key + '-css')
}
}
})
}

// Add type-checking
Expand All @@ -140,6 +164,8 @@ export async function buildServer (ctx: ViteBuildContext) {
const start = Date.now()
logger.info('Building server...')
await vite.build(serverConfig)
// Write production client manifest
await writeManifest(ctx)
await onBuild()
logger.success(`Server built in ${Date.now() - start}ms`)
return
Expand All @@ -150,6 +176,9 @@ export async function buildServer (ctx: ViteBuildContext) {
return
}

// Write dev client manifest
await writeManifest(ctx)

// Start development server
const viteServer = await vite.createServer(serverConfig)
ctx.ssrServer = viteServer
Expand Down
21 changes: 18 additions & 3 deletions test/basic.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { fileURLToPath } from 'node:url'
import { describe, expect, it } from 'vitest'
import { joinURL } from 'ufo'
// import { isWindows } from 'std-env'
import { setup, fetch, $fetch, startServer } from '@nuxt/test-utils'
import { setup, fetch, $fetch, startServer, createPage } from '@nuxt/test-utils'
// eslint-disable-next-line import/order
import { expectNoClientErrors, renderPage } from './utils'

Expand Down Expand Up @@ -384,8 +384,23 @@ if (!process.env.NUXT_TEST_DEV && !process.env.TEST_WITH_WEBPACK) {
expect(html).toContain(style)
}
})
it.todo('does not render style hints for inlined styles')
it.todo('renders client-only styles?', async () => {

it('only renders prefetch for entry styles', async () => {
const html: string = await $fetch('/styles')
expect(html.match(/<link [^>]*href="[^"]*\.css">/)?.map(m => m.replace(/\.[^.]*\.css/, '.css'))).toMatchInlineSnapshot(`
[
"<link rel=\\"prefetch stylesheet\\" href=\\"/_nuxt/entry.css\\">",
]
`)
})

it('still downloads client-only styles', async () => {
const page = await createPage('/styles')
await page.waitForLoadState('networkidle')
expect(await page.$eval('.client-only-css', e => getComputedStyle(e).color)).toBe('rgb(50, 50, 50)')
})

it.todo('renders client-only styles only', async () => {
const html = await $fetch('/styles')
expect(html).toContain('{--client-only:"client-only"}')
})
Expand Down
10 changes: 9 additions & 1 deletion test/fixtures/basic/components/ClientOnlyScript.client.vue
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ export default defineNuxtComponent({

<template>
<div>
<div>client only script component {{ foo }}</div>
<div class="client-only-css">
client only script component {{ foo }}
</div>
<slot name="test" />
</div>
</template>
Expand All @@ -21,3 +23,9 @@ export default defineNuxtComponent({
--client-only: 'client-only';
}
</style>

<style scoped>
.client-only-css {
color: rgb(50, 50, 50);
}
</style>
1 change: 1 addition & 0 deletions test/fixtures/basic/nuxt.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export default defineNuxtConfig({
}
},
experimental: {
inlineSSRStyles: id => !id.includes('assets.vue'),
reactivityTransform: true,
treeshakeClientOnly: true
},
Expand Down