UN-3185 [MISC]: Migrate frontend from Create React App to Vite #1607
Conversation
Migrate the Unstract frontend build system from Create React App (react-scripts 5.0.1) to Vite 6.0.5 for improved development experience and build performance. Key Changes: - Replace react-scripts with Vite 6.0.5 and @vitejs/plugin-react 4.3.4 - Move index.html from public/ to root directory - Replace setupProxy.js with Vite proxy configuration - Update environment variables from REACT_APP_* to VITE_* prefix - Configure esbuild to handle JSX in .js files - Add comprehensive migration documentation Build Optimizations: - Manual chunk splitting for react, antd, and pdf vendors - Dependency pre-bundling for faster cold starts - HMR with polling enabled for Docker volume compatibility Documentation: - Add VITE_MIGRATION.md with comprehensive migration guide - Update README.md with Vite-specific configuration and commands - Document environment variable migration and troubleshooting Performance Benefits: - Development server startup: 10-30s → 1-2s - Hot Module Replacement: 2-5s → <1s - Production build time: 60-120s → 30-60s 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…ports The codebase extensively uses SVG imports with ?react query syntax (39 imports in src/assets/index.js). vite-plugin-svgr is required to transform these imports into React components. Changes: - Add vite-plugin-svgr@^4.5.0 to devDependencies - Restore svgr plugin in vite.config.js - Add vite-plugin-svgr type references in vite-env.d.ts - Remove conflicting public/index.html from CRA migration Fixes white screen issue caused by SVG components failing to render. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
Important Review skippedToo many files! This PR contains 224 files, which is 74 over the limit of 150. You can disable this status message by setting the
WalkthroughSwitches frontend from Create React App to Vite (including bun-based Docker builds), migrates env keys to VITE_* with REACT_APP_* fallbacks, adds Vite config and typings, converts many CommonJS Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer
participant Vite as Vite Dev Server
participant Browser as Browser
participant Proxy as Vite Proxy
participant CRA as CRA (legacy)
rect rgba(200,230,255,0.5)
Note over Dev,Browser: Old flow (CRA)
Dev->>CRA: npm start (react-scripts)
CRA->>Browser: serve app + HMR
Browser->>CRA: /api requests
CRA->>Proxy: forward to REACT_APP_BACKEND_URL (setupProxy.js)
end
rect rgba(220,255,200,0.5)
Note over Dev,Browser: New flow (Vite)
Dev->>Vite: npm run dev (vite)
Vite->>Browser: serve app (native ESM + HMR)
Browser->>Vite: /api requests
Vite->>Proxy: conditional proxy to VITE_BACKEND_URL (vite.config.js)
end
sequenceDiagram
autonumber
participant Docker as Docker build
participant Build as bun/build step
participant Nginx as Nginx (runtime)
participant Browser as Browser
rect rgba(255,245,200,0.5)
Note over Docker,Nginx: Runtime config injection
Docker->>Build: run bun install & bun run build
Build->>Docker: run generate-runtime-config.sh (prefer VITE_, fallback REACT_APP_)
Docker->>Docker: sed inject runtime-config.js tag into /usr/share/nginx/html/index.html
Docker->>Nginx: copy built assets + /docker-entrypoint.d/40-env.sh
Nginx->>Browser: serve index.html + runtime-config.js
Browser->>Browser: app loads runtime config at startup
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Important Action Needed: IP Allowlist UpdateIf your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:
Failure to add the new IP will result in interrupted reviews. 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.
Actionable comments posted: 1
🧹 Nitpick comments (4)
docker/dockerfiles/frontend.Dockerfile (1)
53-54: LGTM! Runtime config injection strategy is sound.The
sedcommand correctly injects the runtime-config.js script into the built index.html's<head>section. This ensureswindow.RUNTIME_CONFIGis available before the application modules execute.Note: The
\nmay not produce an actual newline in Alpine'ssed, resulting in the injected script on the same line as</head>. This doesn't affect functionality, but if formatting is important, consider using a literal newline:-RUN sed -i 's|</head>| <script src="/config/runtime-config.js"></script>\n </head>|' /usr/share/nginx/html/index.html +RUN sed -i 's|</head>| <script src="/config/runtime-config.js"></script>\ + </head>|' /usr/share/nginx/html/index.htmlfrontend/vite.config.js (1)
44-48: Consider renaming WDS_SOCKET_PORT to align with Vite conventions.
WDS_SOCKET_PORTis a webpack-dev-server (CRA) specific variable name. For consistency with the Vite migration and to avoid confusion, consider renaming it toVITE_HMR_CLIENT_PORTor similar.Apply this diff to use a Vite-aligned variable name:
hmr: { port: 3000, - clientPort: env.WDS_SOCKET_PORT ? Number(env.WDS_SOCKET_PORT) : 3000, + clientPort: env.VITE_HMR_CLIENT_PORT ? Number(env.VITE_HMR_CLIENT_PORT) : 3000, },Note: If keeping
WDS_SOCKET_PORTfor backward compatibility is intentional, document this decision and consider adding support for both variable names during a transition period.frontend/docs/VITE_MIGRATION.md (1)
29-47: Add language specifiers to fenced code blocks.For better syntax highlighting and markdown quality, specify the language for the fenced code blocks.
Apply these diffs:
For the file structure block (line 29):
-``` +```plaintext Before (CRA): frontend/For the environment variable mapping block (line 54):
-``` +```bash REACT_APP_BACKEND_URL → VITE_BACKEND_URL REACT_APP_ENABLE_POSTHOG → VITE_ENABLE_POSTHOGBased on static analysis hints.
Also applies to: 54-59
frontend/README.md (1)
508-527: Add language specifier to fenced code block.For better markdown quality and syntax highlighting, specify the language for the project structure code block.
Apply this diff:
-``` +```plaintext frontend/ ├── docs/ # Documentation filesBased on static analysis hints.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (14)
.gitignore(1 hunks)docker/dockerfiles/frontend.Dockerfile(3 hunks)frontend/README.md(2 hunks)frontend/docs/VITE_MIGRATION.md(1 hunks)frontend/generate-runtime-config.sh(1 hunks)frontend/index.html(1 hunks)frontend/package.json(2 hunks)frontend/src/assets/index.js(1 hunks)frontend/src/config.js(1 hunks)frontend/src/helpers/SocketContext.js(1 hunks)frontend/src/index.jsx(1 hunks)frontend/src/setupProxy.js(0 hunks)frontend/vite-env.d.ts(1 hunks)frontend/vite.config.js(1 hunks)
💤 Files with no reviewable changes (1)
- frontend/src/setupProxy.js
🧰 Additional context used
🪛 LanguageTool
frontend/README.md
[grammar] ~1-~1: Ensure spelling is correct
Context: # Unstract Frontend The Unstract frontend is buil...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[uncategorized] ~78-~78: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... - Lightning-fast HMR (updates without full page reload) - On-demand compilation - E...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
🪛 markdownlint-cli2 (0.18.1)
frontend/docs/VITE_MIGRATION.md
29-29: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
54-54: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
frontend/README.md
508-508: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (16)
.gitignore (1)
715-718: Clear and well-placed addition for MCP server configuration.The
.serenaentry is appropriately ignored as part of the MCP (Model Context Protocol) infrastructure, and the placement under the Windsurf section maintains logical organization. The comment provides helpful context.frontend/vite-env.d.ts (1)
1-2: LGTM! Standard Vite TypeScript declarations.The triple-slash references correctly provide ambient types for Vite's client APIs and the SVGR plugin, enabling TypeScript support for
import.meta.envand SVG imports with the?reactsuffix.frontend/src/index.jsx (1)
12-12: LGTM! Correct environment variable migration.The PostHog enablement flag correctly migrates from CRA's
process.env.REACT_APP_*to Vite'simport.meta.env.VITE_*pattern. The conditional logic is preserved.frontend/src/helpers/SocketContext.js (1)
18-19: LGTM! Correct Vite environment variable migration.The environment checks properly migrate to Vite's conventions:
NODE_ENV→MODEREACT_APP_BACKEND_URL→VITE_BACKEND_URLThe logic correctly handles both undefined MODE and explicit "development" mode.
frontend/generate-runtime-config.sh (1)
7-19: LGTM! Excellent backward compatibility approach.The script correctly implements a fallback chain from
VITE_*toREACT_APP_*environment variables using proper shell parameter expansion. This ensures smooth transition during migration while maintaining compatibility with existing deployments.frontend/index.html (1)
1-16: LGTM! Correct Vite HTML structure.The HTML correctly adapts to Vite's conventions:
- Removed CRA's
%PUBLIC_URL%placeholder in favor of absolute paths- Added module script pointing to
/src/index.jsx(Vite's entry point)- Removed inline runtime config (now injected by Docker for production)
The runtime config injection happens in the Dockerfile before the app bundle loads, ensuring
window.RUNTIME_CONFIGis available when needed.frontend/src/config.js (1)
14-21: LGTM! Correct environment variable migration with preserved fallback chain.The configuration correctly migrates to Vite's
import.meta.env.VITE_*pattern while maintaining the established priority order:
- Runtime config (for containers)
- Environment variables (for development)
- Default values (fallback)
docker/dockerfiles/frontend.Dockerfile (1)
26-26: LGTM! Correct environment variable for Vite.The empty
VITE_BACKEND_URLat build time is correct—the actual backend URL will be determined at runtime based on the deployment environment.frontend/src/assets/index.js (1)
1-38: LGTM! Correct SVG import migration for Vite.All SVG imports correctly migrate to the
?reactsuffix pattern used byvite-plugin-svgr. The pattern is consistently applied across all 38 imports, and the exported identifiers remain unchanged, preserving the public API.frontend/vite.config.js (3)
12-26: LGTM! JSX handling in .js files configured correctly.The combination of React plugin inclusion patterns and ESBuild loader configuration properly handles JSX syntax in both
.jsxand.jsfiles, which is essential for CRA migrations where JSX in.jsfiles is common.
59-81: LGTM! Well-structured build configuration.The build configuration includes sensible optimizations:
- Custom output directory maintains compatibility with existing infrastructure
- Manual chunk splitting strategy improves caching by separating frequently-updated code from stable vendor libraries
- Logical grouping of related vendor dependencies (React, Ant Design, PDF libraries)
92-95: LGTM! Compatibility shim for libraries.The
process.env: {}shim maintains compatibility with libraries that expectprocess.envto exist, which is a common requirement during CRA-to-Vite migrations.frontend/package.json (2)
57-69: LGTM! Scripts properly updated for Vite.The script configuration correctly:
- Provides both
devandstartcommands pointing to Vite for flexibility- Uses
vite buildfor production builds- Replaces Jest with Vitest for testing
- Adds
previewfor testing production builds locally- Appropriately removes the CRA-specific
ejectcommand
82-94: LGTM! Dependencies correctly updated for Vite ecosystem.The devDependencies reflect a complete migration to Vite tooling with appropriate plugin versions for React support, SVG handling, and testing via Vitest.
frontend/docs/VITE_MIGRATION.md (1)
1-368: LGTM! Excellent comprehensive migration guide.This documentation provides thorough coverage of the CRA-to-Vite migration including:
- Clear before/after comparisons for configuration and code
- Environment variable migration mapping with examples
- Docker-specific considerations and solutions
- Performance optimizations with explanations
- Troubleshooting section for common issues
This will be valuable for developers working with the migrated codebase.
frontend/README.md (1)
237-377: LGTM! Comprehensive Docker and performance documentation.The documentation thoroughly covers:
- Docker development workflow with HMR support
- Production build process and optimizations
- Runtime configuration injection mechanism
- Performance improvements with specific metrics
- Chunk splitting strategy and benefits
This provides valuable context for both development and production deployments.
…ntation The README documentation showed incorrect HMR clientPort configuration using process.env instead of the env variable from loadEnv. Changes: - Update clientPort example to use env.WDS_SOCKET_PORT (not process.env) - Change from falsy coalescing to explicit conditional check - Ensure documentation matches vite.config.js implementation Addresses CodeRabbit feedback on PR #1607. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/README.md (1)
508-526: Add language specifier to fenced code block.The tree-structure code block at line 508 lacks a language specifier. Markdown linters require all fenced code blocks to declare a language for proper syntax highlighting and lint compliance.
Apply this diff to specify the language:
-``` +```text frontend/ ├── docs/ # Documentation files
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
frontend/README.md(2 hunks)
🧰 Additional context used
🪛 LanguageTool
frontend/README.md
[grammar] ~1-~1: Ensure spelling is correct
Context: # Unstract Frontend The Unstract frontend is buil...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[uncategorized] ~78-~78: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... - Lightning-fast HMR (updates without full page reload) - On-demand compilation - E...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
🪛 markdownlint-cli2 (0.18.1)
frontend/README.md
508-508: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (1)
frontend/README.md (1)
315-329: ✓ HMR configuration now consistent with implementation.The HMR configuration example at line 326 now correctly uses
env.WDS_SOCKET_PORT ? Number(env.WDS_SOCKET_PORT) : 3000and matches the actual vite.config.js implementation. The previous inconsistency flagged in earlier reviews has been resolved.
…mentation The README proxy configuration example incorrectly used process.env instead of the env variable from loadEnv. Changes: - Update proxy target to use env.VITE_BACKEND_URL (not process.env) - Ensure documentation matches vite.config.js implementation - Maintain consistency across all Vite configuration examples Addresses CodeRabbit critical feedback on PR #1607. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/README.md (1)
508-526: Add language identifier to fenced code block.The project structure diagram lacks a language specifier for the code fence, which may cause rendering or linting issues.
Apply this diff to specify the language as
text:-``` +```text frontend/ ├── docs/ # Documentation files │ └── VITE_MIGRATION.md # Vite migration guide ├── public/ # Static assets (served as-is) │ ├── manifest.json │ └── robots.txt ├── src/ # Source code │ ├── assets/ # Images, fonts, etc. │ ├── components/ # Reusable React components │ ├── helpers/ # Utility functions and helpers │ ├── pages/ # Page components │ ├── config.js # App configuration │ └── index.jsx # Application entry point ├── index.html # HTML entry point (Vite) ├── vite.config.js # Vite configuration ├── package.json # Dependencies and scripts └── .env # Environment variables (VITE_ prefix) -``` +```
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
frontend/README.md(2 hunks)
🧰 Additional context used
🪛 LanguageTool
frontend/README.md
[grammar] ~1-~1: Ensure spelling is correct
Context: # Unstract Frontend The Unstract frontend is buil...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[uncategorized] ~78-~78: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... - Lightning-fast HMR (updates without full page reload) - On-demand compilation - E...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
🪛 markdownlint-cli2 (0.18.1)
frontend/README.md
508-508: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (3)
frontend/README.md (3)
1-6: Documentation comprehensively covers the Vite migration.The README clearly documents the transition from CRA to Vite with accurate environment variable patterns, Docker setup, HMR configuration, and a helpful migration checklist. Examples at lines 188 (proxy target) and 326 (HMR clientPort) correctly use Vite's
envpattern fromloadEnv()rather thanprocess.env.
27-43: Environment variable setup section is clear and well-structured.The quick-start guidance correctly emphasizes the
VITE_prefix requirement and provides concrete examples for.envconfiguration. This aligns well with the migration objectives and helps prevent a common source of confusion for developers transitioning from CRA.
443-460: Migration checklist is thorough and actionable.The checklist at lines 452–460 covers the critical breaking changes: environment variables,
import.meta.envusage, SVG imports, HTML paths, proxy setup, and HMR testing. This proactively addresses the most common migration pain points and reduces support burden.
hari-kuriakose
requested review from
jagadeeswaran-zipstack,
jaseemjaskp and
vishnuszipstack
# Conflicts: # docker/dockerfiles/frontend.Dockerfile # frontend/package-lock.json
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/src/assets/index.js (1)
1-81: Fix OrganizationIcon export to use Vite SVGR default import (build currently fails).Line 81 uses CRA-style
ReactComponentexport which is incompatible with Vite SVGR. This pattern fails with "does not provide an export namedReactComponent" error. All other SVGs in this file correctly use the?reactquery pattern. Update OrganizationIcon to match: import with?reactand add to the main export list.🛠️ Suggested fix
import ETLIcon from "./etl.svg?react"; +import OrganizationIcon from "./organization-icon.svg?react"; export { SunIcon, MoonIcon, Logo64, Logo24, @@ CustomToolIcon, ETLIcon, Task, + OrganizationIcon, }; -export { ReactComponent as OrganizationIcon } from "./organization-icon.svg";frontend/package.json (1)
86-100: Update Node engine range to match Vite 6 requirements.Vite 6 requires Node ≥18, but the current range (
>=16.0.0 <20) allows Node 16, 17, and 19—none of which are compatible. Additionally, Vite 6 supports Node 18, 20, and 22+ (Node 21 is not supported). Update to allow only supported versions:Suggested engines update
"engines": { - "node": ">=16.0.0 <20", + "node": ">=18.0.0", "npm": ">=8.19.4" }Align CI and Docker configurations accordingly.
…e compatibility Converted all require() calls in frontend/src to use try/catch with top-level await import() to support Vite's native ES module system. Plugin imports remain optional via try/catch to preserve OSS compatibility where plugin files are not present. Special cases: - Pipelines.jsx: static import (non-plugin, always available) - DsSettingsCard.jsx: async IIFE inside useEffect
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/src/components/custom-tools/output-analyzer/OutputAnalyzerCard.jsx (1)
16-45: Fix async import so public docs load reliably.
publicDocumentApiis assigned asynchronously at module scope, but that doesn’t trigger a re-render. For public sources,fileUrlEndpointcan staynull, so the document never loads. Move the import into component state/effect and include it in theuseMemodeps.✅ Suggested fix
-let publicDocumentApi; -try { - const mod = await import("../../../plugins/prompt-studio-public-share/helpers/PublicShareAPIs"); - publicDocumentApi = mod.publicDocumentApi; -} catch { - // The component will remain null if it is not available -} - function OutputAnalyzerCard({ doc, selectedPrompts, totalFields }) { + const [publicDocumentApi, setPublicDocumentApi] = useState(null); + + useEffect(() => { + let cancelled = false; + (async () => { + try { + const mod = await import( + "../../../plugins/prompt-studio-public-share/helpers/PublicShareAPIs" + ); + if (!cancelled) setPublicDocumentApi(() => mod.publicDocumentApi); + } catch { + // Optional dependency; leave null + } + })(); + return () => { + cancelled = true; + }; + }, []); + const [fileUrl, setFileUrl] = useState(""); const [isDocLoading, setIsDocLoading] = useState(false); const [filledFields, setFilledFields] = useState(0); @@ const fileUrlEndpoint = useMemo(() => { if (!doc) return null; if (isPublicSource) { return publicDocumentApi?.(id, doc.document_id, null); } else { return `/api/v1/unstract/${sessionDetails?.orgId}/prompt-studio/file/${details?.tool_id}?document_id=${doc?.document_id}`; } - }, [doc]); + }, [doc, isPublicSource, id, sessionDetails?.orgId, details?.tool_id, publicDocumentApi]);frontend/src/hooks/useSessionValid.js (1)
38-46: Violation of Rules of Hooks:useSelectedProductStorecalled conditionally.Calling
useSelectedProductStoreinside a try-catch block with a conditional check violates React's Rules of Hooks. Hooks must be called unconditionally at the top level of the hook/component, in the same order on every render. This can lead to bugs where React's internal hook state gets out of sync.🐛 Proposed fix
Call the hook unconditionally using optional chaining, moving the try-catch pattern outside:
function useSessionValid() { const setSessionDetails = useSessionStore((state) => state.setSessionDetails); const handleException = useExceptionHandler(); const { setAlertDetails } = useAlertStore(); const navigate = useNavigate(); const userSession = useUserSession(); - try { - if (selectedProductStore?.useSelectedProductStore) { - selectedProduct = selectedProductStore?.useSelectedProductStore( - (state) => state?.selectedProduct - ); - } - } catch (error) { - // Do nothing - } + // Call hook unconditionally - will be undefined if plugin unavailable + selectedProduct = selectedProductStore?.useSelectedProductStore?.( + (state) => state?.selectedProduct + );Note: This assumes
useSelectedProductStoregracefully handles being called when the store isn't properly initialized. If not, you may need to restructure so this hook is only rendered when the plugin is available.
🤖 Fix all issues with AI agents
In
`@frontend/src/components/agency/configure-connector-modal/ConfigureConnectorModal.jsx`:
- Around line 31-35: The dynamic import that assigns RuleEngine and
ruleEngineTabs (the await import(...) that currently runs at module top-level)
is resolved at build-time by Vite and will fail the build if the optional
manual-review module is missing; move this import out of top-level module scope
into a runtime async path (for example inside a React useEffect or a lazily
loaded component/route) so the try/catch can handle missing modules at runtime,
perform the await import(...) there, and keep the existing assignments to
RuleEngine and ruleEngineTabs inside that guarded block; ensure any UI that
depends on RuleEngine/ruleEngineTabs checks for their presence (e.g., loading
state or conditional rendering) before using them.
In `@frontend/src/components/custom-tools/document-parser/DocumentParser.jsx`:
- Around line 19-24: Set the Vite build target to ES2022 so top-level await used
in files like DocumentParser.jsx and Router.jsx works: update the build section
in vite.config.js to include target: 'es2022'. If you cannot raise the target,
refactor top-level await usages (the dynamic import lines in DocumentParser.jsx
/ Router.jsx and the other ~50 files) into async initializer functions instead.
In
`@frontend/src/components/custom-tools/output-for-doc-modal/OutputForDocModal.jsx`:
- Around line 29-32: The top-level await import in OutputForDocModal.jsx (the
try block that assigns publicOutputsApi and publicAdapterApi) requires an ES2022
build target; update your Vite config to set build.target to 'ES2022' so
top-level await is preserved during bundling, or alternatively refactor the
import out of module top-level into a lazy-loaded function (e.g.,
loadPublicShareAPIs()) that does the dynamic import and is called only when
isPublicSource is true to avoid needing top-level await. Ensure you reference
the same exported names (publicOutputsApi, publicAdapterApi) when wiring the
lazy loader.
In `@frontend/src/components/custom-tools/pdf-viewer/PdfViewer.jsx`:
- Around line 12-16: The top-level await import of
"../../../plugins/pdf-highlight/RenderHighlights" must be removed and replaced
with a lazy-load inside the PdfViewer component: delete the module-scope
try/await import block and instead call import(...) from within a useEffect in
the PdfViewer function, set the loaded RenderHighlights into component state via
useState (e.g., setRenderHighlights), handle errors in the catch and provide a
loading/fallback render while undefined, and ensure any component unmount
cleanup guards setting state after unmount.
In `@frontend/src/components/custom-tools/settings-modal/SettingsModal.jsx`:
- Around line 25-33: The current single try/catch around importing
SummarizeManager, ChallengeManager, and HighlightManager causes one failed
import to skip the rest; update SettingsModal.jsx to import each optional plugin
in its own try/catch so failures are isolated: wrap the import and assignment of
SummarizeManager, ChallengeManager, and HighlightManager in three separate try
blocks, catch and ignore errors per-block (leaving the variable null) and keep
the rest of the imports running.
In `@frontend/src/components/helpers/custom-tools/CustomToolsHelper.js`:
- Around line 15-17: The top-level await used when dynamically importing
"../../../plugins/prompt-studio-public-share/helpers/PublicShareAPIs" (assigning
shareManagerToolSource in CustomToolsHelper.js) requires ES2022; fix by either
adding build: { target: 'es2022' } to your Vite config so Vite will transpile
top-level await, or refactor the dynamic import out of top-level scope (e.g.,
move the import into an async initializer function such as
initializeShareManager or into a React useEffect/lazy loader that awaits
import(...) and sets shareManagerToolSource) so no top-level await is used.
In `@frontend/src/hooks/usePromptOutput.js`:
- Around line 9-20: The optional imports promptOutputApiSps and publicOutputsApi
may be undefined causing getUrl calls to throw when flags isSimplePromptStudio
or isPublicSource are true; update the code that uses these helpers (the getUrl
invocation paths guarded by isSimplePromptStudio/isPublicSource) to first check
that promptOutputApiSps/publicOutputsApi and their getUrl functions exist and
are functions, and if not either use a safe fallback (e.g., return null/empty
URL) or throw a clear, descriptive error before calling; locate references to
promptOutputApiSps, publicOutputsApi, isSimplePromptStudio, isPublicSource and
getUrl to apply the guards and explicit error/fallback.
In `@frontend/src/hooks/useSessionValid.js`:
- Around line 12-18: The code calls the React hook usePlatformAdmin() at module
initialization (top-level) via the dynamic import block which breaks the Rules
of Hooks; change the pattern so the dynamic import only obtains the module
reference (or optional hook factory) at module load but do NOT call
usePlatformAdmin there, and instead call the hook inside the custom hook
function useSessionValid (e.g., resolve the imported module or optional hook
reference and invoke usePlatformAdmin within useSessionValid to set
isPlatformAdmin). Locate the top-level dynamic import and the isPlatformAdmin
variable and move the hook invocation into useSessionValid while keeping the
dynamic import/resolution logic separate from the hook call.
🧹 Nitpick comments (5)
frontend/src/components/agency/ds-settings-card/DsSettingsCard.jsx (1)
71-83: Dynamic import migration looks correct; consider addingflagsto dependency array.The conversion from
require()to asyncimport()is the correct pattern for Vite/ESM compatibility. However,flagsis read inside the effect but not listed in the dependency array. Ifflags.app_deploymentbecomes truthy after mount (e.g., session loads asynchronously), the plugin option won't be added.Also, minor typo on line 79: "of it is not" → "if it is not".
💡 Suggested improvement
useEffect(() => { const loadPlugin = async () => { try { const mod = await import("../../../plugins/dscard-input-options/AppDeploymentCardInputOptions"); if (flags.app_deployment && mod.appDeploymentInputOption) { setUpdatedInputoptions(mod.appDeploymentInputOption); } } catch { - // The component will remain null of it is not available + // The component will remain unchanged if the plugin is not available } }; loadPlugin(); - }, []); + }, [flags.app_deployment]);frontend/src/components/pipelines-or-deployments/pipelines/Pipelines.jsx (1)
59-78: Simplify import: use direct named import forfetchExecutionLogs.The module only exports a single named export, so the namespace import with destructuring is unnecessarily verbose.
♻️ Suggested refactor
-import * as fetchExecutionLogsModule from "../log-modal/fetchExecutionLogs.js"; +import { fetchExecutionLogs } from "../log-modal/fetchExecutionLogs.js"; function Pipelines({ type }) { const [tableData, setTableData] = useState([]); const [openEtlOrTaskModal, setOpenEtlOrTaskModal] = useState(false); const [openDeleteModal, setOpenDeleteModal] = useState(false); const [selectedPorD, setSelectedPorD] = useState({}); const [tableLoading, setTableLoading] = useState(true); const { sessionDetails } = useSessionStore(); const { setAlertDetails } = useAlertStore(); const axiosPrivate = useAxiosPrivate(); const handleException = useExceptionHandler(); const { clearFileHistory, isClearing: isClearingFileHistory } = useClearFileHistory(); const [isEdit, setIsEdit] = useState(false); const [openLogsModal, setOpenLogsModal] = useState(false); const [executionLogs, setExecutionLogs] = useState([]); const [executionLogsTotalCount, setExecutionLogsTotalCount] = useState(0); const [openFileHistoryModal, setOpenFileHistoryModal] = useState(false); - const { fetchExecutionLogs } = fetchExecutionLogsModule;frontend/src/components/helpers/auth/RequireAuth.js (1)
36-51: Potential React hooks rule violation with conditional hook calls.The
useSelectedProductStorehook is called conditionally inside try/catch blocks (lines 37-43 and 44-51). While this works because the module-level import determines whetherselectedProductStoreexists, calling hooks conditionally can violate React's Rules of Hooks if the condition changes between renders.Since
selectedProductStoreis set at module load time (top-level await), it won't change between renders, so this should be safe in practice. However, consider consolidating these two identical hook calls into a single call for clarity:♻️ Suggested consolidation
+ let currentSelectedProduct; + try { + currentSelectedProduct = selectedProductStore?.useSelectedProductStore( + (state) => state?.selectedProduct + ); + } catch { + // Do nothing + } + const isLlmWhisperer = currentSelectedProduct === "llm-whisperer"; + const isVerticals = currentSelectedProduct === "verticals"; - try { - isLlmWhisperer = - selectedProductStore.useSelectedProductStore( - (state) => state?.selectedProduct - ) === "llm-whisperer"; - } catch (error) { - // Do nothing - } - try { - isVerticals = - selectedProductStore.useSelectedProductStore( - (state) => state?.selectedProduct - ) === "verticals"; - } catch (error) { - // Do nothing - }frontend/src/routes/useMainAppRoutes.js (1)
238-251: Minor: Redundant optional chaining onObject.keys()result.
Object.keys(PRODUCT_NAMES)always returns an array (even for empty objects), so the?.lengthoptional chaining is unnecessary. This is harmless but slightly misleading.♻️ Suggested simplification
- if (OnboardProduct && Object.keys(PRODUCT_NAMES)?.length) { + if (OnboardProduct && Object.keys(PRODUCT_NAMES).length) {frontend/src/components/navigations/top-nav-bar/TopNavBar.jsx (1)
140-155: Consider defensive check before hook access.At line 141,
selectedProductStore.useSelectedProductStoreis accessed without optional chaining, while the try/catch block at lines 146-155 uses optional chaining (unstractSubscriptionPlanStore?.useUnstractSubscriptionPlanStore). For consistency and safety:♻️ Suggested defensive pattern
- if (selectedProductStore) { - selectedProduct = selectedProductStore.useSelectedProductStore( + if (selectedProductStore?.useSelectedProductStore) { + selectedProduct = selectedProductStore.useSelectedProductStore( (state) => state?.selectedProduct ); }
frontend/src/components/agency/configure-connector-modal/ConfigureConnectorModal.jsx
Show resolved
Hide resolved
frontend/src/components/custom-tools/document-parser/DocumentParser.jsx
Outdated
Show resolved
Hide resolved
frontend/src/components/custom-tools/output-for-doc-modal/OutputForDocModal.jsx
Show resolved
Hide resolved
frontend/src/components/helpers/custom-tools/CustomToolsHelper.js
Outdated
Show resolved
Hide resolved
- Make selectedPrompts optional in CombinedOutput since ToolsMain does not pass it and the component handles undefined - Change llmProfiles PropType from string to array in JsonViewBody to match the actual data type
- Change coverageCountData from PropTypes.object to PropTypes.array in PromptCardWrapper, PromptCard, and PromptCardItems - Fix PropTypes.text (undefined) to PropTypes.string for promptKey and promptText in PromptCardItems - Change highlightData to PropTypes.oneOfType([object, array]) in DisplayPromptResult - Change apiService from PropTypes.func to PropTypes.object in ManageKeys - Change connMode from required to optional in ConfigureConnectorModal - Change type from required to optional in ListOfSources
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@frontend/src/components/custom-tools/add-llm-profile/AddLlmProfile.jsx`:
- Around line 211-233: The code iterates data with data.forEach which will throw
if data is null or not an array; update the block that builds
llm/vectorDb/embedding/x2Text (the loop that creates option objects and calls
setLlmItems, setVectorDbItems, setEmbeddingItems, setX2TextItems and
setAreAdaptersReady) to first guard data by using a safe default or check (e.g.
Array.isArray(data) ? data : []) before iterating, or bail out early when data
is falsy/non-array, so the UI doesn't crash on non-array API responses.
🧹 Nitpick comments (1)
frontend/src/components/deployments/manage-keys/ManageKeys.jsx (1)
357-364: PreferPropTypes.shapeforapiServiceto validate required methods.
PropTypes.objectwon’t catch missing methods at runtime. A shape with the expected functions will provide clearer warnings.♻️ Proposed change
- apiService: PropTypes.object.isRequired, + apiService: PropTypes.shape({ + createApiKey: PropTypes.func.isRequired, + updateApiKey: PropTypes.func.isRequired, + deleteApiKey: PropTypes.func.isRequired, + }).isRequired,
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/src/components/navigations/side-nav-bar/SideNavBar.jsx (2)
34-85: Static optional imports will break the Vite build—plugin directory is missing.The
frontend/src/pluginsdirectory does not exist in this repository. Vite resolves static-stringimport()calls at build time (not runtime), so the try/catch blocks on lines 35–82 are ineffective; the build will fail immediately when Vite encounters paths like"../../../plugins/app-deployment/getMenuItem"that do not resolve.While the code includes good runtime defensive checks (optional chaining, flags), these cannot prevent build-time failures. For truly optional plugins, use either:
- Vite-safe dynamic imports via
import.meta.glob()- Stub files in the plugins directory with empty exports
- Conditional imports based on environment variables or build-time configuration
🔧 Example: Dynamic import pattern with import.meta.glob
+const pluginModules = import.meta.glob( + "../../../plugins/**/*.{js,jsx}", + { eager: false } +); +const safeImport = async (modulePath) => { + const loader = pluginModules[modulePath]; + return loader ? await loader() : null; +}; + let getMenuItem; try { - getMenuItem = await import("../../../plugins/app-deployment/getMenuItem"); + getMenuItem = await safeImport( + "../../../plugins/app-deployment/getMenuItem.js" + ); } catch { // Plugin unavailable. }
34-85: Normalize dynamic import results to handle both default and named exports consistently.The code imports plugin modules but assumes inconsistent export patterns: Line 327 explicitly accesses
getMenuItem.default(), while lines 159–177 expect named exports (useUnstractSubscriptionPlanStore,useSelectedProductStore,useSideMenu) without fallback. If any plugin uses CommonJS default export instead of ES6 named export, calls likesideMenu.useSideMenu()will fail at runtime since the property will be undefined.Apply the normalization pattern to all dynamic imports:
Normalize import pattern
-let sideMenu; +let useSideMenu; try { - sideMenu = await import("../../../plugins/hooks/useSideMenu"); + const sideMenuModule = await import("../../../plugins/hooks/useSideMenu"); + useSideMenu = sideMenuModule?.useSideMenu ?? sideMenuModule?.default; } catch { // Plugin unavailable. } let menu; -if (sideMenu) { - menu = sideMenu.useSideMenu(); +if (useSideMenu) { + menu = useSideMenu(); }Apply the same pattern to:
selectedProductStore,unstractSubscriptionPlanStore, andgetMenuItem(lines 158–178, 326–328).
Add optionalPluginImports() Rollup plugin to vite.config.js that resolves missing plugin paths to an empty module at build time instead of failing. This allows the existing try/catch dynamic import pattern to work during production builds when plugin files are absent.
The codebase uses top-level await for optional plugin imports, which requires es2022+. Vite's default target (es2020) does not support this, causing the build to fail.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@frontend/vite.config.js`:
- Around line 77-98: The hmr clientPort currently uses
Number(env.WDS_SOCKET_PORT) without validating numericness, which can produce
NaN and be ignored silently; update the server.hmr.clientPort logic to parse
env.WDS_SOCKET_PORT, check Number.isFinite(parsed) and if valid use parsed,
otherwise fall back to 3000 (i.e., compute a const parsedPort =
Number(env.WDS_SOCKET_PORT); clientPort: Number.isFinite(parsedPort) ?
parsedPort : 3000), referencing env.WDS_SOCKET_PORT and the
server.hmr.clientPort setting so invalid/non-numeric values properly fallback to
3000.
🧹 Nitpick comments (2)
frontend/vite.config.js (2)
7-44: Scope optional-plugin fallback to the intended plugins directory.Right now any relative import containing
plugins/can be silently replaced with an empty module, which can mask genuine missing imports outside the optional plugin area. Consider scoping to a known plugins root (adjust the path as needed for your repo).♻️ Suggested diff
+const PLUGINS_ROOT = path.resolve(__dirname, 'src/plugins') + function optionalPluginImports() { return { name: 'optional-plugin-imports', resolveId(source, importer) { if (!importer || !source.includes('plugins/')) return null // Only handle relative imports that go through a plugins directory if (!source.startsWith('.')) return null - const resolved = path.resolve(path.dirname(importer), source) + const resolved = path.resolve(path.dirname(importer), source) + if (!resolved.startsWith(PLUGINS_ROOT + path.sep)) return null
134-136: Consider definingprocess.env.NODE_ENVif legacy dependencies require it; currently not needed for this app.The suggested addition
'process.env.NODE_ENV': JSON.stringify(mode)is technically valid—modeis available in the vite.config.js function signature, and the pattern correctly handles Vite's compile-time replacement. However, there are no detected dependencies in the frontend bundle that actually referenceprocess.env.NODE_ENVat runtime. Thehttp-proxy-middlewaredependency (which might checkNODE_ENV) is server-side only and not bundled into the client code. Vite apps should preferimport.meta.env.DEV,import.meta.env.PROD, orimport.meta.env.MODEfor environment checks. If a future dependency does requireprocess.env.NODE_ENV, the suggested diff is the correct approach.
- Switch Dockerfile base image from node:20-alpine to oven/bun:1-alpine - Replace npm install/run commands with bun equivalents in Dockerfile - Remove engines.npm constraint, update engines.node to >=18.0.0 - Replace npm run with bun run in lint:all script - Delete package-lock.json, add bun.lock
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/package.json (1)
18-20: Add setupFiles configuration to vite.config.js for@testing-library/jest-dom.The
setupTests.jsfile exists and correctly imports@testing-library/jest-dom, but Vitest won't load it automatically without explicit configuration. AddsetupFilesto vite.config.js:export default defineConfig(({ mode }) => { return { // ... existing config ... test: { setupFiles: ['./src/setupTests.js'], }, } })Without this, jest-dom matchers won't be available during tests.
🤖 Fix all issues with AI agents
In `@docker/dockerfiles/frontend.Dockerfile`:
- Around line 57-58: The COPY instruction referencing
"../frontend/generate-runtime-config.sh" is outside the Docker build context and
will fail; change the COPY to reference the script within the build context
(e.g., "frontend/generate-runtime-config.sh" or simply
"generate-runtime-config.sh" depending on your context) so Docker can access it,
or alternatively adjust the docker build context to include the frontend
directory; ensure the destination remains /docker-entrypoint.d/40-env.sh and
retain the subsequent chmod +x step.
🧹 Nitpick comments (1)
frontend/package.json (1)
31-31: Removehttp-proxy-middlewaredependency—it's no longer used after the Vite migration.The proxy configuration has been properly moved from
setupProxy.js(which requiredhttp-proxy-middleware) tovite.config.js(lines 90-97), which uses Vite's native proxy feature. This dependency can be safely removed to reduce bundle size and maintenance overhead.
jaseemjaskp
changed the title
…1771) * feat: replace ESLint and Prettier with Biome for frontend linting and formatting - Add @biomejs/biome v2.3.13 as dev dependency - Create biome.json with migrated ESLint rules and formatter config - Update package.json scripts (lint, format, check) to use Biome - Remove ESLint and Prettier dependencies and .eslintrc.json - Add Biome pre-commit hook to .pre-commit-config.yaml - Fix undeclared sessionDetails variable in HeaderTitle.jsx - Fix Error() and Array() to use new keyword - Fix prismjs import order in CombinedOutput.jsx - Reformat all frontend source files with Biome * fix: skip biome-check in pre-commit CI and add robust flags - Add biome-check to ci.skip list since pre-commit.ci lacks reliable npm registry access for language:system hooks - Add --files-ignore-unknown=true and --no-errors-on-unmatched flags to the biome-check hook entry for robustness
Test ResultsSummary
Runner Tests - Full Report
SDK1 Tests - Full Report
|
|
Summary
This PR migrates the Unstract frontend from Create React App (CRA) to Vite for improved development experience, faster build times, and better performance. It also migrates the package manager from npm to Bun. The migration includes comprehensive configuration updates, Docker compatibility fixes, and documentation.
Key Changes
react-scriptswith Vite 6.0.5 and@vitejs/plugin-reactvite.config.jswith optimized proxy, HMR, and chunk splittingREACT_APP_*toVITE_*prefixindex.htmlto root, removedsetupProxy.jsvite-plugin-svgrfor SVG-as-React-component importsrequire()calls with ES dynamicimport()for Vite compatibilityesnextfor top-level await supportPerformance Improvements
Docker Compatibility
Breaking Changes
Environment Variables
All environment variables must now use
VITE_prefix:Code Changes
Package Manager
Files Changed
vite.config.js,vite-env.d.ts, updatedpackage.jsondocker/dockerfiles/frontend.Dockerfileto use Bun and fixed runtime config COPY pathfrontend/docs/VITE_MIGRATION.mdwith comprehensive guidepublic/index.htmlto root, removedsetupProxy.jspackage-lock.jsonwithbun.lock, added Vite toolingCombinedOutput,JsonViewBody, and other componentsrequire()with ES dynamicimport()for Vite compatibilityTesting Checklist
bun run start/bun run dev)bun run build)VITE_prefixDocumentation
Comprehensive migration guide added at
frontend/docs/VITE_MIGRATION.mdcovering:Deployment Notes
Required Actions Before Deployment:
.envfiles to useVITE_prefixbuninstead ofnpmfor frontend commandsCommits
[FEAT] Migrate frontend from Create React App to Vite- Core migrationfix(frontend): Restore vite-plugin-svgr for SVG-as-React-component imports- SVG support fixdocs(frontend): Fix HMR configuration example to match actual implementationdocs(frontend): Fix proxy configuration example to match actual implementationrefactor: replace CommonJS require() with ES dynamic import() for Vite compatibilityfix: correct PropTypes for CombinedOutput and JsonViewBodyfix: correct PropTypes warnings across frontend componentsfix: add Rollup plugin to resolve missing optional plugin importsfix: set build target to esnext for top-level await supportfeat: replace npm with bun for frontend package management (#1770)fix: correct COPY path in frontend.Dockerfile for runtime config script🤖 Generated with Claude Code
Co-Authored-By: Claude noreply@anthropic.com