-
-
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: support preloading also on legacy bundle #9920
base: main
Are you sure you want to change the base?
Conversation
a413bc0
to
2dc09b4
Compare
…ite into preload-on-system-format
Help: Don't know why vue legacy tests are failing. |
It seems this image is not being loaded when doing vite/playground/vue-legacy/module.vue Line 9 in 4158b98
|
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. |
This is done by comparing short pathes to the full ones.
…mance by caching.
and improve typing
This implementation depends on regexs very heavily and I felt a bit fragile. 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 is why I added testing snapshots😃
How do we get the dependency list here? Feels like we need to have the regexps (or an alternative solution) for this again. |
I think we can't. Dependency graph can only be obtained in
I guess we can use |
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 The
On the first one, the tailor-made new function On the second one however, the format might be Now return to what I started with in this comment - it seems that we need the locations of the dynamic imports on the vite/packages/vite/src/node/plugins/importAnalysisBuild.ts Lines 525 to 540 in 4b8a587
If we can overcome this issue, we can eliminate the need of the function Additionally, another sad truth - we still need the newly introduced function vite/packages/vite/src/node/plugins/css.ts Lines 630 to 655 in 6d4d82e
How then can we overcome the removing process of pure css imports? |
@sapphi-red, continuing the logic in my previous comment, do you think that any (breaking?) change of Vite 4.0 can help us here? |
We only need that what will be dynamically imported in that chunk. I'm suggesting to declare the following code in the head of the chunk, not aside of the window.__viteLegacyDeps['chunk file'] = [/* dep list */] For example, transform this code // main.000.js ------------------------------
System.register([], function (exports, module) {
module.import('foo.111.js')
module.import('bar.222.js')
});
// foo.111.js ------------------------------
System.register([], function (exports, module) {
console.log('foo.111.js')
});
// foo.111.css ------------------------------
/* css content */
// bar.222.js ------------------------------
System.register([], function (exports, module) {
console.log('bar.222.js')
module.import('foobar.333.js')
});
// foobar.333.js ------------------------------
System.register([], function (exports, module) {
console.log('fioobar.333.js')
}); to // main.000.js ------------------------------
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)
}
}
window.__viteLegacyDeps['foo.111.js'] = ['foo.111.css']
window.__viteLegacyDeps['bar.222.js'] = []
System.register([], function (exports, module) {
module.import('foo.111.js')
module.import('bar.222.js')
});
// foo.111.js ------------------------------
System.register([], function (exports, module) {
console.log('foo.111.js')
});
// foo.111.css ------------------------------
/* css content */
// bar.222.js ------------------------------
window.__viteLegacyDeps['foobar.333.js'] = []
System.register([], function (exports, module) {
console.log('bar.222.js')
module.import('foobar.333.js')
});
// foobar.333.js ------------------------------
System.register([], function (exports, module) {
console.log('fioobar.333.js')
});
How about injecting
I'm not able to track the changes deeply but I guess no. |
Took some work but finally made it with SystemJS hooks! |
I'm sorry that it took a long time. I tried this PR out and found that it doesn't work when there's a dynamic import in a dynamic imported chunk. |
What is the way to go with that? |
I think injecting the following code to every chunk in ${preloadListMarker}.forEach((data) => {
${preloadLegacyImportsToken}[assetsURL(data[0], import.meta.url)] = {
deps: data[1],
pure: data.length === 3
}
})
This PR already depends on |
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 #10285
Additional context
As mentioned in #9902 and #10285, 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?
Before submitting the PR, please make sure you do the following
fixes #123
).