-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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(asset): respect assetFileNames if rollupOptions.output is an array #8561
fix(asset): respect assetFileNames if rollupOptions.output is an array #8561
Conversation
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. | ||
let assetFileNames: Exclude<OutputOptions['assetFileNames'], undefined> = | ||
(output && !Array.isArray(output) ? output.assetFileNames : undefined) ?? | ||
// defaults to '<assetsDir>/[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 | ||
} | ||
|
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.
Using the first output configured by the user is a better choice here. Should we consider outputting multiple assets according to array in lib mode? (Maybe it has nothing to related to this PR)
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.
Yes, we could keep working to improve this later. At least this PR fixes the plugin legacy issue that is very common. So i think we should merge and then work on another PR
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.
BTW, lib mode is just asset output once. and chunk and entry will output multi bundle
Hello @poyoho This MR caused a problem in our Vite Config and I would like to understand a couple of things before creating an issue. My Vite Config looks like this export default defineConfig(() => {
return {
...
build: {
outDir: "dist/fragments",
assetsDir,
rollupOptions: {
input: {
...
},
output: [
{
entryFileNames: `${assetsDir}/fallback/[name].js`,
chunkFileNames: `${assetsDir}/fallback/[name].js`,
assetFileNames: `${assetsDir}/fallback/[name].[ext]`,
},
{
entryFileNames: `${assetsDir}/[name].[hash].js`,
chunkFileNames: `${assetsDir}/[name].[hash].js`,
assetFileNames: `${assetsDir}/[name].[hash].[ext]`,
},
],
},
},
};
}); We had this config since we need a copy of our builds without hashed names. This has worked for us with vIte@v2. Now the question, Why should output assetFileNames be ignored if it is an array, it's not really "respecting" rollup options. I've checked your MR #8597 and it seems that the same warning is still there. Am I missing something here? |
Fix #4628
Description
If build.rollupOptions.output is configured as array, the assetFileNames option will be ignored, instead using a default assetFileNames. This PR adopt the first assetFileNames option in the array, and give a warn if multiple assetFileNames return different values.
Most users encounter this bug while using @vitejs/plugin-legacy, which wraps the output option, change it from object to array.
Minimal reproduction
Steps:
npm run build
typescript.svg
. It should be emmited todist/assets/subdir
, but actually indist/assets
.Additional context
I create a vite config file
vite.config-multiple-output.js
in playground/legacy, and a corresponding npm scriptpnpm build:multiple-output
. After running this script, a vite.svg should be emitted todist/assets/subdir/vite.[some hash].svg
, and a warn log colored with yellow should be observed.What is the purpose of this pull request?
Bug fix
New Feature
Documentation update
Other
Read the Contributing Guidelines.
Read the Pull Request Guidelines and follow the Commit Convention.
Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g.
fixes #123
).Ideally, include relevant tests that fail without this PR but pass with it.