Skip to content
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

Merged
merged 7 commits into from Jun 15, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 23 additions & 0 deletions packages/vite/src/node/config.ts
Expand Up @@ -566,6 +566,29 @@ export async function resolveConfig(
)
}

// 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 assetFileNamesList = outputOption.map(
(output) => output.assetFileNames
)
if (assetFileNamesList.length > 1) {
const firstAssetFileNames = assetFileNamesList[0]
const hasDifferentReference = assetFileNamesList.some(
(assetFileNames) => assetFileNames !== firstAssetFileNames
)
if (hasDifferentReference) {
resolved.logger.warn(
colors.yellow(`
assetFileNames isn't equal for every build.rollupOptions.output. A single pattern across all outputs is supported by Vite.
`)
)
}
}
}

return resolved
}

Expand Down
19 changes: 15 additions & 4 deletions packages/vite/src/node/plugins/asset.ts
Expand Up @@ -353,11 +353,22 @@ async function fileToBuiltUrl(
const { search, hash } = parseUrl(id)
const postfix = (search || '') + (hash || '')
const output = config.build?.rollupOptions?.output
const assetFileNames =

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
}

Comment on lines +357 to +371
Copy link
Member

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)

Copy link
Member

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

Copy link
Member

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

const fileName = assetFileNamesToFileName(
assetFileNames,
file,
Expand Down
1 change: 1 addition & 0 deletions playground/legacy/main.js
@@ -1,4 +1,5 @@
import './style.css'
import './vite.svg'

async function run() {
const { fn } = await import('./async.js')
Expand Down
1 change: 1 addition & 0 deletions playground/legacy/package.json
Expand Up @@ -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"
},
Expand Down
28 changes: 28 additions & 0 deletions playground/legacy/vite.config-multiple-output.js
@@ -0,0 +1,28 @@
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() {
return 'assets/subdir/[name].[hash][extname]'
},
entryFileNames: `assets/subdir/[name].js`,
chunkFileNames: `assets/subdir/[name].js`
},
{
assetFileNames() {
return 'assets/subdir/[name].[hash][extname]'
},
entryFileNames: `assets/anotherSubdir/[name].js`,
chunkFileNames: `assets/anotherSubdir/[name].js`
}
]
}
}
})
1 change: 1 addition & 0 deletions playground/legacy/vite.svg
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.