test: missing type module in ssr-vue package

[patak-dev]

Description

I think this may be causing a race condition that results in this kind of glitches: https://github.com/vitejs/vite/runs/6406742290?check_suite_focus=true#step:11:85

The change in #7568 may make this glitch more common.

---

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

[patak-dev]

Interesting, so instead of fixing the issue adding "type": "module" make it reproducible. I may be missing something here 🤔

[bluwy]

The error doesn't seem to come from optimizeDeps but in node when it tries to load the Vue file, but it looks like with this change example-external-component is being externalized in SSR which should be not. Setting ssr.noExternal: ['example-external-component'] made it worked again.

Though I'm not sure why type: "module" made this more obvious, perhaps the file type mismatch caused Vite to no-external it previously.

[patak-dev]

Looks like we were ignoring a warning before, this is the code path:

      const pkg = JSON.parse(pkgContent)
      if (pkg.type === 'module' || esmEntry.endsWith('.mjs')) {
        ssrExternals.add(id)
        continue
      }
      // check if the entry is cjs
      const content = fs.readFileSync(esmEntry, 'utf-8')
      if (CJS_CONTENT_RE.test(content)) {
        ssrExternals.add(id)
        continue
      }
      logger.warn(
        `${id} doesn't appear to be written in CJS, but also doesn't appear to be a valid ES module (i.e. it doesn't have "type": "module" or an .mjs extension for the entry point). Please contact the package author to fix.`
      )

When there isn't a type module, the package is wrong and Vite logs a warning and doesn't add it to SSR externals. Proper ESM packages are externalized now.

IIUC, @benmccann's PR to externalize by default would make this issue even more evident. @bluwy, users that want to use an ESM dependency with uncompiled .vue files should add it to ssr.noExternal, no? This isn't a bug in Vite, but an issue in the test setup. I'll add that to this PR.

[bluwy]

Yup I think adding to ssr.noExternal is a fair change, and likely something end-users would do too. It's not common though since Vue libraries are often pre-compiled. In Svelte land, vite-plugin-svelte auto-adds Svelte libraries to ssr.noExternal by checking the pkg.svelte key (which isn't the most elegant solution).