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.

Sign up for GitHub

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

Jump to bottom

feat(css): build assets with the entry name when it is an entry point #11578

Merged
merged 4 commits into from Aug 19, 2023
Merged

feat(css): build assets with the entry name when it is an entry point #11578

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ad70c53
chore: build assets with the entry name when it is an entry point
aleen42 Jan 4, 2023
fec0bee
review: use `chunk.name` to assign `cssAssetName` directly
aleen42 Jun 7, 2023
b6367ba
fix: windows test
sapphi-red Jun 13, 2023
795e054
test: restore test for #9353
sapphi-red Jun 13, 2023
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
10 changes: 6 additions & 4 deletions packages/vite/src/node/plugins/css.ts
View file
Open in desktop
Expand Up @@ -569,9 +569,12 @@ export function cssPostPlugin(config: ResolvedConfig): Plugin {
pureCssChunks.add(chunk)
}
if (opts.format === 'es' || opts.format === 'cjs') {
const cssAssetName = chunk.facadeModuleId
? normalizePath(path.relative(config.root, chunk.facadeModuleId))
: chunk.name
const isEntry = chunk.isEntry && isPureCssChunk
const cssAssetName = normalizePath(
!isEntry && chunk.facadeModuleId
? path.relative(config.root, chunk.facadeModuleId)
: chunk.name,
)

const lang = path.extname(cssAssetName).slice(1)
const cssFileName = ensureFileExt(cssAssetName, '.css')
Expand Down Expand Up @@ -601,7 +604,6 @@ export function cssPostPlugin(config: ResolvedConfig): Plugin {
source: chunkCSS,
})
const originalName = isPreProcessor(lang) ? cssAssetName : cssFileName
const isEntry = chunk.isEntry && isPureCssChunk
generatedAssets
.get(config)!
.set(referenceId, { originalName, isEntry })
Expand Down
5 changes: 4 additions & 1 deletion playground/backend-integration/__tests__/backend-integration.spec.ts
View file
Open in desktop
Expand Up @@ -37,7 +37,7 @@ describe.runIf(isBuild)('build', () => {
const cssAssetEntry = manifest['global.css']
const scssAssetEntry = manifest['nested/blue.scss']
const imgAssetEntry = manifest['../images/logo.png']
const dirFooAssetEntry = manifest['../../dir/foo.css'] // '\\' should not be used even on windows
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have a reason to remove this test? I think we shouldn't remove this one.

Copy link
Contributor Author

@aleen42 aleen42 Jun 7, 2023 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the behaviour has broken and used the chunk name as the name of the assets, so should test manifest['bar.css'] and manifest['foo.css']. The case has not actually been removed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't that because you changed this line?
https://github.com/vitejs/vite/pull/11578/files#diff-4fdf9a690fa55c7c730e03a900f729ce1afb886d64b4280cd4ce0f32d5844074L22-R22
I guess it would work if you revert that change.
I think you can append another entry point for the new test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if I revert the change, the chunk name has been used as the CSS asset name, instead of the path before, because this is an entry.

const dirFooAssetEntry = manifest['../dynamic/foo.css'] // '\\' should not be used even on windows
expect(htmlEntry.css.length).toEqual(1)
expect(htmlEntry.assets.length).toEqual(1)
expect(cssAssetEntry?.file).not.toBeUndefined()
Expand All @@ -48,6 +48,9 @@ describe.runIf(isBuild)('build', () => {
expect(imgAssetEntry?.file).not.toBeUndefined()
expect(imgAssetEntry?.isEntry).toBeUndefined()
expect(dirFooAssetEntry).not.toBeUndefined()
// use the entry name
expect(manifest['bar.css']).not.toBeUndefined()
expect(manifest['foo.css']).toBeUndefined()
})
})

Expand Down
2 changes: 1 addition & 1 deletion playground/backend-integration/dir/foo.css
View file
Open in desktop
@@ -1,3 +1,3 @@
.windows-path-foo {
.entry-name-foo {
color: blue;
}
3 changes: 3 additions & 0 deletions playground/backend-integration/frontend/dynamic/foo.css
View file
Open in desktop
@@ -0,0 +1,3 @@
.windows-path-foo {
color: blue;
}
1 change: 1 addition & 0 deletions playground/backend-integration/frontend/dynamic/foo.ts
View file
Open in desktop
@@ -0,0 +1 @@
import './foo.css'
1 change: 1 addition & 0 deletions playground/backend-integration/frontend/entrypoints/main.ts
View file
Open in desktop
@@ -1,4 +1,5 @@
import 'vite/modulepreload-polyfill'
import('../dynamic/foo') // should be dynamic import to split chunks

export const colorClass = 'text-black'

Expand Down
2 changes: 1 addition & 1 deletion playground/backend-integration/vite.config.js
View file
Open in desktop
Expand Up @@ -19,7 +19,7 @@ function BackendIntegrationExample() {
.map((filename) => [path.relative(root, filename), filename])

entrypoints.push(['tailwindcss-colors', 'tailwindcss/colors.js'])
entrypoints.push(['foo.css', path.resolve(__dirname, './dir/foo.css')])
entrypoints.push(['bar.css', path.resolve(__dirname, './dir/foo.css')])

return {
build: {
Expand Down