-
-
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
feat: experimental.renderBuiltUrl (revised build base options) #8762
Conversation
✅ Deploy Preview for vite-docs-main ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
@danielroe about your use case
with |
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.
Not following all the base
change lately, but I like this API more too
Renamed to The proposed config option in this PR (maybe future hook) would be an extended version of https://rollupjs.org/guide/en/#resolvefileurl, but working also with public files (we can't use this rollup hook, see here: vite/packages/vite/src/node/plugins/asset.ts Line 394 in d90409e
About the API,
|
One more thing about the API, I think we should pass the @danielroe I tried to do a plugin that uses this config together with Another option is to have a plugin that on |
Merging soon so Nuxt can continue to test this API in the next few days. Discussed with @danielroe offline that this seems a lot more flexible than the |
Co-authored-by: Bjorn Lu <bjornlu.dev@gmail.com>
{ | ||
name: 'virtual-module', | ||
resolveId(id) { | ||
if (id === virtualFile) { | ||
return virtualId | ||
} else if (id === nestedVirtualFile) { | ||
return nestedVirtualId | ||
} | ||
}, | ||
load(id) { | ||
if (id === virtualId) { | ||
return `export { msg } from "@nested-virtual-file";` | ||
} else if (id === nestedVirtualId) { | ||
return `export const msg = "[success] from conventional virtual file"` | ||
} | ||
} | ||
}, |
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.
@patak-dev this plugin it seems is already defined above 👀
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.
Ha, good catch! Would you like to do a PR to remove the second one from the config?
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.
will do
The old "advanced base options" supported this, was it removed in this version?? |
@ropez it should be called for the import path of the bundle, but not for preloads. We've decided to remove preload as they always work when you define relative base ( |
I'm having trouble with the TS types for Edit: Got it, the docs should read like this: experimental: {
renderBuiltUrl: (filename, { hostType }) => {
if (hostType === 'js') {
return { runtime: `window.__toCdnUrl(${JSON.stringify(filename)})` }
} else {
return { relative: true }
}
}
} i.e. remove the types on |
👋 Hi there! We're looking at using this feature and I was curious if there are any currently planned updates to the API. Fully aware that it's still under the experimental label in Vite v4, so it's probably not possible to promise no changes, and that's OK. I'm just curious if there is any general sense of how final this API is now that it's been out in the world for ~8 months, or any estimate on when it will be promoted. |
@wilcoxmd Nuxt and others are using this API already. You can safely use it. If we change something, it will be in a Major now. We'll review if in Vite 5 we can remove from experimental these features that have already been in Vite for a while. |
Amazing. Thanks for the quick update @patak-dev! Looking forward to trying this out. |
We're thinking of stabilizing this feature in Vite 5. If you have feedback, please comment in this discussion: |
Description
Alternative design to
Nuxt is planning to release an RC using the new experimental support for advanced base options, as it greatly improves the robustness of path handling for them (that before were using a post regex based approach)
We discussed with @danielroe and revised the API now that we have more experience with real usage.
API Simplification
Edited with the lastest changes in the PR
Inversion of control to simplify the API, making it more flexible. The option can now be only of type:
Example:
{ public, assets }
as in feat: experimental.buildAdvancedBaseOptions #8450, as the asset type (hashed assets:'asset'
, public files:'public'
) is passed to the function. The user can decide to treat both in the same way or do something specific for one of them.relative
option. The general relative option is set withbase: './'
as before, and the user can decide to change the default by returning{ relative: boolean }
for one of the assets.url
option. The user should return a proper path from therenderBuiltAssetUrl
function.runtime
option. The user can return{ runtime: code }
if the importer is a JS file. An error will be thrown if this is used in a different contextOther changes
/
(prev public files started with/
)"..."
wrapping. @danielroe I know we discussed that it would be better to keep the wrapping, but we needed this change to be able to return a path from the function asstring
instead of a wrapped path. This function is not only forruntime
now.modulePreload
config entry with filtering and transform, so we are going to be able to do this in the future through it if needed.New possibilities
The previous
runtime
option was only called for JS importers. The newrenderBuiltAssetUrl
is called for assets referenced in JS, CSS, and HTML. This may allow us in the future to create dynamic paths in CSS using CSS Variables (still not possible, but probably it will with Houdini).The new function also takes an
ssr
flag as there are times when the paths should be different. For example,relative
is ignored in SSR. @danielroe I think the flag is useful here to avoid a conditional config (and especially thinking about converting this to a hook), but we could end up removing it in the final version depending on usage.The name is inspired by other
renderYYY
hooks in rollup. I think we could make this option a hook in the future and the design is thought to enable us to do so. Keeping it as anexperimental
config option allows us to more easily change the design and avoid breaking compat so IMO is a good idea to wait until we are sure this is going to be the final version to convert it to a hook.What is the purpose of this pull request?