SSR external packages with entries break when not hoisted

[mattcompiles]

Describe the bug

When marking a package as an SSR external, it should also mark any sub-entry points of the same module as external.
While it's not explicitly documented, this is what I believe the intended behaviour is from reading prior issues and the source code.

e.g.

// vite.config.js
export default {
  ssr: {
    external: ['lodash']
  }
}

// should be external
import lodash from 'lodash';

// should also be external
import omit from 'lodash/omit';

Unfortunately, this feature (sub-entry externals) doesn't work when the package being marked as external isn't a top-level package inside node_modules. Package managers that use hoisting likely get around this issue, however pnpm and other package manages that don't hoist seem to break this feature.

In practice, this means that the behaviour of ssr.external is different depending on whether packages are top-level dependencies or not. Due to the way ssr.noExternal works, using ssr.external on nested deps is essential in more complex scenarios.

After debugging the source code, this issue seems to occur because import specifiers that exactly match the external are treated differently to package matches. Package matches have to go through a node resolve check in order to be externalised, however the resolve check runs from the project root, therefore always failing for non top-level packages.

Excerpt from the source code:

// copied from https://github.com/vitejs/vite/blob/9ac5075825212e5f4f3b262225ff4a3e46b8e8d1/packages/vite/src/node/ssr/ssrExternal.ts#L158

  // Returns true if it is configured as external, false if it is filtered
  // by noExternal and undefined if it isn't affected by the explicit config
  return (id: string) => {
    const { ssr } = config
    if (ssr) {
      if (
        // If this id is defined as external, force it as external
        // Note that individual package entries are allowed in ssr.external
        ssr.external?.includes(id)
      ) { 
        return true // <-- straight external match doesn't require more validation
      }
      const pkgName = getNpmPackageName(id)
      if (!pkgName) {
        return isExternalizable(id)
      }
      if (
        // A package name in ssr.external externalizes every
        // externalizable package entry
        ssr.external?.includes(pkgName)
      ) {
        return isExternalizable(id, true) // <-- package entry external match requires resolve check for some reason?
      }
      if (typeof noExternal === 'boolean') {
        return !noExternal
      }
      if (noExternalFilter && !noExternalFilter(pkgName)) {
        return false
      }
    }
    return isExternalizable(id)
  }

I have validated that changing the package entry check above to immediately return true rather than going through isExternalizable fixes this issue. Unless this breaks other Vite behaviour, I'd suggest that would be the easiest fix and I'd be happy to submit a PR doing this.

Alternatively, the node resolve algorithm inside isExternalizable could resolve from the dependent package location rather than the project root.

I think this issue is related to but not the same as #9710.

Reproduction

https://github.com/mattcompiles/vite-ssr-external-repro

System Info

System:
    OS: macOS 12.5
    CPU: (10) arm64 Apple M1 Pro
    Memory: 609.50 MB / 32.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 16.15.0 - ~/.volta/tools/image/node/16.15.0/bin/node
    npm: 8.5.5 - ~/.volta/tools/image/node/16.15.0/bin/npm
    pnpm: 7.9.5
  Browsers:
    Chrome: 104.0.5112.101
    Firefox: 104.0.1
    Safari: 15.6
  npmPackages:
    @vitejs/plugin-react: ^2.0.0 => 2.0.0
    vite: ^3.0.9 => 3.0.9

Used Package Manager

pnpm

Logs

Click to expand!

> ssr-external-repro@0.0.0 test /Users/mjones/projects/ssr-external-repro
> node test

node:internal/process/esm_loader:94
    internalBinding('errors').triggerUncaughtException(
                              ^

ReferenceError: require is not defined
    at eval (/node_modules/.pnpm/lodash@4.17.21/node_modules/lodash/mapValues.js:3:36)
    at instantiateModule (file:///Users/mjones/projects/ssr-external-repro/node_modules/.pnpm/vite@3.0.9/node_modules/vite/dist/node/chunks/dep-0fc8e132.js:50548:15)

Validations

• Follow our Code of Conduct
• Read the Contributing Guidelines.
• Read the docs.
• Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• Make sure this is a Vite issue and not a framework-specific issue. For example, if it's a Vue SFC related bug, it should likely be reported to vuejs/core instead.
• Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
• The provided reproduction is a minimal reproducible example of the bug.

[rtsao]

Alternatively, the node resolve algorithm inside isExternalizable could resolve from the dependent package location rather than the project root.

This is the fix implemented in #9763

[sapphi-red]

maybe related: #9549

[sapphi-red]

Closing as #9763 is merged.