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
prepareChunk hook #4713
Comments
So you suggest to add a hook that runs at chunk generation time that allows you to modify chunks before I am not sure this will work. The problem is that when we are calling So my first question is: Why can't you replace these references in a transform hook that runs after all other hooks? For assets referencing assets, this would be quite a different challenge. And maybe a chance to evolve the asset handling quite a bit more. I would be more than happy to bounce ideas with Vite here to create an API that is actually useful. So for assets, I guess replacing placeholders would be the way to go. This would happen when assets are finalized and would be straightforward as long as assets only reference assets and there are no circular references between assets. Alternatively, we could even go one step further and align the asset and chunk hashing so that like chunks, assets only receive preliminary file names with placeholders first. Then it would be a two-pass process: When they are finalized, all file references in assets are replaced by their preliminary names, which could also includes referencing chunks from assets. Then in a second pass, we calculate the actual hashes and replace the placeholders with their final values. To top it off, there has been the question of a But maybe let's go back to the first question above. |
We can't replace these references in a transform hook because we need the chunk fileName (actually we only need the folder where it ends up) because we want to allow relative paths from the host chunk to the assets (in the same way that So the idea of having a This is only needed for JS chunks, and these referenced assets are standalone (don't reference any other asset). We emit assets for CSS files in some cases, and these could reference other assets but in that case, we can compute the relative path between assets before the chunking process starts. In case it is useful, here is where we are replacing the placeholder for JS files (this runs in |
Of course I meant replacing them with Or is the core problem that you cannot emit the file in Rollup for some reason? |
For some reason, I thought that our |
Ok, I tried to move our processing to a If I understand correctly, replacing our To avoid the duplication of getRelativeUrlFromDocument in Vite (see here), maybe this function could be exported as an util from rollup? |
I think the problem is that I am still not 100% sure about what you are trying to do, so let me try to paraphrase:
Of course we could also think about exporting something like |
Thanks for the detailed response. I thought about using I'll try once more to refactor things on Vite's side. I think we could start with |
Some more information after hitting a wall again. And I think this also shows that a
For all of these cases, we currently use the same scheme. We have So, some issues when trying to use
I still don't see how we could unify these cases using I think we should close this issue, or leave it open while we discuss, this was really useful to understand what options we have already. We already made some progress to move some of the duplicated and custom handling of Vite back into Rollup with:
And we are also thinking about: Maybe it is already enough for Vite 4. |
I think you should definitely proceed with Vite 4 at this point, I hope to do Rollup 4 a little earlier after Node 14 goes EOL next year (and probably with much fewer breaking changes), so there will be enough reason for you to do a Vite 5 in reasonable time. I think we should definitely continue discussions about these topics, especially about assets referencing assets and the "b.2" case above. Though my feeling is, the easy solution would be to just remove the asset reference in renderChunk when inlining, so one would just need a way to also remove the asset at this point. |
I think next year if there aren't many breaking changes in Rollup v4, we could start work in Vite v5 a lot sooner so we can release the two with less time between them. We also plan to release a new major to drop Node 14 support. Thanks again for this discussion! |
Feature Use Case
In Vite we are currently using our own scheme when emitting assets. Instead of
import.meta.ROLLUP_FILE_URL_referenceId
, we have__VITE_ASSET_referenceId__
. The original reasons this was created has been resolved by:We are exploring simplifying some of Vite's code to use more of Rollup standard paths to help with compatibility with both config options and Rollup plugins. See as an example:
I think we can't directly use
import.meta.ROLLUP_FILE_URL_referenceId
because we are emitting assets that are referenced in CSS files too. See for example here where we replace the__VITE_ASSET_referenceId__
in therenderChunk
hook:https://github.com/vitejs/vite/blob/main/packages/vite/src/node/plugins/asset.ts#L70
We also have an API to modify these asset URL references that unifies handling across JS, HTML, and CSS, and both emitted assets and public assets (a feature of Vite where files in a specific folder are copied as is to the dist folder). See docs for
renderBuiltUrl
here. This API is similar toresolveFileUrl
.As part of
renderBuiltUrl
, we are forced to recreate some of whatresolveFileUrl
already does. See Vite inlining some Rollup functions from here.My current thinking is that at least for Vite 4, we aren't going to drop our current asset referencing scheme. But I'm trying to see if we should convert
__VITE_ASSET__referenceId__
toimport.meta.ROLLUP_FILE_URL_referenceId
for JS chunks when we are generating relative URLs to avoid duplicating the above functions and to give users a way to configure the way these URLs at runtime are generated using the standardresolveFileUrl
.Feature Proposal
I think to be able to do this, we need a new
prepareChunk
hook that will let us process our own asset reference scheme and in some cases replace our placeholders withimport.meta.ROLLUP_FILE_URL_referenceId
, beforeresolveFileUrl
is called for the chunk.I don't know if this is enough justification for a new hook. If not, we could see if there is a way to call
resolveFileUrl
and the standardimport.meta.ROLLUP_FILE_URL_referenceId
handling on our side duringrenderChunk
. I don't see an easy way to do this without exposing more internals from Rollup though.The text was updated successfully, but these errors were encountered: