fix(vite): pre-bundle CJS dependencies#104
fix(vite): pre-bundle CJS dependencies#104ericelliott wants to merge 1 commit intozachwinter:masterfrom
Conversation
ericelliott
commented
Jan 16, 2026
- Filter out non-existent 'phone' dependency
- Add socket.io-client and music-metadata to optimizeDeps
- Resolves ESM/CJS module resolution errors
- Filter out non-existent 'phone' dependency - Add socket.io-client and music-metadata to optimizeDeps - Resolves ESM/CJS module resolution errors
Summary of ChangesHello @ericelliott, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refines the Vite configuration to resolve module resolution issues, particularly those arising from CommonJS dependencies. By explicitly including Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request updates the Vite configuration to pre-bundle CJS dependencies, resolving module resolution errors. It correctly filters out a non-existent 'phone' dependency and adds 'socket.io-client' and 'music-metadata' to optimizeDeps. My review includes two main points: first, a high-severity issue regarding the music-metadata package which is included for optimization but is missing from package.json. Second, a medium-severity suggestion to improve code readability by refactoring the dependency array construction.
| "dayjs/locale/en", | ||
| "dayjs/esm/locale/en", | ||
| "socket.io-client", | ||
| "music-metadata" |
There was a problem hiding this comment.
The music-metadata package is being added to optimizeDeps.include but is not declared as a project dependency in package.json. This can lead to build failures in clean environments or unpredictable behavior if a transitive dependency version changes. To ensure a stable and predictable build, please add music-metadata to the dependencies in package.json.
| // Fix dayjs ES module issue from AppKit - merge configs properly | ||
| // Pre-bundle packages with CommonJS dependencies so Vite converts them to ESM | ||
| return { | ||
| ...baseConfig, | ||
| optimizeDeps: { | ||
| ...baseConfig.optimizeDeps, | ||
| include: [...(baseConfig.optimizeDeps?.include || []), "dayjs", "dayjs/locale/en", "dayjs/esm/locale/en"] | ||
| include: [ | ||
| ...(baseConfig.optimizeDeps?.include || []).filter(dep => dep !== 'phone'), | ||
| "dayjs", | ||
| "dayjs/locale/en", | ||
| "dayjs/esm/locale/en", | ||
| "socket.io-client", | ||
| "music-metadata" | ||
| ] | ||
| } | ||
| }; |
There was a problem hiding this comment.
For better readability and to avoid a long inline expression, you could extract the logic for filtering base dependencies into a constant. This makes the include array's composition clearer.
// Fix dayjs ES module issue from AppKit - merge configs properly
// Pre-bundle packages with CommonJS dependencies so Vite converts them to ESM
const baseIncludes = (baseConfig.optimizeDeps?.include || []).filter(
(dep) => dep !== "phone"
);
return {
...baseConfig,
optimizeDeps: {
...baseConfig.optimizeDeps,
include: [
...baseIncludes,
"dayjs",
"dayjs/locale/en",
"dayjs/esm/locale/en",
"socket.io-client",
"music-metadata"
]
}
};