fix(web): stabilize homepage loading and build pipeline#4210
Closed
tianyou-lab wants to merge 1 commit intoQuantumNous:mainfrom
Closed
fix(web): stabilize homepage loading and build pipeline#4210tianyou-lab wants to merge 1 commit intoQuantumNous:mainfrom
tianyou-lab wants to merge 1 commit intoQuantumNous:mainfrom
Conversation
Contributor
WalkthroughThe pull request refactors the frontend build pipeline and home page functionality. Changes include switching from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/src/pages/Home/index.jsx (1)
92-96:⚠️ Potential issue | 🟠 MajorCopy the same URL that you display.
The input now shows the selected endpoint, but the copy action still writes only
serverAddress. After the user picks a path, the pasted URL no longer matches the visible field.Proposed fix
+ const selectedEndpoint = endpointItems[endpointIndex]?.value || ''; + const handleCopyBaseURL = async () => { - const ok = await copy(serverAddress); + const ok = await copy(`${serverAddress}${selectedEndpoint}`); if (ok) { showSuccess(t('已复制到剪切板')); } }; @@ - value={`${serverAddress}${endpointItems[endpointIndex] || ''}`} + value={`${serverAddress}${selectedEndpoint}`}Also applies to: 132-141
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/Home/index.jsx` around lines 92 - 96, The copy handler currently copies only serverAddress; update handleCopyBaseURL (and the analogous copy handler around lines 132-141) to copy the full visible URL shown in the input (concatenate serverAddress with the currently selected path/endpoint or read the input's current value) instead of always copying serverAddress so the pasted URL matches what the user sees; locate and change the copy(...) call in handleCopyBaseURL (and the corresponding second handler) to use the computedDisplayedUrl/currentInputValue used to render the field.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Dockerfile`:
- Line 9: Replace the direct invocation "bunx vite build --minify=false" in the
Dockerfile RUN line with the canonical frontend entrypoint by running "bun run
build" (preserving the DISABLE_ESLINT_PLUGIN, NODE_OPTIONS, and
VITE_REACT_APP_VERSION env values around the command), and move any Vite/minify
tweaks into the frontend's package.json build script or Vite config (so that the
"build" script in package.json and Vite config control minification/flags rather
than bypassing them); ensure Docker uses Bun as the package manager/runner for
consistency with "bun install", "bun run dev", "bun run build", and "bun run
i18n:*".
In `@web/src/components/common/ErrorBoundary.jsx`:
- Around line 58-64: The headings "Error" and "Component Stack" in the
ErrorBoundary component should be routed through i18n: import and use the
useTranslation hook inside the ErrorBoundary component (referencing
ErrorBoundary, errorMessage, and componentStack) and replace the hardcoded
strings with t('中文 key for Error') and t('中文 key for Component Stack'); ensure
the chosen Chinese keys are added to the locale JSONs under
web/src/i18n/locales/{lang}.json as flat keys and call t(...) where the divs
render the headings.
- Around line 24-27: The ErrorBoundary currently stores and renders raw
errorMessage and componentStack (see componentDidCatch/setState setting
errorMessage and componentStack and the fallback render block around lines
53-70); change it to avoid exposing details to users by gating detailed info
behind a dev/debug flag: in componentDidCatch only save full error data when a
debug flag is true (use a prop like debug or an env check such as
process.env.NODE_ENV !== 'production'), otherwise set a generic user-facing
message and clear componentStack; update the fallback render to conditionally
show the detailed panel only when debug is enabled and always render the generic
fallback for production users.
In `@web/src/pages/Home/index.jsx`:
- Around line 80-81: The iframe handshake in Home/index.jsx is forcing themeMode
to 'light' which breaks dark/auto syncing; update the postMessage to send the
current theme value (use the same actualTheme / theme variable used in
useHeaderBar.js) instead of the hardcoded 'light', and add a safe fallback
(e.g., 'light' only if actualTheme is undefined) so
iframe.contentWindow.postMessage uses the real theme state.
- Around line 72-82: The postMessage handler is being attached before the iframe
is rendered because setHomePageContent(content) is async; update the logic so
messaging is attached after the iframe actually mounts: after calling
setHomePageContent (and when rawContent.startsWith('https://')), move the iframe
query/iframe.onload registration into a follow-up effect or callback that runs
after the DOM updates (e.g., a useEffect that depends on the state updated by
setHomePageContent or a ref callback for the iframe), locate the code around
setHomePageContent, rawContent, document.querySelector('iframe') and
iframe.onload, and ensure you add and clean up the load listener before calling
iframe.contentWindow.postMessage({ themeMode: 'light' }) and postMessage({ lang:
currentLanguage }).
- Around line 132-160: endpointItems contains objects like { value }, but the
JSX treats each item as a string, causing [object Object] rendering and
duplicate keys; update the input value to use
endpointItems[endpointIndex]?.value (e.g.,
value={`${serverAddress}${endpointItems[endpointIndex]?.value || ''}`), change
the map to use .map((endpoint, idx) => ...) and use key={endpoint.value},
display {endpoint.value}, compare against endpointItems[endpointIndex]?.value
for active styling, and setEndpointIndex(idx) in the onClick handler (avoid
indexOf on objects).
---
Outside diff comments:
In `@web/src/pages/Home/index.jsx`:
- Around line 92-96: The copy handler currently copies only serverAddress;
update handleCopyBaseURL (and the analogous copy handler around lines 132-141)
to copy the full visible URL shown in the input (concatenate serverAddress with
the currently selected path/endpoint or read the input's current value) instead
of always copying serverAddress so the pasted URL matches what the user sees;
locate and change the copy(...) call in handleCopyBaseURL (and the corresponding
second handler) to use the computedDisplayedUrl/currentInputValue used to render
the field.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 564bb291-cc11-45ce-b822-0585d4a60c67
📒 Files selected for processing (5)
Dockerfileweb/src/App.jsxweb/src/components/common/ErrorBoundary.jsxweb/src/pages/Home/index.jsxweb/vite.config.js
| COPY ./web . | ||
| COPY ./VERSION . | ||
| RUN DISABLE_ESLINT_PLUGIN='true' VITE_REACT_APP_VERSION=$(cat VERSION) bun run build | ||
| RUN DISABLE_ESLINT_PLUGIN='true' NODE_OPTIONS='--max-old-space-size=4096' VITE_REACT_APP_VERSION=$(cat VERSION) bunx vite build --minify=false |
Contributor
There was a problem hiding this comment.
Keep the frontend image build on bun run build.
Calling bunx vite build here sidesteps the repo’s canonical frontend build entrypoint, so Docker can silently diverge from local/CI builds once package.json adds flags, wrappers, or pre/post hooks. Please fold the Vite/minify tweak into the normal build script/config instead of bypassing it.
Based on learnings: Use Bun as the preferred package manager and script runner for the frontend (web/ directory). Use bun install, bun run dev, bun run build, and bun run i18n:* commands.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Dockerfile` at line 9, Replace the direct invocation "bunx vite build
--minify=false" in the Dockerfile RUN line with the canonical frontend
entrypoint by running "bun run build" (preserving the DISABLE_ESLINT_PLUGIN,
NODE_OPTIONS, and VITE_REACT_APP_VERSION env values around the command), and
move any Vite/minify tweaks into the frontend's package.json build script or
Vite config (so that the "build" script in package.json and Vite config control
minification/flags rather than bypassing them); ensure Docker uses Bun as the
package manager/runner for consistency with "bun install", "bun run dev", "bun
run build", and "bun run i18n:*".
Comment on lines
+24
to
+27
| this.setState({ | ||
| errorMessage: error?.message || String(error || ''), | ||
| componentStack: errorInfo?.componentStack || '', | ||
| }); |
Contributor
There was a problem hiding this comment.
Don't expose raw exception details to every user.
Rendering errorMessage and especially componentStack in the fallback leaks internal implementation details on production crashes. Please gate this panel behind a dev/debug flag and keep the default user-facing fallback generic.
Also applies to: 53-70
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/common/ErrorBoundary.jsx` around lines 24 - 27, The
ErrorBoundary currently stores and renders raw errorMessage and componentStack
(see componentDidCatch/setState setting errorMessage and componentStack and the
fallback render block around lines 53-70); change it to avoid exposing details
to users by gating detailed info behind a dev/debug flag: in componentDidCatch
only save full error data when a debug flag is true (use a prop like debug or an
env check such as process.env.NODE_ENV !== 'production'), otherwise set a
generic user-facing message and clear componentStack; update the fallback render
to conditionally show the detailed panel only when debug is enabled and always
render the generic fallback for production users.
Comment on lines
+58
to
+64
| <div className='text-sm font-semibold mb-2'>Error</div> | ||
| <div className='text-xs text-semi-color-text-1'>{errorMessage}</div> | ||
| {componentStack && ( | ||
| <> | ||
| <div className='text-sm font-semibold mt-4 mb-2'> | ||
| Component Stack | ||
| </div> |
Contributor
There was a problem hiding this comment.
Localize the new diagnostics headings.
If this panel stays, Error and Component Stack should also go through t(...); they currently bypass the app’s i18n flow.
As per coding guidelines: web/src/**/*.{ts,tsx,js,jsx}: Frontend i18n: Use i18next + react-i18next + i18next-browser-languagedetector. Translation files in web/src/i18n/locales/{lang}.json must be flat JSON with Chinese source strings as keys. Use useTranslation() hook and call t('中文key') in components.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/common/ErrorBoundary.jsx` around lines 58 - 64, The
headings "Error" and "Component Stack" in the ErrorBoundary component should be
routed through i18n: import and use the useTranslation hook inside the
ErrorBoundary component (referencing ErrorBoundary, errorMessage, and
componentStack) and replace the hardcoded strings with t('中文 key for Error') and
t('中文 key for Component Stack'); ensure the chosen Chinese keys are added to the
locale JSONs under web/src/i18n/locales/{lang}.json as flat keys and call t(...)
where the divs render the headings.
Comment on lines
72
to
82
| setHomePageContent(content); | ||
| localStorage.setItem('home_page_content', content); | ||
|
|
||
| // 如果内容是 URL，则发送主题模式 | ||
| if (data.startsWith('https://')) { | ||
| if (rawContent.startsWith('https://')) { | ||
| const iframe = document.querySelector('iframe'); | ||
| if (iframe) { | ||
| iframe.onload = () => { | ||
| iframe.contentWindow.postMessage({ themeMode: actualTheme }, '*'); | ||
| iframe.contentWindow.postMessage({ lang: i18n.language }, '*'); | ||
| iframe.contentWindow.postMessage({ themeMode: 'light' }, '*'); | ||
| iframe.contentWindow.postMessage({ lang: currentLanguage }, '*'); | ||
| }; |
Contributor
There was a problem hiding this comment.
Attach iframe messaging after the iframe is rendered.
setHomePageContent(content) only schedules the rerender. On a cold load, querySelector('iframe') runs before the iframe exists, so the onload handler is never registered and the external page usually never receives the initial lang payload.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/pages/Home/index.jsx` around lines 72 - 82, The postMessage handler
is being attached before the iframe is rendered because
setHomePageContent(content) is async; update the logic so messaging is attached
after the iframe actually mounts: after calling setHomePageContent (and when
rawContent.startsWith('https://')), move the iframe query/iframe.onload
registration into a follow-up effect or callback that runs after the DOM updates
(e.g., a useEffect that depends on the state updated by setHomePageContent or a
ref callback for the iframe), locate the code around setHomePageContent,
rawContent, document.querySelector('iframe') and iframe.onload, and ensure you
add and clean up the load listener before calling
iframe.contentWindow.postMessage({ themeMode: 'light' }) and postMessage({ lang:
currentLanguage }).
Comment on lines
+80
to
+81
| iframe.contentWindow.postMessage({ themeMode: 'light' }, '*'); | ||
| iframe.contentWindow.postMessage({ lang: currentLanguage }, '*'); |
Contributor
There was a problem hiding this comment.
Don't hardcode the iframe theme to light.
This regresses dark/auto mode for external homepages. web/src/hooks/common/useHeaderBar.js:106-117 already uses actualTheme for iframe sync, but this code now forces 'light' during the homepage handshake.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/pages/Home/index.jsx` around lines 80 - 81, The iframe handshake in
Home/index.jsx is forcing themeMode to 'light' which breaks dark/auto syncing;
update the postMessage to send the current theme value (use the same actualTheme
/ theme variable used in useHeaderBar.js) instead of the hardcoded 'light', and
add a safe fallback (e.g., 'light' only if actualTheme is undefined) so
iframe.contentWindow.postMessage uses the real theme state.
Comment on lines
+132
to
+160
| value={`${serverAddress}${endpointItems[endpointIndex] || ''}`} | ||
| className='flex-1 !rounded-full' | ||
| size={isMobile ? 'default' : 'large'} | ||
| suffix={ | ||
| <div className='flex items-center gap-2'> | ||
| <ScrollList | ||
| bodyHeight={32} | ||
| style={{ border: 'unset', boxShadow: 'unset' }} | ||
| > | ||
| <ScrollItem | ||
| mode='wheel' | ||
| cycled={true} | ||
| list={endpointItems} | ||
| selectedIndex={endpointIndex} | ||
| onSelect={({ index }) => setEndpointIndex(index)} | ||
| /> | ||
| </ScrollList> | ||
| <Button | ||
| type='primary' | ||
| onClick={handleCopyBaseURL} | ||
| icon={<IconCopy />} | ||
| className='!rounded-full' | ||
| /> | ||
| </div> | ||
| <Button | ||
| type='primary' | ||
| onClick={handleCopyBaseURL} | ||
| icon={<IconCopy />} | ||
| className='!rounded-full' | ||
| /> | ||
| } | ||
| /> | ||
| </div> | ||
| {endpointItems.length > 0 && ( | ||
| <div className='flex flex-wrap justify-center gap-2 mt-4 max-w-3xl'> | ||
| {endpointItems.slice(0, 6).map((endpoint) => ( | ||
| <button | ||
| key={endpoint} | ||
| type='button' | ||
| className={`px-3 py-1.5 rounded-full text-xs md:text-sm border transition-colors ${ | ||
| endpoint === endpointItems[endpointIndex] | ||
| ? 'bg-blue-500 text-white border-blue-500' | ||
| : 'bg-transparent text-semi-color-text-1 border-semi-color-border' | ||
| }`} | ||
| onClick={() => | ||
| setEndpointIndex(endpointItems.indexOf(endpoint)) | ||
| } | ||
| > | ||
| {endpoint} |
Contributor
There was a problem hiding this comment.
Render the endpoint value, not the whole object.
endpointItems contains { value } objects, so ${endpointItems[endpointIndex]}, key={endpoint}, and {endpoint} all stringify to [object Object]. That breaks the displayed base URL and gives every button the same React key.
Proposed fix
- value={`${serverAddress}${endpointItems[endpointIndex] || ''}`}
+ value={`${serverAddress}${endpointItems[endpointIndex]?.value || ''}`}
@@
- {endpointItems.slice(0, 6).map((endpoint) => (
+ {endpointItems.slice(0, 6).map(({ value }) => (
<button
- key={endpoint}
+ key={value}
type='button'
className={`px-3 py-1.5 rounded-full text-xs md:text-sm border transition-colors ${
- endpoint === endpointItems[endpointIndex]
+ value === endpointItems[endpointIndex]?.value
? 'bg-blue-500 text-white border-blue-500'
: 'bg-transparent text-semi-color-text-1 border-semi-color-border'
}`}
- onClick={() =>
- setEndpointIndex(endpointItems.indexOf(endpoint))
- }
+ onClick={() =>
+ setEndpointIndex(
+ endpointItems.findIndex((item) => item.value === value),
+ )
+ }
>
- {endpoint}
+ {value}
</button>
))}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/pages/Home/index.jsx` around lines 132 - 160, endpointItems contains
objects like { value }, but the JSX treats each item as a string, causing
[object Object] rendering and duplicate keys; update the input value to use
endpointItems[endpointIndex]?.value (e.g.,
value={`${serverAddress}${endpointItems[endpointIndex]?.value || ''}`), change
the map to use .map((endpoint, idx) => ...) and use key={endpoint.value},
display {endpoint.value}, compare against endpointItems[endpointIndex]?.value
for active styling, and setEndpointIndex(idx) in the onClick handler (avoid
indexOf on objects).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Important
📝 变更描述 / Description
(简述：做了什么？为什么这样改能生效？请基于你对代码逻辑的理解来写，避免粘贴未经整理的内容)
🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
(请在此粘贴截图、关键日志或测试报告，以证明变更生效)
Summary by CodeRabbit
Bug Fixes
Improvements
Chores