-
-
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
fix: inline dynamic imports for ssr-webworker (fixes #9385) #9401
Conversation
This PR passed ecosystem-ci: https://github.com/vitejs/vite-ecosystem-ci/actions/runs/2751820829. |
) { | ||
return { | ||
code: | ||
`import { __ssr_vue_processAssetPath } from '${virtualId}';` + | ||
`import { __ssr_vue_processAssetPath } from '${virtualId}';__ssr_vue_processAssetPath;` + |
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.
Why is this needed? And why the guard against importing it twice has been removed?
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.
I'm not sure why this is needed. But if I remove this, assets/Home.hash.js
won't include the import.
About the guard, it was an artifact during struggling about that. 😅 Thanks for pointing it out!
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.
Maybe moduleSideEffects: 'no-treeshake'
means the module is side-effectful but the imports can be removed if not used?
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.
I would think that should be respected, but we could move with your change for the moment until we find out a better way.
Description
#8641 disabled
inlineDynamicImports
for ssr-webworker. Instead, it should disable for ssr-node.This PR fixes that.
fixes #9385
Additional context
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).