-
-
Notifications
You must be signed in to change notification settings - Fork 7.6k
feat(build): provide names for asset entrypoints #19912
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
feat(build): provide names for asset entrypoints #19912
Conversation
sapphi-red
left a comment
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.
The change makes sense to me, and the implementation looks good.
Note: For non-CSS assets, the output file name cannot be adjusted based on the name specified in the input option. So in that case, this new property wouldn't be useful, but I think that's fine.
Instead of adding a test in packages/vite/src/node/__tests__/build.spec.ts, could you update the test in
| expect(dirFooAssetEntry.file).toMatch('assets/bar-') |
There's a similar entry for
input already so you should be able to add an expect without adding a new entry.| entrypoints.push(['bar.css', path.resolve(__dirname, './dir/foo.css')]) |
b48363c to
a066578
Compare
|
@sapphi-red done! |
sapphi-red
left a comment
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.
Thanks!
Co-authored-by: Stanislav Lashmanov <slashmanov@gitlab.com>
|
I am trying this out and it seems that Vite transforms entrypoint names for some reason. If you get I can not use I am not really sure how to proceed because one fix negates the other fix. @sapphi-red maybe you have thoughts on this? |
|
Here's an illustrated example. Entrypoint config: {
build: {
rollupOptions: {
input: {
'styles/tailwind.scss': `${path.join(ROOT_PATH, 'app/assets/builds/tailwind.css')}`
}
}
}
}Manifest output: {
"builds/tailwind.css": {
"file": "assets/tailwind-CefKy2DB.css",
"src": "builds/tailwind.css",
"isEntry": true,
"names": [
"styles/tailwind.css"
]
},
}I expected that entrypoint names would be preserved. - "names": ["styles/tailwind.css"]
+ "names": ["styles/tailwind.scss"] |
This is to emit the file with vite/packages/vite/src/node/plugins/css.ts Line 841 in c4e01dc
I guess we can fix this by saving the non-replaced one here and then using that here.
This sounds like a bug to me. Would you create an issue with a reproduction? |
That's my problem with it. Entrypoint names are not files, they're just identifiers. So I have an entrypoint with a name
Thanks @sapphi-red! Do you want me to replace what we have in
Of course, I'll try to isolate the problem. |
I think we can replace it. I think the expected behavior is to have the same value that is set in the
That makes sense to me. I guess we need to store whether the asset is specified as an entrypoint to achieve that. |
The problem was the following: {
find: /^gitlab-(sans|mono)\//,
- replacement: 'node_modules/@gitlab/fonts/gitlab-$1/',
+ replacement: '@gitlab/fonts/gitlab-$1/',
},I think this was not the issue with Vite but with our configuration. Changing this to a proper replacement fixed the issue. |
Description
Add
namesfield to asset entrypoints in manifest.This allows you to map back your asset-only entrypoints to corresponding files in the bundle. This was not possible previously because asset entrypoints only had
file,srcandisEntryfiles, none of which included information about entrypoint name.