fix(vite): broaden checks for existing client input#4077
fix(vite): broaden checks for existing client input#4077RihanArfan wants to merge 2 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughReplaced validation-based detection of a configured client entry with a direct boolean check that treats the presence of either Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/build/vite/plugin.ts (1)
101-104: Consider a more robust emptiness check to handle edge cases.The change correctly fixes the duplicate entry issue by detecting non-
indexkeyed inputs like{ main: "virtual:..." }thatgetEntry()would miss. However, using!!coercion treats empty objects{}and empty arrays[]as "configured" (both are truthy in JavaScript), which could skip auto-detection when no actual entry exists.Suggested more robust check
- let clientEntryConfigured = !!( - userConfig.environments?.client?.build?.rolldownOptions?.input || - userConfig.environments?.client?.build?.rollupOptions?.input - ); + const clientInput = + userConfig.environments?.client?.build?.rolldownOptions?.input || + userConfig.environments?.client?.build?.rollupOptions?.input; + let clientEntryConfigured = + typeof clientInput === "string" + ? clientInput.length > 0 + : Array.isArray(clientInput) + ? clientInput.length > 0 + : clientInput != null && Object.keys(clientInput).length > 0;Alternatively, a simpler approach could use the existing
getEntry()but extend it to handle any object key (not justindex):function getEntry(input: InputOption | undefined): string | undefined { if (typeof input === "string") { return input; } else if (Array.isArray(input) && input.length > 0) { return input[0]; - } else if (input && "index" in input) { - return input.index as string; + } else if (input && typeof input === "object") { + const values = Object.values(input); + return values.length > 0 ? (values[0] as string) : undefined; } }This would make
getEntry()work with{ main: "..." }inputs, allowing the original pattern!!getEntry(...)to work while rejecting empty objects.Based on learnings: Changes in
src/buildaffect build output and require careful review for backwards compatibility.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/build/vite/plugin.ts` around lines 101 - 104, The current clientEntryConfigured uses !! coercion which treats empty arrays/objects as configured; change it to explicitly check that the rollup/rolldown input is non-empty: locate the clientEntryConfigured assignment and replace the boolean coercion with a check that reads the input value (from userConfig.environments?.client?.build?.rolldownOptions?.input || userConfig.environments?.client?.build?.rollupOptions?.input) and returns true only if it's a non-empty string, a non-empty array (length > 0), or an object with at least one own enumerable key (Object.keys(...).length > 0); alternatively extend getEntry(...) to recognize any object key (not only "index") and then use !!getEntry(...) as before, referencing getEntry, clientEntryConfigured, and userConfig.environments?.client?.build in your change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/build/vite/plugin.ts`:
- Around line 101-104: The current clientEntryConfigured uses !! coercion which
treats empty arrays/objects as configured; change it to explicitly check that
the rollup/rolldown input is non-empty: locate the clientEntryConfigured
assignment and replace the boolean coercion with a check that reads the input
value (from userConfig.environments?.client?.build?.rolldownOptions?.input ||
userConfig.environments?.client?.build?.rollupOptions?.input) and returns true
only if it's a non-empty string, a non-empty array (length > 0), or an object
with at least one own enumerable key (Object.keys(...).length > 0);
alternatively extend getEntry(...) to recognize any object key (not only
"index") and then use !!getEntry(...) as before, referencing getEntry,
clientEntryConfigured, and userConfig.environments?.client?.build in your
change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4e519234-780f-48bb-a6a8-e4d9dd0f1752
📒 Files selected for processing (1)
src/build/vite/plugin.ts
commit: |
1 participant
🔗 Linked issue
Closes #4069
❓ Type of change
📚 Description
Removes using
getEntry()since we don't need the exact input, just to know whether an existing client entrypoint is configured.📝 Checklist