-
Notifications
You must be signed in to change notification settings - Fork 689
Remove embedded docsite, always use live docs.waveterm.dev #2503
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
Conversation
|
|
Co-authored-by: sawka <2722291+sawka@users.noreply.github.com>
WalkthroughThis pull request removes embedded docsite functionality across the entire codebase. Changes include deleting the Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes The changes form a coherent removal pattern affecting multiple layers consistently—primarily deletions of docsite-related code across build system, Electron main process, frontend components, backend Go code, and type definitions. However, the distributed nature across different systems and languages requires verification of each removal area independently. Areas requiring extra attention during review:
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
Taskfile.yml(0 hunks)docs/package.json(0 hunks)electron-builder.config.cjs(0 hunks)emain/docsite.ts(0 hunks)emain/emain.ts(1 hunks)emain/preload.ts(0 hunks)frontend/app/view/helpview/helpview.tsx(3 hunks)frontend/types/custom.d.ts(0 hunks)pkg/docsite/docsite.go(0 hunks)pkg/web/web.go(0 hunks)
💤 Files with no reviewable changes (8)
- pkg/web/web.go
- emain/docsite.ts
- emain/preload.ts
- docs/package.json
- pkg/docsite/docsite.go
- electron-builder.config.cjs
- frontend/types/custom.d.ts
- Taskfile.yml
🔇 Additional comments (4)
emain/emain.ts (1)
23-23: LGTM! Clean removal of docsite initialization.The removal of the
initDocsiteimport and call is consistent with the PR objectives to eliminate embedded docsite functionality. The relocation of theinitIpcHandlersimport and the initialization sequence at line 339 remain properly structured.Also applies to: 339-339
frontend/app/view/helpview/helpview.tsx (3)
5-5: LGTM: Clean import removal.The removal of
getApi,useAtomValue,useCallback, andfireAndForgetproperly reflects the simplification of the help view - no more dynamic URL fetching or error callback handling.Also applies to: 9-9
44-44: LGTM: Simplified homepage URL initialization.Using the constant
docsiteUrldirectly eliminates the need for async IPC calls and makes the homepage URL deterministic. This is a clean simplification.
11-11: Docs URL is accessible and live.Verification confirmed that
https://docs.waveterm.dev/?ref=appreturns a 200 status code and is accessible. The hardcoded URL supports the PR objective to always use live documentation, with the?ref=appparameter enabling tracking.
| function HelpView(props: ViewComponentProps<HelpViewModel>) { | ||
| const model = props.model; | ||
| const homepageUrl = useAtomValue(model.homepageUrl); | ||
|
|
||
| // Effect to update the docsite base url when the app restarts, since the webserver port is dynamic | ||
| const onFailLoad = useCallback( | ||
| (url: string) => | ||
| fireAndForget(async () => { | ||
| const newDocsiteUrl = getApi().getDocsiteUrl(); | ||
|
|
||
| // Correct the homepage URL, if necessary | ||
| if (newDocsiteUrl !== homepageUrl) { | ||
| await model.setHomepageUrl(newDocsiteUrl, "block"); | ||
| } | ||
|
|
||
| // Correct the base URL of the current page, if necessary | ||
| const newBaseUrl = baseUrlRegex.exec(newDocsiteUrl)?.[0]; | ||
| const curBaseUrl = baseUrlRegex.exec(url)?.[0]; | ||
| if (curBaseUrl && newBaseUrl && curBaseUrl !== newBaseUrl) { | ||
| model.loadUrl(url.replace(curBaseUrl, newBaseUrl), "fix-fail-load"); | ||
| } | ||
| }), | ||
| [homepageUrl] | ||
| ); | ||
| return ( | ||
| <div className="w-full h-full"> | ||
| <WebView {...props} onFailLoad={onFailLoad} /> | ||
| <WebView {...props} /> | ||
| </div> | ||
| ); | ||
| } |
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.
Consider adding error handling for network failures.
The removal of the onFailLoad callback means users will see a browser error page if they're offline or if the docs site is inaccessible. This degrades the user experience without providing any context.
Consider implementing:
- A custom error message when the docs site fails to load
- An indicator that help requires an internet connection
- A retry mechanism or link to open docs in browser
Would you like me to suggest an implementation for graceful error handling when the docs site is unavailable?
🤖 Prompt for AI Agents
In frontend/app/view/helpview/helpview.tsx around lines 126 to 132, the WebView
no longer handles load failures so users see the browser error page when offline
or the docs site is down; restore graceful error handling by adding an
onFailLoad handler prop to WebView that sets component state (error message and
offline flag), renders a small fallback UI in place of the WebView showing a
clear custom error message and an indicator that help requires internet, and
provide actions to retry loading (re-render WebView or call reload), and an
“Open in external browser” link (or fallback to local embedded docs) so users
can recover. Ensure the handler captures the load error details for logging and
use accessible buttons for retry/open-external.
Removes the embedded Docusaurus build from the application. The help view now always loads from https://docs.waveterm.dev/ instead of serving a locally bundled copy. This eliminates package size constraints on documentation content (e.g., videos) and removes unnecessary build/packaging overhead.
Backend changes:
pkg/docsitepackage entirelypkg/web/web.goemain/docsite.tsto return constant URL instead of probing for embedded docsFrontend changes:
helpview.tsxBuild configuration:
docsite:build:embeddedtask and all referencesdist/docsite/**/*from electron-builder asarUnpackbuild-embeddedscript from docs package.jsonNet result: -141 lines, +9 lines across 8 files.
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.