Skip to content

Commit

Permalink
fix: don't duplicate styles with dynamic import (fix #9967) (#9970)
Browse files Browse the repository at this point in the history
  • Loading branch information
bgoscinski committed Sep 23, 2022
1 parent 533d13c commit 65f97bd
Show file tree
Hide file tree
Showing 10 changed files with 179 additions and 3 deletions.
20 changes: 18 additions & 2 deletions packages/vite/src/node/plugins/importAnalysisBuild.ts
Expand Up @@ -65,6 +65,8 @@ function preload(
return baseModule()
}

const links = document.getElementsByTagName('link')

return Promise.all(
deps.map((dep) => {
// @ts-ignore
Expand All @@ -75,10 +77,24 @@ function preload(
seen[dep] = true
const isCss = dep.endsWith('.css')
const cssSelector = isCss ? '[rel="stylesheet"]' : ''
// @ts-ignore check if the file is already preloaded by SSR markup
if (document.querySelector(`link[href="${dep}"]${cssSelector}`)) {
const isBaseRelative = !!importerUrl

// check if the file is already preloaded by SSR markup
if (isBaseRelative) {
// When isBaseRelative is true then we have `importerUrl` and `dep` is
// already converted to an absolute URL by the `assetsURL` function
for (let i = links.length - 1; i >= 0; i--) {
const link = links[i]
// The `links[i].href` is an absolute URL thanks to browser doing the work
// for us. See https://html.spec.whatwg.org/multipage/common-dom-interfaces.html#reflecting-content-attributes-in-idl-attributes:idl-domstring-5
if (link.href === dep && (!isCss || link.rel === 'stylesheet')) {
return
}
}
} else if (document.querySelector(`link[href="${dep}"]${cssSelector}`)) {
return
}

// @ts-ignore
const link = document.createElement('link')
// @ts-ignore
Expand Down
121 changes: 121 additions & 0 deletions playground/css-dynamic-import/__tests__/css-dynamic-import.spec.ts
@@ -0,0 +1,121 @@
import type { InlineConfig } from 'vite'
import { build, createServer, preview } from 'vite'
import { expect, test } from 'vitest'
import { getColor, isBuild, isServe, page, ports, rootDir } from '~utils'

const baseOptions = [
{ base: '', label: 'relative' },
{ base: '/', label: 'absolute' }
]

const getConfig = (base: string): InlineConfig => ({
base,
root: rootDir,
logLevel: 'silent',
preview: { port: ports['css/dynamic-import'] },
build: { assetsInlineLimit: 0 }
})

async function withBuild(base: string, fn: () => Promise<void>) {
const config = getConfig(base)
await build(config)
const server = await preview(config)

try {
await page.goto(server.resolvedUrls.local[0])
await fn()
} finally {
server.httpServer.close()
}
}

async function withServe(base: string, fn: () => Promise<void>) {
const config = getConfig(base)
const server = await createServer(config)
await server.listen()
await new Promise((r) => setTimeout(r, 500))

try {
await page.goto(server.resolvedUrls.local[0])
await fn()
} finally {
await server.close()
}
}

async function getLinks() {
const links = await page.$$('link')
return await Promise.all(
links.map((handle) => {
return handle.evaluate((link) => ({
pathname: new URL(link.href).pathname,
rel: link.rel,
as: link.as
}))
})
)
}

baseOptions.forEach(({ base, label }) => {
test.runIf(isBuild)(
`doesn't duplicate dynamically imported css files when built with ${label} base`,
async () => {
await withBuild(base, async () => {
await page.waitForSelector('.loaded', { state: 'attached' })

expect(await getColor('.css-dynamic-import')).toBe('green')
expect(await getLinks()).toEqual([
{
pathname: expect.stringMatching(/^\/assets\/index\..+\.css$/),
rel: 'stylesheet',
as: ''
},
{
pathname: expect.stringMatching(/^\/assets\/dynamic\..+\.css$/),
rel: 'preload',
as: 'style'
},
{
pathname: expect.stringMatching(/^\/assets\/dynamic\..+\.js$/),
rel: 'modulepreload',
as: 'script'
},
{
pathname: expect.stringMatching(/^\/assets\/dynamic\..+\.css$/),
rel: 'stylesheet',
as: ''
},
{
pathname: expect.stringMatching(/^\/assets\/static\..+\.js$/),
rel: 'modulepreload',
as: 'script'
},
{
pathname: expect.stringMatching(/^\/assets\/index\..+\.js$/),
rel: 'modulepreload',
as: 'script'
}
])
})
}
)

test.runIf(isServe)(
`doesn't duplicate dynamically imported css files when served with ${label} base`,
async () => {
await withServe(base, async () => {
await page.waitForSelector('.loaded', { state: 'attached' })

expect(await getColor('.css-dynamic-import')).toBe('green')
// in serve there is no preloading
expect(await getLinks()).toEqual([
{
pathname: '/dynamic.css',
rel: 'preload',
as: 'style'
}
])
})
}
)
})
10 changes: 10 additions & 0 deletions playground/css-dynamic-import/__tests__/serve.ts
@@ -0,0 +1,10 @@
// this is automatically detected by playground/vitestSetup.ts and will replace
// the default e2e test serve behavior

// The server is started in the test, so we need to have a custom serve
// function or a default server will be created
export async function serve() {
return {
close: () => Promise.resolve()
}
}
3 changes: 3 additions & 0 deletions playground/css-dynamic-import/dynamic.css
@@ -0,0 +1,3 @@
.css-dynamic-import {
color: green;
}
6 changes: 6 additions & 0 deletions playground/css-dynamic-import/dynamic.js
@@ -0,0 +1,6 @@
import './dynamic.css'

export const lazyLoad = async () => {
await import('./static.js')
document.body.classList.add('loaded')
}
3 changes: 3 additions & 0 deletions playground/css-dynamic-import/index.html
@@ -0,0 +1,3 @@
<p class="css-dynamic-import">This should be green</p>

<script type="module" src="./index.js"></script>
10 changes: 10 additions & 0 deletions playground/css-dynamic-import/index.js
@@ -0,0 +1,10 @@
import './static.js'

const link = document.head.appendChild(document.createElement('link'))
link.rel = 'preload'
link.as = 'style'
link.href = new URL('./dynamic.css', import.meta.url).href

import('./dynamic.js').then(async ({ lazyLoad }) => {
await lazyLoad()
})
3 changes: 3 additions & 0 deletions playground/css-dynamic-import/static.css
@@ -0,0 +1,3 @@
.css-dynamic-import {
color: red;
}
3 changes: 3 additions & 0 deletions playground/css-dynamic-import/static.js
@@ -0,0 +1,3 @@
import './static.css'

export const foo = 'foo'
3 changes: 2 additions & 1 deletion playground/test-utils.ts
Expand Up @@ -29,7 +29,8 @@ export const ports = {
'ssr-vue': 9604,
'ssr-webworker': 9605,
'css/postcss-caching': 5005,
'css/postcss-plugins-different-dir': 5006
'css/postcss-plugins-different-dir': 5006,
'css/dynamic-import': 5007
}
export const hmrPorts = {
'optimize-missing-deps': 24680,
Expand Down

0 comments on commit 65f97bd

Please sign in to comment.