refactor: simplify resolveSSRExternal
#5210
Conversation
- add `collectExternals` function for recursive tracing - apply `config.ssr` just once, after all externals are collected - support packages with deep imports but no main entry - remove `knownImports` argument
aleclarson
added
feat: ssr
p2-edge-case
This argument is necessary to ensure transient dependencies in node_modules are marked as external.
|
Okay, I realized why |
| const { root } = config | ||
| for (const dep of knownImports) { | ||
| // assume external if not yet seen | ||
| if (!seen.has(dep)) { |
There was a problem hiding this comment.
At this point, the project root and any linked packages have had their dependencies checked, so we can safely mark any knownImports not yet seen as external. They are guaranteed to be dependencies of packages in node_modules.
There was a problem hiding this comment.
should your comment here perhaps be a code comment?
| } | ||
| // check if the entry is cjs | ||
| } | ||
| // externalize js entries with commonjs |
There was a problem hiding this comment.
there changes are conflicting with the changes in https://github.com/vitejs/vite/pull/5197/files#diff-a6d6596967a09b96fa100c4b77480490b5ef95df88987ecbfa00ba1c868e4f53R97
rebased version is here: #5544
if you'd prefer to update this PR, I'm happy to close mine
collectExternalsfunction for recursive tracingconfig.ssrjust once, after all externals are collectedTo clarify #3, a package might allow imports like
foo/barbut notfooalone. Before this PR, packages like this were never marked external (unless done so explicitly in user config), because the call totryNodeResolvewould throw. As a workaround, you can add the package's name tossr.externalor add an emptyindex.jsmodule to the package.