Skip to content

Commit

Permalink
fix(css): file or contents missing error in build watch (vitejs#3742) (
Browse files Browse the repository at this point in the history
…vitejs#3747)

Co-authored-by: Shinigami <chrissi92@hotmail.de>
  • Loading branch information
2 people authored and aleclarson committed Jun 25, 2021
1 parent 558302d commit 6c72373
Show file tree
Hide file tree
Showing 7 changed files with 94 additions and 18 deletions.
16 changes: 15 additions & 1 deletion packages/playground/assets/__tests__/assets.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ import {
isBuild,
listAssets,
readManifest,
readFile
readFile,
editFile,
notifyRebuildComplete
} from '../../testUtils'

const assetMatch = isBuild
Expand Down Expand Up @@ -195,3 +197,15 @@ if (isBuild) {
}
})
}
describe('css and assets in css in build watch', () => {
if (isBuild) {
test('css will not be lost and css does not contain undefined', async () => {
editFile('index.html', (code) => code.replace('Assets', 'assets'), true)
await notifyRebuildComplete(watcher)
const cssFile = findAssetFile(/index\.\w+\.css$/, 'foo')
expect(cssFile).not.toBe('')
expect(cssFile).not.toMatch(/undefined/)
watcher?.close()
})
}
})
3 changes: 2 additions & 1 deletion packages/playground/assets/vite.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ module.exports = {
},
build: {
outDir: 'dist/foo',
manifest: true
manifest: true,
watch: {}
}
}
2 changes: 2 additions & 0 deletions packages/playground/testEnv.d.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Page } from 'playwright-chromium'
import { RollupWatcher } from 'rollup'

declare global {
// injected by the custom jest env in scripts/jestEnv.js
Expand All @@ -7,4 +8,5 @@ declare global {
// injected in scripts/jestPerTestSetup.ts
const browserLogs: string[]
const viteTestUrl: string
const watcher: RollupWatcher
}
13 changes: 11 additions & 2 deletions packages/playground/testUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,12 @@ export function readFile(filename: string) {
return fs.readFileSync(path.resolve(testDir, filename), 'utf-8')
}

export function editFile(filename: string, replacer: (str: string) => string) {
if (isBuild) return
export function editFile(
filename: string,
replacer: (str: string) => string,
runInBuild: boolean = false
): void {
if (isBuild && !runInBuild) return
filename = path.resolve(testDir, filename)
const content = fs.readFileSync(filename, 'utf-8')
const modified = replacer(content)
Expand Down Expand Up @@ -122,3 +126,8 @@ export async function untilUpdated(
}
}
}

/**
* Send the rebuild complete message in build watch
*/
export { notifyRebuildComplete } from '../../scripts/jestPerTestSetup'
27 changes: 17 additions & 10 deletions packages/vite/src/node/plugins/asset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,21 @@ const assetHashToFilenameMap = new WeakMap<
ResolvedConfig,
Map<string, string>
>()
// save hashes of the files that has been emitted in build watch
const emittedHashMap = new WeakMap<ResolvedConfig, Set<string>>()

/**
* Also supports loading plain strings with import text from './foo.txt?raw'
*/
export function assetPlugin(config: ResolvedConfig): Plugin {
// assetHashToFilenameMap initialization in buildStart causes getAssetFilename to return undefined
assetHashToFilenameMap.set(config, new Map())
return {
name: 'vite:asset',

buildStart() {
assetCache.set(config, new Map())
assetHashToFilenameMap.set(config, new Map())
emittedHashMap.set(config, new Set())
},

resolveId(id) {
Expand Down Expand Up @@ -203,8 +207,6 @@ async function fileToBuiltUrl(
}

const file = cleanUrl(id)
const { search, hash } = parseUrl(id)
const postfix = (search || '') + (hash || '')
const content = await fsp.readFile(file)

let url
Expand All @@ -224,21 +226,26 @@ async function fileToBuiltUrl(
// https://bundlers.tooling.report/hashing/asset-cascade/
// https://github.com/rollup/rollup/issues/3415
const map = assetHashToFilenameMap.get(config)!

const contentHash = getAssetHash(content)
const { search, hash } = parseUrl(id)
const postfix = (search || '') + (hash || '')
const basename = path.basename(file)
const ext = path.extname(basename)
const fileName = path.posix.join(
config.build.assetsDir,
`${basename.slice(0, -ext.length)}.${contentHash}${ext}`
)
if (!map.has(contentHash)) {
const basename = path.basename(file)
const ext = path.extname(basename)
const fileName = path.posix.join(
config.build.assetsDir,
`${basename.slice(0, -ext.length)}.${contentHash}${ext}`
)
map.set(contentHash, fileName)
}
const emittedSet = emittedHashMap.get(config)!
if (!emittedSet.has(contentHash)) {
pluginContext.emitFile({
fileName,
type: 'asset',
source: content
})
emittedSet.add(contentHash)
}

url = `__VITE_ASSET__${contentHash}__${postfix ? `$_${postfix}__` : ``}`
Expand Down
4 changes: 2 additions & 2 deletions packages/vite/src/node/plugins/css.ts
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,8 @@ export function cssPlugin(config: ResolvedConfig): Plugin {
* Plugin applied after user plugins
*/
export function cssPostPlugin(config: ResolvedConfig): Plugin {
let styles: Map<string, string>
// styles initialization in buildStart causes a styling loss in watch
const styles: Map<string, string> = new Map<string, string>()
let pureCssChunks: Set<string>

// when there are multiple rollup outputs and extracting CSS, only emit once,
Expand All @@ -236,7 +237,6 @@ export function cssPostPlugin(config: ResolvedConfig): Plugin {

buildStart() {
// Ensure new caches for every build (i.e. rebuilding in watch mode)
styles = new Map<string, string>()
pureCssChunks = new Set<string>()
outputToExtractedCSSMap = new Map<NormalizedOutputOptions, string>()
},
Expand Down
47 changes: 45 additions & 2 deletions scripts/jestPerTestSetup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,17 @@ import fs from 'fs-extra'
import * as http from 'http'
import { resolve, dirname } from 'path'
import sirv from 'sirv'
import { createServer, build, ViteDevServer, UserConfig } from 'vite'
import {
createServer,
build,
ViteDevServer,
UserConfig,
PluginOption,
ResolvedConfig
} from 'vite'
import { Page } from 'playwright-chromium'
// eslint-disable-next-line node/no-extraneous-import
import { RollupWatcher, RollupWatcherEvent } from 'rollup'

const isBuildTest = !!process.env.VITE_TEST_BUILD

Expand All @@ -18,6 +27,7 @@ declare global {
interface Global {
page?: Page
viteTestUrl?: string
watcher?: RollupWatcher
}
}
}
Expand Down Expand Up @@ -99,7 +109,22 @@ beforeAll(async () => {
await page.goto(url)
} else {
process.env.VITE_INLINE = 'inline-build'
await build(options)
// determine build watch
let resolvedConfig: ResolvedConfig
const resolvedPlugin: () => PluginOption = () => ({
name: 'vite-plugin-watcher',
configResolved(config) {
resolvedConfig = config
}
})
options.plugins = [resolvedPlugin()]
const rollupOutput = await build(options)
const isWatch = !!resolvedConfig!.build.watch
// in build watch,call startStaticServer after the build is complete
if (isWatch) {
global.watcher = rollupOutput as RollupWatcher
await notifyRebuildComplete(global.watcher)
}
const url = (global.viteTestUrl = await startStaticServer())
await page.goto(url)
}
Expand Down Expand Up @@ -168,3 +193,21 @@ function startStaticServer(): Promise<string> {
})
})
}

/**
* Send the rebuild complete message in build watch
*/
export async function notifyRebuildComplete(
watcher: RollupWatcher
): Promise<RollupWatcher> {
let callback: (event: RollupWatcherEvent) => void
await new Promise((resolve, reject) => {
callback = (event) => {
if (event.code === 'END') {
resolve(true)
}
}
watcher.on('event', callback)
})
return watcher.removeListener('event', callback)
}

0 comments on commit 6c72373

Please sign in to comment.