feat(server-utils): Warn when bundler config has instrumented module in external#22379
feat(server-utils): Warn when bundler config has instrumented module in external#22379timfish wants to merge 1 commit into
Conversation
| const externalizedModules = moduleNames.filter(name => | ||
| external.some(entry => externalEntryMatchesModule(entry, name)), |
There was a problem hiding this comment.
Bug: The code doesn't handle RegExp objects in Vite's ssr.external configuration, causing a TypeError when startsWith() is called on a RegExp and crashing the build.
Severity: HIGH
Suggested Fix
Before calling externalEntryMatchesModule, add a type guard to ensure the entry from the external array is a string. Regular expressions should be handled separately or skipped, similar to how the Webpack bundler implementation handles them. For example, wrap the call in an if (typeof entry === 'string') block.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: packages/server-utils/src/orchestrion/bundler/vite.ts#L43-L44
Potential issue: The code in `vite.ts` processes the `ssr.external` configuration
option, which can be an array of strings and/or `RegExp` objects according to Vite's
documentation. The implementation iterates through this array and passes each `entry` to
the `externalEntryMatchesModule` function. This helper function incorrectly assumes the
`entry` is always a string and calls `.startsWith()` on it. If a user configures
`ssr.external` with a `RegExp` (e.g., `[/^mysql/]`), the code will attempt to call
`.startsWith()` on the `RegExp` object, resulting in a `TypeError: entry.startsWith is
not a function` and causing the build process to crash.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
Not a valid issue. ssr.external is typed string[] | true in every Vite major from 6 through 8 (current latest 8.1.5) — RegExp is only accepted in ssr.noExternal, which this code never reads. The same holds for the Environment API successor, resolve.external. Since the Array.isArray check on line 42 already filters out true/undefined, every entry reaching externalEntryMatchesModule is a string per Vite's API contract, so the described TypeError cannot occur with a valid config. The finding conflates the external and noExternal signatures.
If users have a library we instrument in their bundler
externalconfig we should warn them about the fact that this stops it from getting instrumented!