From 0996a1b6b976e487439554ecbc13fb3103b121e0 Mon Sep 17 00:00:00 2001 From: chenyitao Date: Mon, 13 Jun 2022 17:38:40 +0800 Subject: [PATCH 1/7] fix(asset): respect assetFileNames if rollupOptions.output is an array (fix #4628) --- packages/vite/src/node/plugins/asset.ts | 47 +++++++++++++++++-- playground/legacy/main.js | 1 + playground/legacy/package.json | 1 + .../legacy/vite.config-multiple-output.js | 24 ++++++++++ playground/legacy/vite.svg | 1 + 5 files changed, 70 insertions(+), 4 deletions(-) create mode 100644 playground/legacy/vite.config-multiple-output.js create mode 100644 playground/legacy/vite.svg diff --git a/packages/vite/src/node/plugins/asset.ts b/packages/vite/src/node/plugins/asset.ts index 4c1bcfea0fd50b..da379101b3e031 100644 --- a/packages/vite/src/node/plugins/asset.ts +++ b/packages/vite/src/node/plugins/asset.ts @@ -4,10 +4,12 @@ import fs, { promises as fsp } from 'fs' import * as mrmime from 'mrmime' import type { OutputOptions, PluginContext } from 'rollup' import MagicString from 'magic-string' +import colors from 'picocolors' import type { Plugin } from '../plugin' import type { ResolvedConfig } from '../config' import { cleanUrl, getHash, isRelativeBase, normalizePath } from '../utils' import { FS_PREFIX } from '../constants' +import { createLogger } from '../logger' export const assetUrlRE = /__VITE_ASSET__([a-z\d]{8})__(?:\$_(.*?)__)?/g @@ -353,11 +355,48 @@ async function fileToBuiltUrl( const { search, hash } = parseUrl(id) const postfix = (search || '') + (hash || '') const output = config.build?.rollupOptions?.output - const assetFileNames = + + // Steps to determine which assetFileNames will be actually used. + const defaultAssetFileNames = path.posix.join( + config.build.assetsDir, + '[name].[hash][extname]' + ) + // First, if output is an object, use assetFileNames in it. + // And a default assetFileNames as fallback. + let assetFileNames: Exclude = (output && !Array.isArray(output) ? output.assetFileNames : undefined) ?? - // defaults to '/[name].[hash][extname]' - // slightly different from rollup's one ('assets/[name]-[hash][extname]') - path.posix.join(config.build.assetsDir, '[name].[hash][extname]') + defaultAssetFileNames + if (output && Array.isArray(output)) { + // Second, if output is an array, adopt assetFileNames in the first object. + assetFileNames = output[0].assetFileNames ?? assetFileNames + // Third, check if assetFileNames among output array return the same result. + // If not, log a warn and keep using the first assetFileNames. + const filenameSet = output.reduce( + (cumulate, { assetFileNames = defaultAssetFileNames }) => { + const filename = + typeof assetFileNames === 'string' + ? assetFileNames + : assetFileNames({ + type: 'asset', + name: file, + source: content + }) + cumulate.add(filename) + return cumulate + }, + new Set() + ) + if (filenameSet.size > 1) { + createLogger('warn').warn( + colors.yellow(` +Found that you've configure multiple assetFileNames in build.rollupOptions.output, +and their return values are inconsistent. Vite has adopted the first assetFileNames in the array. +If you expecting a different asset file name, change the first assetFileNames in the array. +`) + ) + } + } + const fileName = assetFileNamesToFileName( assetFileNames, file, diff --git a/playground/legacy/main.js b/playground/legacy/main.js index 157b6c8448e9c3..24959b129f28f2 100644 --- a/playground/legacy/main.js +++ b/playground/legacy/main.js @@ -1,4 +1,5 @@ import './style.css' +import './vite.svg' async function run() { const { fn } = await import('./async.js') diff --git a/playground/legacy/package.json b/playground/legacy/package.json index 347e410e2bf4d6..a82e67d493637a 100644 --- a/playground/legacy/package.json +++ b/playground/legacy/package.json @@ -7,6 +7,7 @@ "build": "vite build --debug legacy", "build:custom-filename": "vite --config ./vite.config-custom-filename.js build --debug legacy", "build:dynamic-css": "vite --config ./vite.config-dynamic-css.js build --debug legacy", + "build:multiple-output": "vite --config ./vite.config-multiple-output.js build", "debug": "node --inspect-brk ../../packages/vite/bin/vite", "preview": "vite preview" }, diff --git a/playground/legacy/vite.config-multiple-output.js b/playground/legacy/vite.config-multiple-output.js new file mode 100644 index 00000000000000..c11159a78ef5f2 --- /dev/null +++ b/playground/legacy/vite.config-multiple-output.js @@ -0,0 +1,24 @@ +import legacy from '@vitejs/plugin-legacy' +import { defineConfig } from 'vite' + +export default defineConfig({ + plugins: [legacy({ modernPolyfills: true })], + build: { + manifest: true, + minify: false, + rollupOptions: { + output: [ + { + assetFileNames: 'assets/subdir/[name].[hash][extname]', + entryFileNames: `assets/subdir/[name].js`, + chunkFileNames: `assets/subdir/[name].js` + }, + { + assetFileNames: 'assets/anotherSubdir/[name].[hash][extname]', + entryFileNames: `assets/anotherSubdir/[name].js`, + chunkFileNames: `assets/anotherSubdir/[name].js` + } + ] + } + } +}) diff --git a/playground/legacy/vite.svg b/playground/legacy/vite.svg new file mode 100644 index 00000000000000..e7b8dfb1b2a60b --- /dev/null +++ b/playground/legacy/vite.svg @@ -0,0 +1 @@ + \ No newline at end of file From 99fab6ff56d3c56a3910405b11b370892e7505d1 Mon Sep 17 00:00:00 2001 From: chenyitao Date: Mon, 13 Jun 2022 23:58:37 +0800 Subject: [PATCH 2/7] perf(asset): move assetFileNames check to config.ts --- packages/vite/src/node/config.ts | 30 ++++++++++++++++++++++- packages/vite/src/node/plugins/asset.ts | 32 +++---------------------- 2 files changed, 32 insertions(+), 30 deletions(-) diff --git a/packages/vite/src/node/config.ts b/packages/vite/src/node/config.ts index ce897d745d2e27..464732027e033a 100644 --- a/packages/vite/src/node/config.ts +++ b/packages/vite/src/node/config.ts @@ -8,7 +8,7 @@ import type { Alias, AliasOptions } from 'types/alias' import { createFilter } from '@rollup/pluginutils' import aliasPlugin from '@rollup/plugin-alias' import { build } from 'esbuild' -import type { RollupOptions } from 'rollup' +import type { OutputOptions, RollupOptions } from 'rollup' import type { Plugin } from './plugin' import type { BuildOptions, ResolvedBuildOptions } from './build' import { resolveBuildOptions } from './build' @@ -566,6 +566,34 @@ export async function resolveConfig( ) } + // Check if all assetFileNames functions have the same reference. + // If not, display a warn for user. + const outputOption = config.build?.rollupOptions?.output ?? [] + // Use isArray to narrow its type to array + if (Array.isArray(outputOption)) { + const assetFileNamesFunctions = outputOption + .map((output) => output.assetFileNames) + .filter( + (assetFileNames) => typeof assetFileNames === 'function' + ) as Exclude< + OutputOptions['assetFileNames'], + undefined | string + >[] + if (assetFileNamesFunctions.length > 1) { + const firstFunction = assetFileNamesFunctions[0] + const hasDifferentFunction = assetFileNamesFunctions + .slice(1) + .some((assetFileNames) => assetFileNames !== firstFunction) + if (hasDifferentFunction) { + resolved.logger.warn(colors.yellow(` +It's recommended that all assetFileNames in function syntax have the same reference. +In other words, every function1 === function2 should be true. +Vite adopts the first assetFileNames if build.rollupOptions.output is an array. +`)) + } + } + } + return resolved } diff --git a/packages/vite/src/node/plugins/asset.ts b/packages/vite/src/node/plugins/asset.ts index da379101b3e031..37106e0714afff 100644 --- a/packages/vite/src/node/plugins/asset.ts +++ b/packages/vite/src/node/plugins/asset.ts @@ -356,12 +356,12 @@ async function fileToBuiltUrl( const postfix = (search || '') + (hash || '') const output = config.build?.rollupOptions?.output - // Steps to determine which assetFileNames will be actually used. const defaultAssetFileNames = path.posix.join( config.build.assetsDir, '[name].[hash][extname]' - ) - // First, if output is an object, use assetFileNames in it. + ) + // Steps to determine which assetFileNames will be actually used. + // First, if output is an object or string, use assetFileNames in it. // And a default assetFileNames as fallback. let assetFileNames: Exclude = (output && !Array.isArray(output) ? output.assetFileNames : undefined) ?? @@ -369,32 +369,6 @@ async function fileToBuiltUrl( if (output && Array.isArray(output)) { // Second, if output is an array, adopt assetFileNames in the first object. assetFileNames = output[0].assetFileNames ?? assetFileNames - // Third, check if assetFileNames among output array return the same result. - // If not, log a warn and keep using the first assetFileNames. - const filenameSet = output.reduce( - (cumulate, { assetFileNames = defaultAssetFileNames }) => { - const filename = - typeof assetFileNames === 'string' - ? assetFileNames - : assetFileNames({ - type: 'asset', - name: file, - source: content - }) - cumulate.add(filename) - return cumulate - }, - new Set() - ) - if (filenameSet.size > 1) { - createLogger('warn').warn( - colors.yellow(` -Found that you've configure multiple assetFileNames in build.rollupOptions.output, -and their return values are inconsistent. Vite has adopted the first assetFileNames in the array. -If you expecting a different asset file name, change the first assetFileNames in the array. -`) - ) - } } const fileName = assetFileNamesToFileName( From f3bd5701eb4d1ed8add4685dffa84c0ac4e63ad1 Mon Sep 17 00:00:00 2001 From: chenyitao Date: Tue, 14 Jun 2022 10:05:43 +0800 Subject: [PATCH 3/7] chore: fix eslint issues --- packages/vite/src/node/plugins/asset.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/vite/src/node/plugins/asset.ts b/packages/vite/src/node/plugins/asset.ts index 37106e0714afff..36013e77c8f2f0 100644 --- a/packages/vite/src/node/plugins/asset.ts +++ b/packages/vite/src/node/plugins/asset.ts @@ -4,12 +4,10 @@ import fs, { promises as fsp } from 'fs' import * as mrmime from 'mrmime' import type { OutputOptions, PluginContext } from 'rollup' import MagicString from 'magic-string' -import colors from 'picocolors' import type { Plugin } from '../plugin' import type { ResolvedConfig } from '../config' import { cleanUrl, getHash, isRelativeBase, normalizePath } from '../utils' import { FS_PREFIX } from '../constants' -import { createLogger } from '../logger' export const assetUrlRE = /__VITE_ASSET__([a-z\d]{8})__(?:\$_(.*?)__)?/g @@ -359,7 +357,7 @@ async function fileToBuiltUrl( const defaultAssetFileNames = path.posix.join( config.build.assetsDir, '[name].[hash][extname]' - ) + ) // Steps to determine which assetFileNames will be actually used. // First, if output is an object or string, use assetFileNames in it. // And a default assetFileNames as fallback. From 325c0739c3ce1ce8f7a550b5a719f2fce0b1c529 Mon Sep 17 00:00:00 2001 From: chenyitao Date: Tue, 14 Jun 2022 10:14:44 +0800 Subject: [PATCH 4/7] chore: fix prettier issues --- packages/vite/src/node/config.ts | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/packages/vite/src/node/config.ts b/packages/vite/src/node/config.ts index 464732027e033a..0f58fb37128499 100644 --- a/packages/vite/src/node/config.ts +++ b/packages/vite/src/node/config.ts @@ -575,21 +575,20 @@ export async function resolveConfig( .map((output) => output.assetFileNames) .filter( (assetFileNames) => typeof assetFileNames === 'function' - ) as Exclude< - OutputOptions['assetFileNames'], - undefined | string - >[] + ) as Exclude[] if (assetFileNamesFunctions.length > 1) { const firstFunction = assetFileNamesFunctions[0] const hasDifferentFunction = assetFileNamesFunctions .slice(1) .some((assetFileNames) => assetFileNames !== firstFunction) if (hasDifferentFunction) { - resolved.logger.warn(colors.yellow(` + resolved.logger.warn( + colors.yellow(` It's recommended that all assetFileNames in function syntax have the same reference. In other words, every function1 === function2 should be true. Vite adopts the first assetFileNames if build.rollupOptions.output is an array. -`)) +`) + ) } } } From 03409aa26ad49ca5fc7d573c8d931d2be9659a38 Mon Sep 17 00:00:00 2001 From: chenyitao Date: Tue, 14 Jun 2022 20:14:32 +0800 Subject: [PATCH 5/7] test(legacy): modify multiple output build config --- playground/legacy/vite.config-multiple-output.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/playground/legacy/vite.config-multiple-output.js b/playground/legacy/vite.config-multiple-output.js index c11159a78ef5f2..6d958bd892d621 100644 --- a/playground/legacy/vite.config-multiple-output.js +++ b/playground/legacy/vite.config-multiple-output.js @@ -9,12 +9,16 @@ export default defineConfig({ rollupOptions: { output: [ { - assetFileNames: 'assets/subdir/[name].[hash][extname]', + assetFileNames() { + return 'assets/subdir/[name].[hash][extname]' + }, entryFileNames: `assets/subdir/[name].js`, chunkFileNames: `assets/subdir/[name].js` }, { - assetFileNames: 'assets/anotherSubdir/[name].[hash][extname]', + assetFileNames() { + return 'assets/subdir/[name].[hash][extname]' + }, entryFileNames: `assets/anotherSubdir/[name].js`, chunkFileNames: `assets/anotherSubdir/[name].js` } From 92354a809a47921abc26c3db7a6505d69e75128c Mon Sep 17 00:00:00 2001 From: chenyitao Date: Tue, 14 Jun 2022 20:36:53 +0800 Subject: [PATCH 6/7] fix(asset): modify multiple output assetFileNames config check strategy --- packages/vite/src/node/config.ts | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/packages/vite/src/node/config.ts b/packages/vite/src/node/config.ts index 0f58fb37128499..1585d3b83381fd 100644 --- a/packages/vite/src/node/config.ts +++ b/packages/vite/src/node/config.ts @@ -8,7 +8,7 @@ import type { Alias, AliasOptions } from 'types/alias' import { createFilter } from '@rollup/pluginutils' import aliasPlugin from '@rollup/plugin-alias' import { build } from 'esbuild' -import type { OutputOptions, RollupOptions } from 'rollup' +import type { RollupOptions } from 'rollup' import type { Plugin } from './plugin' import type { BuildOptions, ResolvedBuildOptions } from './build' import { resolveBuildOptions } from './build' @@ -566,26 +566,24 @@ export async function resolveConfig( ) } - // Check if all assetFileNames functions have the same reference. + // Check if all assetFileNames have the same reference. // If not, display a warn for user. const outputOption = config.build?.rollupOptions?.output ?? [] // Use isArray to narrow its type to array if (Array.isArray(outputOption)) { - const assetFileNamesFunctions = outputOption - .map((output) => output.assetFileNames) - .filter( - (assetFileNames) => typeof assetFileNames === 'function' - ) as Exclude[] - if (assetFileNamesFunctions.length > 1) { - const firstFunction = assetFileNamesFunctions[0] - const hasDifferentFunction = assetFileNamesFunctions + const assetFileNamesList = outputOption.map( + (output) => output.assetFileNames + ) + if (assetFileNamesList.length > 1) { + const firstAssetFileNames = assetFileNamesList[0] + const hasDifferentReference = assetFileNamesList .slice(1) - .some((assetFileNames) => assetFileNames !== firstFunction) - if (hasDifferentFunction) { + .some((assetFileNames) => assetFileNames !== firstAssetFileNames) + if (hasDifferentReference) { resolved.logger.warn( colors.yellow(` -It's recommended that all assetFileNames in function syntax have the same reference. -In other words, every function1 === function2 should be true. +It's recommended that all assetFileNames have the same reference. +In other words, every assetFileNames1 === assetFileNames2 should be true. Vite adopts the first assetFileNames if build.rollupOptions.output is an array. `) ) From b1282d366be8b76f1051ca9bda2c00f3c4b389e4 Mon Sep 17 00:00:00 2001 From: patak-dev Date: Tue, 14 Jun 2022 22:03:11 +0200 Subject: [PATCH 7/7] chore: update --- packages/vite/src/node/config.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/packages/vite/src/node/config.ts b/packages/vite/src/node/config.ts index 1585d3b83381fd..b3be2de2a4d500 100644 --- a/packages/vite/src/node/config.ts +++ b/packages/vite/src/node/config.ts @@ -576,15 +576,13 @@ export async function resolveConfig( ) if (assetFileNamesList.length > 1) { const firstAssetFileNames = assetFileNamesList[0] - const hasDifferentReference = assetFileNamesList - .slice(1) - .some((assetFileNames) => assetFileNames !== firstAssetFileNames) + const hasDifferentReference = assetFileNamesList.some( + (assetFileNames) => assetFileNames !== firstAssetFileNames + ) if (hasDifferentReference) { resolved.logger.warn( colors.yellow(` -It's recommended that all assetFileNames have the same reference. -In other words, every assetFileNames1 === assetFileNames2 should be true. -Vite adopts the first assetFileNames if build.rollupOptions.output is an array. +assetFileNames isn't equal for every build.rollupOptions.output. A single pattern across all outputs is supported by Vite. `) ) }