fix(ssr-manifest): check name before saving to ssrManifest

[jambonn]

Description

fix: #9292.

Additional context

I experience the same situation when using async component but import from variable cdn and setup build minify = false. Please update for both v3
Example:

const app = createSSRApp(App)
app.component('Test', defineAsyncComponent(() => import(/* @vite-ignore */ `https://cdn.com/${variable}`)))

---

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

Before submitting the PR, please make sure you do the following

• Read the Contributing Guidelines.
• Read the Pull Request Guidelines and follow the Commit Convention.
• Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
• 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.

[netlify]

✅ Deploy Preview for vite-docs-v2 ready!

Name	Link
🔨 Latest commit	5b3f493
🔍 Latest deploy log	https://app.netlify.com/sites/vite-docs-v2/deploys/62f472928e011e000a6885e7
😎 Deploy Preview	https://deploy-preview-9595--vite-docs-v2.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[bluwy]

I think it would be nice to have a test for this. From what I understand I think dynamic imports should already be skipped at

vite/packages/vite/src/node/ssr/ssrManifestPlugin.ts

Line 42 in b718f95

imports = parseImports(code)[0].filter((i) => i.d > -1)

[jambonn]

I think it would be nice to have a test for this. From what I understand I think dynamic imports should already be skipped at

vite/packages/vite/src/node/ssr/ssrManifestPlugin.ts

Line 42 in b718f95

imports = parseImports(code)[0].filter((i) => i.d > -1)

@bluwy Thank you for comments. I'll try where you contribute and add it to the test file.

[jambonn]

@bluwy I edited as per your suggestion and checked back at my project and the result was correct. I created a test file but when I ran it, the package was missing so i don't push code. Please review for me.

[bluwy]

I was mentioning that your example that caused the issue was:

const app = createSSRApp(App)
app.component('Test', defineAsyncComponent(() => import(/* @vite-ignore */ `https://cdn.com/${variable}`)))

Presumably the dynamic import is causing issues, but in the code we already skipped dynamic imports, so I was hoping you can clarify how this happens, and maybe in a way of writing a test.

Feel free to push the test code so we can review it too.

[jambonn]

@bluwy Sorry, this is my demo https://stackblitz.com/edit/vitejs-vite-y363sz?file=vue-project%2Fsrc%2Fmain.js,vue-project%2Fpackage.json&terminal=dev. The steps are as follows

1. cd vue-project
2. yarn
3. yarn build
   You'll see an error. In file vite.config.js if you setup minify: true, there will be no errors. Thank you

[bluwy]

Presumably the dynamic import is causing issues, but in the code we already skipped dynamic imports, so I was hoping you can clarify how this happens, and maybe in a way of writing a test.

Ugh just realized my brainfart 😅 So apparently we're filtering and only want dynamic imports only. My mind sort of flipped there.

This change makes sense then, and I think you uncovered a bigger problem too that why this only happens for minify: false. So it looks like that we only handle this when we see __vitePreload in the code, but after minifying the name is minified too, skipping this.

I think this PR is good without a test for now, thanks for the explanation!

[jambonn]

@bluwy It's great that you understand my problem. Thank you for support!

[jambonn]

@bluwy @patak-dev Do I need to create MR for v3?

[bluwy]

Sure feel free 👍