chore(client): Migrate client config to esm - #545
Conversation
|
Warning Review limit reached
Next review available in: 8 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (8)
📝 WalkthroughWalkthroughClient config is migrated to ESM: the package declares ChangesES Module System Migration
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR migrates the client’s build/styling configuration to ESM and removes the CommonJS compatibility plugin from Vite.
Changes:
- Switch
tailwind.config.jsandpostcss.config.jsfrom CommonJS (module.exports/require) to ESM (import/export default). - Mark the client package as ESM via
"type": "module"and update Tailwind animate plugin usage to ESM import. - Remove
vite-plugin-commonjsfrom bothvite.config.tsandpackage.json.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| client/vite.config.ts | Drops vite-plugin-commonjs from Vite plugins (and adds Vitest type reference). |
| client/tailwind.config.js | Converts Tailwind config to ESM and switches tailwindcss-animate to an ESM import. |
| client/postcss.config.js | Converts PostCSS config to ESM export. |
| client/package.json | Sets ESM mode for the package and removes vite-plugin-commonjs dependency. |
Files not reviewed (1)
- client/package-lock.json: Language not supported
6d2c5b1 to
0f50f95
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@client/vite.config.ts`:
- Line 1: The file currently references Vitest types incorrectly; update the
triple-slash reference so the Vitest v4 config typings apply (so the test config
field is properly typed). Replace the top reference that points to "vitest" with
the v4 config reference used by Vitest ("vitest/config") and ensure defineConfig
(imported from Vite) and the test property use those updated types; verify the
test config autocompletes correctly for functions like defineConfig and the test
object.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a11b25b0-0007-4973-9ba8-05becea2980c
⛔ Files ignored due to path filters (1)
client/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
client/.eslintrc.cjsclient/eslint.config.mjsclient/eslint/package.jsonclient/package.jsonclient/postcss.config.mjsclient/tailwind.config.mjsclient/vite.config.ts
💤 Files with no reviewable changes (3)
- client/postcss.config.mjs
- client/package.json
- client/tailwind.config.mjs
✅ Files skipped from review due to trivial changes (1)
- client/.eslintrc.cjs
0f50f95 to
cd61a28
Compare
serendipty01
changed the title
2a0f7e5 to
361ef99
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- client/package-lock.json: Generated file
Comments suppressed due to low confidence (1)
client/postcss.config.mjs:1
client/.dockerignorecurrently whitelistspostcss.config.jsandtailwind.config.js(but not the new.mjsfilenames). Because the dockerignore starts with*, Docker builds that use this context will exclude these config files, which can break Tailwind/PostCSS during container builds. Updateclient/.dockerignoreto whitelistpostcss.config.mjsandtailwind.config.mjs(and drop the old.jsentries if the files are truly removed).
12da623 to
7ff57d3
Compare
Vite bundles the config through esbuild and shims __dirname/__filename for it, so this worked fine as-is (verified: npm run build succeeds both before and after). Making it explicit anyway per Copilot review, using the same fileURLToPath(import.meta.url) pattern already used in server/lib/cache/stores/RedisStore/RedisStore.ts, so the config doesn't rely on Vite's config-loader shim. Co-Authored-By: Claude <noreply@anthropic.com>
Context & Requests for Reviewers
fixes #546
Migrates the client's build configuration from CommonJS to native ES Modules:
tailwind.config.js→tailwind.config.mjsandpostcss.config.js→postcss.config.mjs—module.exportsbecomesexport default, andrequire('tailwindcss-animate')becomes a real ESM import.client/package.jsongains"type": "module", so every.jsfile in the package has one unambiguous module format. The in-repo ESLint rules folder gets its ownpackage.jsonwith"type": "commonjs"as a shield for the still-CJSeslint/index.js.vite-plugin-commonjsdependency — Vite's built-inoptimizeDepshandles CJS interop natively, so the plugin was redundant (one less dependency to audit and keep compatible with Vite upgrades)./// <reference types="vitest/config" />directive tovite.config.ts, fixing a previously-masked TypeScript error on thetestconfig block.Why: ESM configs are the direction the ecosystem has settled on (Vite, Tailwind 4, and most tooling are ESM-first), the server package is already
"type": "module", and with the flag set a strayrequire()now fails loudly (ERR_REQUIRE_ESM) instead of behaving differently per tool. This also clears the path for future Vite/Tailwind major upgrades that assume ESM configs.Tests
Automated (all at branch head):
npm run build✓,npm run lint0 errors, test suite shows only failures that also fail on pristinemainrg "require\(" client/src --glob '*.js'→ onlysrc/rechartsScaleWrapper.js, which is dead code onmaintoo (its alias never fires; verified absent from the build output) — no regression from removingvite-plugin-commonjs*.config.jsnamesManual, on a fresh environment (volumes dropped, all migrations re-applied, dev servers):
animate-in fade-incomputesanimation-name: enter, 0.15s— direct proof thetailwindcss-animateESM default import resolved to a real plugin (the riskiest line in this PR; had it resolved toundefined, Tailwind would silently drop all animation utilities)🤖 AI Usage Disclosure
Generated with Claude Code
Summary by CodeRabbit
Summary by CodeRabbit