vite
vite copied to clipboard
feat: support preloading also on legacy bundle
Description
Support preloading also on legacy bundle.
A test case is included for a utility function I'm using.
closes #9902 and even more (since it's also preload JS scripts) fixes #2062 closes #5901 fixes #9967 (by a more careful checking on the preload function)
Additional context
As mentioned in #9902, by solving the issue with this PR, it makes the legacy build support for SvelteKit I'm working on(sveltejs/kit#6265) to load correctly the CSS files when navigating(and more correctly, on preloading) from one page to the other.
What is the purpose of this pull request?
- [X] New Feature
- [ ] Documentation update
- [ ] Other
Before submitting the PR, please make sure you do the following
- [X] Read the Contributing Guidelines.
- [X] Read the Pull Request Guidelines and follow the Commit Convention.
- [X] Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
- [x] Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g.
fixes #123
). - [ ] Ideally, include relevant tests that fail without this PR but pass with it.
Help: Don't know why vue legacy tests are failing.
It seems this image is not being loaded when doing pnpm build && pnpm preview
in playground/vue-legacy
.
https://github.com/vitejs/vite/blob/4158b98c8fc30070967de1176f4b1c252d275386/playground/vue-legacy/module.vue#L9
It seems this image is not being loaded when doing
pnpm build && pnpm preview
inplayground/vue-legacy
.https://github.com/vitejs/vite/blob/4158b98c8fc30070967de1176f4b1c252d275386/playground/vue-legacy/module.vue#L9
Tnx! I had realized that the root cause for the failure is because I'm fighting with Vite way to inline the CSS on legacy. But the immediate question now is, why? I have commented this in the original issue I had opened: https://github.com/vitejs/vite/issues/9902#issuecomment-1236194069
I tested with Chrome 16 and it worked greatly. 👍 (when minify is disabled)
Test now, it's working on my machine also after minification, including testing.
Note: As said, I had stripped out the XHR call. This results with the side effect that if the network resource(the CSS file) isn't available, then no exception will be raised on legacy.
If this is crucial I can return this extra call.
Note2: I had modified a little bit the directory structure of the testing, to include the code samples. Hope it's fine.
Now I think it's ready. ~Suggest to merge #9970 first, both for credits and test units.~
I had removed the fix of #9967 from this PR, since it's handled better in #9970. Now both #9970 and this PR are independent, and merging one won't affect the other.
Note: Need to still implement in the future the suggestion appears in the discussion of https://github.com/vitejs/vite/pull/9970#discussion_r969505865, but this can be done later in a different PR, as this suggestion is nice to have.
So now I'm ready for review&merge.
This implementation depends on regexs very heavily and I felt a bit fragile.
So I have a suggestion, how about using instantiate
hook of SystemJS? In this way, I think we could avoid regexs.
In entry chunks:
if (!window.__viteLegacyDeps) {
window.__viteLegacyDeps = {}
const originalInstantiate = System.constructor.prototype.instantiate
System.constructor.prototype.instantiate = function (args) {
// preload using the information of `window.__viteLegacyDeps`
return existingHook.call(this, args)
}
}
In each chunks:
window.__viteLegacyDeps['chunk file'] = [/* dep list */]
This implementation depends on regexs very heavily and I felt a bit fragile.
This is why I added testing snapshots😃
I agree however about the principle. ESModules import are scanned well by Vite. Do you think we can somehow generate a map at the transform
/renderChunk
stage(or such) between the es
-formatted chunk to their (well-)detected dynamically imported (other) chunks, and then use it directly for writing the dependency list?
In each chunks:
window.__viteLegacyDeps['chunk file'] = [/* dep list */]
How do we get the dependency list here? Feels like we need to have the regexps (or an alternative solution) for this again.
Do you think we can somehow generate a map at the
transform
/renderChunk
stage(or such) between thees
-formatted chunk to their (well-)detected dynamically imported (other) chunks, and then use it directly for writing the dependency list?
I think we can't. Dependency graph can only be obtained in renderChunk
/generateBundle
, but in those hooks the code is already transformed to SystemJS.
How do we get the dependency list here? Feels like we need to have the regexps (or an alternative solution) for this again.
I guess we can use dynamicImports
property of ChunkInfo
to get what is dynamic-imported. Tracing static-imports could be done like we do now.
https://rollupjs.org/guide/en/#generatebundle
Do you think we can somehow generate a map at the
transform
/renderChunk
stage(or such) between thees
-formatted chunk to their (well-)detected dynamically imported (other) chunks, and then use it directly for writing the dependency list?I think we can't. Dependency graph can only be obtained in
renderChunk
/generateBundle
, but in those hooks the code is already transformed to SystemJS.How do we get the dependency list here? Feels like we need to have the regexps (or an alternative solution) for this again.
I guess we can use
dynamicImports
property ofChunkInfo
to get what is dynamic-imported. Tracing static-imports could be done like we do now. https://rollupjs.org/guide/en/#generatebundle
I deepen into Rollup chunk information, but it seems to tell us only about the dependencies sources, but not on the location on the chunk that the "import" is used, and parseImports
/parseImportsSystemJS
give us this extra information.
The parseImports
is used twice:
- On the
transform
stage. - On the
generateBundle
phase.
On the first one, the tailor-made new function parseImportsSystemJS
isn't needed, since on the transform
stage, the code is always in the es
format, before the legacy conversions, so no problem at all.
On the second one however, the format might be es
(modern) or system
(legacy), so this is why we have both parseImports
and parseImportsSystemJS
, where the latter uses regular expressions.
Now return to what I started with in this comment - it seems that we need the locations of the dynamic imports on the generateBundle
stage only for one good reason - to remove the pure CSS chunk imports:
https://github.com/vitejs/vite/blob/4b8a58703055a0eb5d8ddb04ef9f1eb081fbbc6a/packages/vite/src/node/plugins/importAnalysisBuild.ts#L525-L540
If we can overcome this issue, we can eliminate the need of the function parseImportsSystemJS
.
Additionally, another sad truth - we still need the newly introduced function analyzeSystemRegistration
, since it used (also!) for removing pure css chunks in the css
plugin:
https://github.com/vitejs/vite/blob/6d4d82e87320c3361b41e8d831731bd6a0d57ed7/packages/vite/src/node/plugins/css.ts#L630-L655
How then can we overcome the removing process of pure css imports?