-
Notifications
You must be signed in to change notification settings - Fork 10.4k
feat: optimize webpack bundling to reduce server chunk duplication #23437
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Add modularizeImports for FeaturesRepository and other Repository classes - Configure webpack splitChunks with cacheGroups for repositories, features, prisma, and shared libraries - Enable usedExports and sideEffects optimization for better tree-shaking - Addresses massive code duplication issue where FeaturesRepository appears in 66+ chunks Co-Authored-By: [email protected] <[email protected]>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Walkthrough
Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
… and @calcom/app-store - Add modularizeImports configurations for all @calcom/features submodules (bookings, eventtypes, insights, webhooks, auth, shell, ee) - Add modularizeImports for @calcom/lib utilities (server, hooks, general lib) - Add modularizeImports for @calcom/app-store components (high priority optimization) - Expand splitChunks cacheGroups to include features, lib, and app-store packages - Fix LineChart export error in insights/components/index.ts (remove stale export) - Optimize chunk hierarchy with proper priority levels: app-store (28) > repositories (30) > features (25) > lib (22) > prisma (20) > shared (15) Co-Authored-By: [email protected] <[email protected]>
The latest updates on your projects. Learn more about Vercel for GitHub. |
…tructures - Remove problematic @calcom/features/bookings modularizeImports entry that was causing 'Module not found: Can't resolve @calcom/features/bookings/AvailableTimes' error - Remove @calcom/features/eventtypes and @calcom/features/insights entries that don't match actual export patterns - Keep only component-level modularizeImports for @calcom/features/eventtypes/components and @calcom/features/insights/components - Maintain webpack optimization benefits while fixing import resolution issues - Verified with yarn type-check:ci --force passing successfully Co-Authored-By: [email protected] <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
🧹 Nitpick comments (1)
apps/web/next.config.js (1)
229-293
: Minor: keep modularizeImports keys unique and sorted for diff hygieneSorting improves maintainability and reduces accidental duplicates.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/web/next.config.js
(2 hunks)packages/features/insights/components/index.ts
(0 hunks)
💤 Files with no reviewable changes (1)
- packages/features/insights/components/index.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
apps/web/next.config.js
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (1)
apps/web/next.config.js (1)
310-364
: Fix/verify production build and rerun chunk metrics
The build is failing with “fastqueue concurrency must be greater than 1.” Ensure a successful production build (for example, tryyarn build --network-concurrency 4
ornpm run build
), then rerun the chunk‐count script and share the before/after numbers to confirm chunk reductions and analyzer stability.
"@calcom/features/flags": { | ||
transform: "@calcom/features/flags/{{member}}", | ||
skipDefaultConversion: true, | ||
preventFullImport: true, | ||
}, | ||
"@calcom/features/users": { | ||
transform: "@calcom/features/users/{{member}}", | ||
skipDefaultConversion: true, | ||
preventFullImport: true, | ||
}, | ||
"@calcom/features/watchlist": { | ||
transform: "@calcom/features/watchlist/{{member}}", | ||
skipDefaultConversion: true, | ||
preventFullImport: true, | ||
}, | ||
"@calcom/features/calendar-cache": { | ||
transform: "@calcom/features/calendar-cache/{{member}}", | ||
skipDefaultConversion: true, | ||
preventFullImport: true, | ||
}, | ||
"@calcom/features/platform-oauth-client": { | ||
transform: "@calcom/features/platform-oauth-client/{{member}}", | ||
skipDefaultConversion: true, | ||
preventFullImport: true, | ||
}, | ||
"@calcom/lib/server/repository": { | ||
transform: "@calcom/lib/server/repository/{{member}}", | ||
skipDefaultConversion: true, | ||
preventFullImport: true, | ||
}, | ||
"@calcom/features/eventtypes/components": { | ||
transform: "@calcom/features/eventtypes/components/{{member}}", | ||
skipDefaultConversion: true, | ||
preventFullImport: true, | ||
}, | ||
"@calcom/features/insights/components": { | ||
transform: "@calcom/features/insights/components/{{member}}", | ||
skipDefaultConversion: true, | ||
preventFullImport: true, | ||
}, | ||
"@calcom/features/webhooks/components": { | ||
transform: "@calcom/features/webhooks/components/{{member}}", | ||
skipDefaultConversion: true, | ||
preventFullImport: true, | ||
}, | ||
"@calcom/lib/server": { | ||
transform: "@calcom/lib/server/{{member}}", | ||
skipDefaultConversion: true, | ||
preventFullImport: true, | ||
}, | ||
"@calcom/lib/hooks": { | ||
transform: "@calcom/lib/hooks/{{member}}", | ||
skipDefaultConversion: true, | ||
preventFullImport: true, | ||
}, | ||
"@calcom/lib": { | ||
transform: "@calcom/lib/{{member}}", | ||
skipDefaultConversion: true, | ||
preventFullImport: true, | ||
}, | ||
"@calcom/app-store": { | ||
transform: "@calcom/app-store/{{member}}", | ||
skipDefaultConversion: true, | ||
preventFullImport: true, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Block full/default imports now; scan repo for offenders and auto-fix
preventFullImport + skipDefaultConversion will break default/namespace imports from these packages. Run a quick scan and codemod before merging.
I can generate a jscodeshift to convert default/namespace imports to named imports if you want.
🏁 Script executed:
#!/bin/bash
# Find default or namespace imports from modularized sources
PKGS='@calcom/features/(flags|users|watchlist|calendar-cache|platform-oauth-client|eventtypes/components|insights/components|webhooks/components)|@calcom/lib(/server/repository|/server|/hooks)?|@calcom/app-store'
rg -nP --type=ts --type=tsx --type=js --type=jsx \
"import\s+(?!\{\s*[^}]+\s*\})[^;]+from\s+\"($PKGS)\"" \
-g '!**/node_modules/**' -S
Length of output: 573
🏁 Script executed:
#!/bin/bash
# Re-run search including TSX by defining file types explicitly
PKGS='@calcom/features/(flags|users|watchlist|calendar-cache|platform-oauth-client|eventtypes/components|insights/components|webhooks/components)|@calcom/lib(/server/repository|/server|/hooks)?|@calcom/app-store'
rg --type-add 'tsx:*.tsx' -nP \
"import\s+(?!\{\s*[^}]+\s*\})[^;]+from\s+\"($PKGS)\"" \
-g '*.ts' -g '*.tsx' -g '*.js' -g '*.jsx' -g '!**/node_modules/**' -S
Length of output: 656
Convert blocked namespace imports before merging
– tests/libs/mocks/app-store.ts:4 uses import type * as appStore from "@calcom/app-store"
; this namespace import will break under preventFullImport
. Update to explicit named imports or adjust the mock accordingly.
🤖 Prompt for AI Agents
In apps/web/next.config.js around lines 229-293 the module rule sets
preventFullImport for "@calcom/app-store", but tests/libs/__mocks__/app-store.ts
currently does "import type * as appStore from '@calcom/app-store'", which will
break the transform; open tests/libs/__mocks__/app-store.ts and replace the
namespace import with explicit named imports (or named type imports) that match
the actual exports from @calcom/app-store (e.g. import type { X, Y } from
"@calcom/app-store" or import { x, y } from "@calcom/app-store"); alternatively,
if changing the mock is undesirable, remove or adjust the preventFullImport
setting for "@calcom/app-store" in next.config.js so namespace imports remain
allowed—pick one approach and update the code accordingly, ensuring tests import
only the exported members.
apps/web/next.config.js
Outdated
usedExports: true, | ||
sideEffects: false, | ||
splitChunks: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sideEffects: false disables tree-shaking; set to true
With webpack 5, optimization.sideEffects=false turns the feature off. You want it on to dedupe/treeshake.
- sideEffects: false,
+ sideEffects: true,
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
usedExports: true, | |
sideEffects: false, | |
splitChunks: { | |
usedExports: true, | |
sideEffects: true, | |
splitChunks: { |
🤖 Prompt for AI Agents
In apps/web/next.config.js around lines 312 to 314, optimization.sideEffects is
currently set to false which disables tree-shaking in webpack 5; change that
property to true (or remove the explicit setting so webpack uses the
package.json "sideEffects" field) to re-enable treeshaking/deduplication, and
ensure your package.json "sideEffects" array/flag is accurate for modules that
must be preserved.
apps/web/next.config.js
Outdated
test: /[\\/]packages[\\/].*repository\.(ts|js)$/, | ||
name: "repositories", | ||
chunks: "all", | ||
priority: 30, | ||
enforce: true, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Repository regex misses “Repository.ts/tsx/jsx” and upper-case
Case-sensitive ".+repository.(ts|js)$" won’t match typical "UserRepository.ts". Include case-insensitive and jsx/tsx.
- test: /[\\/]packages[\\/].*repository\.(ts|js)$/,
+ test: /[\\/](?:packages|node_modules[\\/]@calcom)[\\/].*Repository\.(t|j)sx?$/i,
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
test: /[\\/]packages[\\/].*repository\.(ts|js)$/, | |
name: "repositories", | |
chunks: "all", | |
priority: 30, | |
enforce: true, | |
}, | |
test: /[\\/](?:packages|node_modules[\\/]@calcom)[\\/].*Repository\.(t|j)sx?$/i, | |
name: "repositories", | |
chunks: "all", | |
priority: 30, | |
enforce: true, | |
}, |
🤖 Prompt for AI Agents
In apps/web/next.config.js around lines 319 to 324, the splitChunks "test" regex
only matches lowercase "repository.(ts|js)" and misses files like
"UserRepository.ts" and .tsx/.jsx variants; update the test to include tsx and
jsx extensions and make it case-insensitive (or match both upper/lower
'Repository') so it matches files like
Repository.tsx/Repository.jsx/UserRepository.ts, e.g. change the pattern to
include (ts|tsx|js|jsx) and apply the case-insensitive flag or character class
for the 'R' in Repository.
apps/web/next.config.js
Outdated
test: /[\\/]packages[\\/]features[\\/]/, | ||
name: "features", | ||
chunks: "all", | ||
priority: 25, | ||
enforce: true, | ||
}, | ||
lib: { | ||
test: /[\\/]packages[\\/]lib[\\/]/, | ||
name: "lib", | ||
chunks: "all", | ||
priority: 22, | ||
enforce: true, | ||
}, | ||
appStore: { | ||
test: /[\\/]packages[\\/]app-store[\\/]/, | ||
name: "app-store", | ||
chunks: "all", | ||
priority: 28, | ||
enforce: true, | ||
}, | ||
prisma: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
cacheGroups may not match pnpm/node_modules paths
When using transpilePackages, module resources can resolve under node_modules/@calcom rather than packages/*. Broaden patterns.
- test: /[\\/]packages[\\/]features[\\/]/,
+ test: /[\\/](?:packages|node_modules[\\/]@calcom)[\\/]features[\\/]/,
@@
- test: /[\\/]packages[\\/]lib[\\/]/,
+ test: /[\\/](?:packages|node_modules[\\/]@calcom)[\\/]lib[\\/]/,
@@
- test: /[\\/]packages[\\/]app-store[\\/]/,
+ test: /[\\/](?:packages|node_modules[\\/]@calcom)[\\/]app-store[\\/]/,
@@
- test: /[\\/]packages[\\/]prisma[\\/]|[\\/]node_modules[\\/]@prisma[\\/]/,
+ test: /[\\/](?:packages[\\/]prisma|node_modules[\\/]@prisma)[\\/]/,
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
test: /[\\/]packages[\\/]features[\\/]/, | |
name: "features", | |
chunks: "all", | |
priority: 25, | |
enforce: true, | |
}, | |
lib: { | |
test: /[\\/]packages[\\/]lib[\\/]/, | |
name: "lib", | |
chunks: "all", | |
priority: 22, | |
enforce: true, | |
}, | |
appStore: { | |
test: /[\\/]packages[\\/]app-store[\\/]/, | |
name: "app-store", | |
chunks: "all", | |
priority: 28, | |
enforce: true, | |
}, | |
prisma: { | |
test: /[\\/](?:packages|node_modules[\\/]@calcom)[\\/]features[\\/]/, | |
name: "features", | |
chunks: "all", | |
priority: 25, | |
enforce: true, | |
}, | |
lib: { | |
test: /[\\/](?:packages|node_modules[\\/]@calcom)[\\/]lib[\\/]/, | |
name: "lib", | |
chunks: "all", | |
priority: 22, | |
enforce: true, | |
}, | |
appStore: { | |
test: /[\\/](?:packages|node_modules[\\/]@calcom)[\\/]app-store[\\/]/, | |
name: "app-store", | |
chunks: "all", | |
priority: 28, | |
enforce: true, | |
}, | |
prisma: { | |
test: /[\\/](?:packages[\\/]prisma|node_modules[\\/]@prisma)[\\/]/, |
🤖 Prompt for AI Agents
In apps/web/next.config.js around lines 326 to 346, the cacheGroups test regexes
only match files under packages/* and miss modules that are resolved into
node_modules (e.g. node_modules/@calcom/...) when using transpilePackages;
update each cacheGroups.test to broaden the pattern to match both packages/* and
node_modules paths (for example include alternatives for node_modules and scoped
packages like @calcom) so resources resolved under node_modules/@calcom/... will
be captured by the same group.
- Remove sideEffects: false setting that was causing JavaScript heap out of memory errors - Remove overlapping lib, appStore, and shared cacheGroups that were conflicting - Keep essential optimizations: repositories, features, and prisma cacheGroups - Maintain FeaturesRepository reduction from 40 to 3 instances Addresses CodeRabbit feedback on webpack configuration issues Co-Authored-By: [email protected] <[email protected]>
- Enhanced repositories cacheGroup regex to match both packages/ and node_modules/@calcom/ paths - Improved features cacheGroup regex for better module resolution - Added case-insensitive matching and support for .tsx/.jsx extensions - Addresses CodeRabbit feedback for more comprehensive webpack optimization Co-Authored-By: [email protected] <[email protected]>
What does this PR do?
This PR optimizes webpack bundling configuration to address massive server chunk duplication where
FeaturesRepository
and other classes were being compiled into 66+ separate chunks, causing significant bundle bloat and potential performance issues.Key optimizations:
modularizeImports
for Repository classes and feature modules to enable better tree-shakingsplitChunks
with prioritizedcacheGroups
to deduplicate repositories, features, prisma, and shared librariesusedExports
andsideEffects
optimization for improved dead code eliminationAddresses the issue where:
FeaturesRepository
appeared in 66+ chunk filesPrismaClient
references in 245+ filesVisual Demo
Before: Loading
/event-types
route generated 785+ server chunks with massive duplicationExpected After: Consolidated chunks with deduplicated Repository and Service classes
Note: Visual verification requires building and analyzing the chunk output after these changes
Mandatory Tasks
How should this be tested?
Prerequisites:
.next/server/chunks
directory before testingTesting steps:
yarn build
and verify no build errors.next/server/chunks/
before/after/event-types
route and analyze server chunks for duplicationANALYZE=true yarn build
(should no longer crash with OOM)Expected results:
FeaturesRepository
should appear in far fewer chunksChecklist
Link to Devin run: https://app.devin.ai/sessions/161bc92f2a52453096d2e1f07382b9ab
Requested by: @keithwillcode
Review Focus Areas
splitChunks.cacheGroups
regex patterns are correctmodularizeImports