-
Notifications
You must be signed in to change notification settings - Fork 2.3k
[WEB-3966] chore: update dependencies and optimize dynamic imports #7132
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: preview
Are you sure you want to change the base?
Conversation
…ponents - Updated various dependencies in package.json and yarn.lock. - Refactored layout components to dynamically import heavy components for improved performance. - Enhanced webpack configuration for better chunk splitting and optimization.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis update introduces dynamic imports for several UI components, particularly chart/graph modules and the CommandPalette, deferring their loading to the client side for improved performance. The Next.js configuration is enhanced with advanced bundle optimization, chunk splitting, SVG handling, and integration of the bundle analyzer plugin. Minor refactoring and dependency updates are also included. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Next.js App
participant Client Browser
participant CommandPalette
participant GraphComponent
User->>Next.js App: Request page
Next.js App->>Client Browser: Serve initial HTML/JS (without CommandPalette/Graph)
Client Browser->>Next.js App: Hydrate page
Client Browser->>CommandPalette: Dynamically import on demand (SSR disabled)
Client Browser->>GraphComponent: Dynamically import on demand (SSR disabled)
CommandPalette-->>Client Browser: Render component (after load)
GraphComponent-->>Client Browser: Render chart (after load)
Poem
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Pull Request Overview
This PR refactors layout and graph components to use dynamic imports for performance, updates several build dependencies, and enhances the Next.js bundling configuration.
- Added
@next/bundle-analyzer
and wrapped the Next.js config to enable bundle analysis. - Overhauled
next.config.js
to improve tree-shaking and chunk splitting. - Converted chart and command-palette imports to
next/dynamic
, removing static exports.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
web/package.json | Added @next/bundle-analyzer to devDependencies . |
web/next.config.js | Wrapped config with analyzer, added turbo rules, alias, and custom webpack.optimization . |
web/core/components/ui/graphs/index.tsx | Introduced dynamic imports for heavy chart components with a shared loading spinner. |
web/core/components/ui/graphs/index.ts | Removed static re-exports, leaving an empty file (should be deleted or repurposed). |
web/app/(all)/profile/layout.tsx | Dynamic-loaded CommandPalette and adjusted container height classes. |
web/app/(all)/[workspaceSlug]/(projects)/layout.tsx | Replaced static CommandPalette with dynamic import in project layout. |
Comments suppressed due to low confidence (4)
web/app/(all)/[workspaceSlug]/(projects)/layout.tsx:18
- The
React
namespace isn't imported, so usingReact.ReactNode
will fail. Addimport type { ReactNode } from 'react'
or update the signature accordingly.
export default function WorkspaceLayout({ children }: { children: React.ReactNode }) {
web/core/components/ui/graphs/index.tsx:65
- Invalid TypeScript syntax for re-exporting all types. Use
export * from "./types.d";
or explicitly list types withexport type { ... } from "./types.d";
.
export type * from "./types.d";
web/app/(all)/profile/layout.tsx:16
- Using
React.ReactNode
without importingReact
or theReactNode
type will cause a TS error. Importtype { ReactNode } from 'react'
or adjust to match your JSX transform settings.
export default function ProfileLayout({ children }: { children: React.ReactNode }) {
@sriramveeraghanta I experimented with bundle optimization and discovered some significant opportunities for improvement. Our initial page load was quite heavy (~2.68 MB) because many of our heavier dependencies (Nivo charts, command palette, etc.) weren't being lazy loaded. I implemented dynamic imports using
Results: Reduced initial bundle size from 2.68 MB to 140 kB (~95% reduction) 🎯 This is just a proof of concept to demonstrate the potential impact - not suggesting we merge as-is, but wanted to show what's possible with proper lazy loading strategy. The heavy libraries now only load when actually needed (e.g., charts only load on analytics pages). Happy to discuss the approach and refine it if this direction looks promising! |
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: 1
🧹 Nitpick comments (1)
web/core/components/ui/graphs/index.tsx (1)
4-62
: Consider extracting the loading component to reduce duplication.While the dynamic import implementation is excellent, the loading spinner is duplicated across all 5 components. Consider extracting it to improve maintainability.
Create a shared loading component:
+// Shared loading component for charts +const ChartLoadingSpinner = () => ( + <div className="flex h-96 w-full items-center justify-center"> + <div className="h-8 w-8 animate-spin rounded-full border-2 border-custom-primary border-t-transparent" /> + </div> +); + export const BarGraph = dynamic( () => import("./bar-graph").then(module => ({ default: module.BarGraph })), { ssr: false, - loading: () => ( - <div className="flex h-96 w-full items-center justify-center"> - <div className="h-8 w-8 animate-spin rounded-full border-2 border-custom-primary border-t-transparent" /> - </div> - ), + loading: ChartLoadingSpinner, } );Apply the same pattern to all chart components.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (6)
web/app/(all)/[workspaceSlug]/(projects)/layout.tsx
(1 hunks)web/app/(all)/profile/layout.tsx
(1 hunks)web/core/components/ui/graphs/index.ts
(0 hunks)web/core/components/ui/graphs/index.tsx
(1 hunks)web/next.config.js
(3 hunks)web/package.json
(2 hunks)
💤 Files with no reviewable changes (1)
- web/core/components/ui/graphs/index.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
web/app/(all)/[workspaceSlug]/(projects)/layout.tsx (1)
web/core/components/command-palette/command-palette.tsx (1)
CommandPalette
(39-272)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (12)
web/package.json (1)
81-81
:✅ Verification successful
Verify the bundle analyzer version and security.
The addition of
@next/bundle-analyzer
aligns well with the PR's bundle optimization objectives. However, please verify that version^15.3.2
is current and free from security vulnerabilities.
🏁 Script executed:
#!/bin/bash # Description: Verify the @next/bundle-analyzer package version and security status # Check latest available version on npm curl -s https://registry.npmjs.org/@next/bundle-analyzer/latest | jq '.version' # Check for security advisories gh api graphql -f query=' { securityVulnerabilities(first: 5, ecosystem: NPM, package: "@next/bundle-analyzer") { nodes { advisory { summary severity publishedAt } vulnerableVersionRange firstPatchedVersion { identifier } } } }'Length of output: 443
Dependency version and security verified for @next/bundle-analyzer
Version ^15.3.2 is the latest release and there are no known security advisories against it. No further action needed—this dependency can be safely merged.
web/app/(all)/[workspaceSlug]/(projects)/layout.tsx (2)
3-3
: Good addition of dynamic import capability.Adding the
dynamic
import from Next.js enables the performance optimization for the CommandPalette component.
9-16
: Excellent performance optimization with appropriate configuration.The dynamic import implementation for CommandPalette is well-executed:
- SSR disabled: Correct choice since command palettes are interactive client-side components
- Null loading fallback: Appropriate since the component should load quickly
- Named export extraction: Properly handles the module structure
- Clear documentation: Comment explains the reasoning
This change will improve initial bundle size and loading performance.
web/app/(all)/profile/layout.tsx (3)
3-3
: Consistent dynamic import addition.Good consistency with the workspace layout by adding the same
dynamic
import capability.
7-14
: Consistent CommandPalette optimization across layouts.This dynamic import implementation perfectly matches the workspace layout pattern, ensuring consistent performance optimization across the application. The configuration choices (SSR disabled, null loading) are appropriate for the CommandPalette component.
16-28
: Clean layout structure improvements.The layout structure has been simplified appropriately while maintaining functionality. The function name change to
ProfileLayout
is more descriptive than the previous name.web/core/components/ui/graphs/index.tsx (2)
1-14
: Excellent performance optimization for heavy chart components.The dynamic import approach for Nivo charts is well-implemented:
- SSR disabled: Correct for client-side data visualizations
- Loading spinner: Provides good user feedback
- Consistent pattern: Easy to understand and maintain
64-65
: Clean type re-export approach.The type re-export is well-implemented and maintains type safety without runtime impact.
web/next.config.js (4)
6-10
: LGTM! Standard bundle analyzer setup.The bundle analyzer integration follows the standard pattern with proper conditional enabling.
45-48
: Good additions to optimized package imports.The inclusion of
recharts
,axios
,mobx
, andmobx-react
aligns well with the PR's optimization goals, especially for the dynamic chart imports mentioned in the summary.
143-147
: Good optimization for consistent JSX runtime resolution.The explicit alias for
react/jsx-runtime
ensures consistent module resolution across the build.
50-59
:✅ Verification successful
Verify Turbopack compatibility with webpack-style loader configuration.
The
turbo
configuration appears to use webpack-style loader syntax, but Turbopack (Next.js's Rust-based bundler) may not support this format. This configuration might be ignored or cause build errors when using Turbopack.Consider moving the SVG handling to the webpack configuration section instead, or verify that this syntax is supported in your Next.js version.
🌐 Web query:
Does Next.js Turbopack support webpack-style loader configuration for SVG files using @svgr/webpack?
💡 Result:
Next.js Turbopack Support for Webpack-Style SVG Loader Configuration Using @svgr/webpack
Short Answer:
Yes, Next.js Turbopack supports configuring Webpack-style loaders for SVG files using@svgr/webpack
. This can be accomplished in yournext.config.js
ornext.config.ts
by specifying rules that map SVG files to the@svgr/webpack
loader. However, there are some caveats and ongoing stability considerations you may need to note.
How to Configure @svgr/webpack Loader with Turbopack
Turbopack in Next.js is designed to support many popular webpack loaders, including
@svgr/webpack
, for use cases like importing SVG files as React components[1][5][6]. The configuration is similar in intent to Webpack, but uses theturbopack
key instead of the traditionalwebpack
function.Example Configuration (
next.config.js
):import type { NextConfig } from 'next'; const nextConfig: NextConfig = { turbopack: { rules: { '*.svg': { loaders: ['@svgr/webpack'], as: '*.js', }, }, }, }; export default nextConfig;This setup tells Turbopack to process any SVG file with the
@svgr/webpack
loader, converting SVGs to React components[1][5].
Supported and Verified Webpack Loaders
The following webpack loaders have been verified to work with Turbopack[1][6]:
@svgr/webpack
babel-loader
url-loader
file-loader
raw-loader
sass-loader
yaml-loader
string-replace-loader
Most other basic webpack loaders also work, as Turbopack supports a subset of the webpack loader API[6].
Stability, Limitations, and Known Issues
- Development Support: Turbopack's loader support is stable for local development (
next dev --turbopack
)[6][9].- Production Builds: As of Next.js 15.3 (May 2025), Turbopack production builds (
next build --turbopack
) are still in alpha[5][9]. Loader support may not be consistent in production builds yet.- Potential Issues: There have been some reports of integration issues, especially with shared UI libraries and server components using SVGs. Some developers have encountered errors such as "Element type is invalid" when importing SVGs as React components with Turbopack[2][3]. These issues may be resolved in ongoing releases, but you should test your setup thoroughly and consult the Next.js GitHub discussions for any workarounds or updates.
Summary Table: SVG Loader Support in Next.js Bundlers
Feature Webpack Turbopack (dev) Turbopack (build/alpha) Webpack-style @svgr/webpack
SVG loading✅ Fully stable ✅ Supported & stable ⚠️ Experimental/alphaConfiguration location webpack()
turbopack.rules
turbopack.rules
Known Issues Rare Possible (see notes) More likely, not fully stable
References & Next Steps
- You can find the official configuration documentation and list of supported loaders in the [Next.js Turbopack documentation][1][5][6][9].
- If you run into issues, check the [Next.js GitHub discussions][2][3] for the latest fixes, workarounds, and updates.
In summary: Turbopack supports
@svgr/webpack
for SVG loader configuration in a way similar to Webpack, especially for development. Setup is straightforward, but for production builds and complex scenarios, continue to monitor Next.js releases for the latest improvements and bug fixes[1][5][6][9].Citations:
- 1: https://nextjs.org/docs/app/api-reference/config/next-config-js/turbopack
- 2: How to integrate @svgr/webpack to make turbopack work? vercel/next.js#50337
- 3: Turbopack: Unable to load SVG. Uncaught Error: Element type is invalid: expected a string (for built-in components) or a class/function (for composite components) but got: object. vercel/next.js#66866
- 4: https://react-svgr.com/docs/next/
- 5: https://nextjs.org/blog/next-15-3
- 6: https://nextjs.org/blog/turbopack-for-development-stable
- 7: https://react-svgr.com/docs/webpack/
- 8: https://dev.to/maazakn/a-guide-to-effortlessly-use-svgs-in-next-js-projects-2ean
- 9: https://nextjs.org/docs/app/api-reference/turbopack
- 10: https://nextjs.org/docs/13/pages/api-reference/next-config-js/turbo
Turbopack supports webpack-style @svgr/webpack loader configuration
This
turbopack.rules
block is officially supported in Next.js 15.3+ and will correctly load SVGs via@svgr/webpack
in development:
- Your existing configuration under
turbopack.rules
is valid.- Loader support is stable for
next dev --turbo
.- Production builds with Turbopack (
next build --turbo
) are still in alpha—be sure to test your CI/CD pipelines if you rely on them.No changes are required here.
- Updated webpack settings to improve tree shaking and chunk splitting strategies for client-side production builds. - Increased maximum chunk size to reduce fragmentation and improve loading performance. - Adjusted cache groups for better management of framework and library chunks.
… components - Replaced inline loading divs with a shared LoadingSpinner component across all dynamic graph imports. - Ensured consistent loading behavior for BarGraph, PieGraph, LineGraph, CalendarGraph, and ScatterPlotGraph components.
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.
Pull Request Overview
This PR updates dependencies and refactors heavy components to load dynamically, alongside enhanced webpack optimizations for better performance.
- Added
@next/bundle-analyzer
and enabled build analysis via an environment flag. - Configured aggressive client-side optimizations and chunk splitting in
next.config.js
. - Refactored graph and layout components to use
next/dynamic
for client-only rendering.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
web/package.json | Added @next/bundle-analyzer to devDependencies |
web/next.config.js | Integrated bundle analyzer and tuned webpack optimization settings |
web/core/components/ui/graphs/index.tsx | Introduced dynamic imports for graph components with a loading spinner |
web/core/components/ui/graphs/index.ts | Removed legacy static re-exports |
web/app/(all)/profile/layout.tsx | Refactored to dynamically import CommandPalette and adjusted layout |
web/app/(all)/[workspaceSlug]/(projects)/layout.tsx | Refactored to dynamically import CommandPalette |
Comments suppressed due to low confidence (3)
web/app/(all)/[workspaceSlug]/(projects)/layout.tsx:18
- Referencing
React.ReactNode
without importing the React namespace. Addimport React from "react";
orimport type { ReactNode } from "react"
to ensure the type is resolved.
export default function WorkspaceLayout({ children }: { children: React.ReactNode }) {
web/core/components/ui/graphs/index.tsx:53
- Invalid TypeScript export syntax:
export type * from "./types.d";
. Useexport * from "./types.d";
or explicitly export types withexport type { YourType } from "./types.d"
.
export type * from "./types.d";
web/app/(all)/profile/layout.tsx:16
- Referencing
React.ReactNode
without importing the React namespace. Addimport React from "react";
orimport type { ReactNode } from "react"
to ensure the type is resolved.
export default function ProfileLayout({ children }: { children: React.ReactNode }) {
// Dynamically import heavy chart components (Nivo charts) | ||
export const BarGraph = dynamic( | ||
() => import("./bar-graph").then(module => ({ default: module.BarGraph })), | ||
{ | ||
ssr: false, // Charts are client-side only | ||
loading: () => <LoadingSpinner />, | ||
} | ||
); | ||
|
||
export const PieGraph = dynamic( | ||
() => import("./pie-graph").then(module => ({ default: module.PieGraph })), | ||
{ | ||
ssr: false, | ||
loading: () => <LoadingSpinner />, | ||
} | ||
); | ||
|
||
export const LineGraph = dynamic( | ||
() => import("./line-graph").then(module => ({ default: module.LineGraph })), | ||
{ | ||
ssr: false, | ||
loading: () => <LoadingSpinner />, | ||
} | ||
); | ||
|
||
export const CalendarGraph = dynamic( | ||
() => import("./calendar-graph").then(module => ({ default: module.CalendarGraph })), | ||
{ | ||
ssr: false, | ||
loading: () => <LoadingSpinner />, | ||
} | ||
); | ||
|
||
export const ScatterPlotGraph = dynamic( | ||
() => import("./scatter-plot-graph").then(module => ({ default: module.ScatterPlotGraph })), | ||
{ | ||
ssr: false, | ||
loading: () => <LoadingSpinner />, | ||
} | ||
); | ||
|
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.
[nitpick] Repeated dynamic import blocks for each graph component. Consider creating a helper function to reduce duplication and improve readability.
// Dynamically import heavy chart components (Nivo charts) | |
export const BarGraph = dynamic( | |
() => import("./bar-graph").then(module => ({ default: module.BarGraph })), | |
{ | |
ssr: false, // Charts are client-side only | |
loading: () => <LoadingSpinner />, | |
} | |
); | |
export const PieGraph = dynamic( | |
() => import("./pie-graph").then(module => ({ default: module.PieGraph })), | |
{ | |
ssr: false, | |
loading: () => <LoadingSpinner />, | |
} | |
); | |
export const LineGraph = dynamic( | |
() => import("./line-graph").then(module => ({ default: module.LineGraph })), | |
{ | |
ssr: false, | |
loading: () => <LoadingSpinner />, | |
} | |
); | |
export const CalendarGraph = dynamic( | |
() => import("./calendar-graph").then(module => ({ default: module.CalendarGraph })), | |
{ | |
ssr: false, | |
loading: () => <LoadingSpinner />, | |
} | |
); | |
export const ScatterPlotGraph = dynamic( | |
() => import("./scatter-plot-graph").then(module => ({ default: module.ScatterPlotGraph })), | |
{ | |
ssr: false, | |
loading: () => <LoadingSpinner />, | |
} | |
); | |
// Helper function to dynamically import graph components | |
const createDynamicGraph = (modulePath: string, componentName: string) => | |
dynamic( | |
() => | |
import(modulePath).then(module => ({ | |
default: module[componentName], | |
})), | |
{ | |
ssr: false, // Charts are client-side only | |
loading: () => <LoadingSpinner />, | |
} | |
); | |
// Dynamically import heavy chart components (Nivo charts) | |
export const BarGraph = createDynamicGraph("./bar-graph", "BarGraph"); | |
export const PieGraph = createDynamicGraph("./pie-graph", "PieGraph"); | |
export const LineGraph = createDynamicGraph("./line-graph", "LineGraph"); | |
export const CalendarGraph = createDynamicGraph("./calendar-graph", "CalendarGraph"); | |
export const ScatterPlotGraph = createDynamicGraph("./scatter-plot-graph", "ScatterPlotGraph"); |
Copilot uses AI. Check for mistakes.
29b6226
to
b4eab66
Compare
Pull Request Linked with Plane Work Items
Comment Automatically Generated by Plane |
We are no longer using Nivo charts, as we have switched to Recharts. I guess the old components have not yet been cleaned up. |
@JayashTripathy Can we clean up old analytics components in the PR you have created #7131 . |
Description
update dependencies and optimize dynamic imports in layout components
Type of Change
Screenshots and Media (if applicable)
Test Scenarios
References
Summary by CodeRabbit