Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Oct 31, 2025

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:

  • Deleted pkg/docsite package entirely
  • Removed docsite handler from pkg/web/web.go
  • Simplified emain/docsite.ts to return constant URL instead of probing for embedded docs

Frontend changes:

  • Removed embedded URL translation logic from helpview.tsx
  • Homepage URL now uses constant instead of IPC call

Build configuration:

  • Removed docsite:build:embedded task and all references
  • Removed dist/docsite/**/* from electron-builder asarUnpack
  • Removed build-embedded script from docs package.json

Net result: -141 lines, +9 lines across 8 files.

Original prompt

right now we build a docusaurus project to run both the online docs at docs.waveterm.dev AND to embed into the application. but this limits what we can show in the docs (especially large things like videos etc.)

the internal docs already fall back to the website when it cannot find the embedded docs.

i'd like to update the build and packaging etc to no longer build the embedded doc site. i'd also like to have the "help" view always render the live doc site at https://docs.waveterm.dev.


💡 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.

Copilot AI self-assigned this Oct 31, 2025
@CLAassistant
Copy link

CLAassistant commented Oct 31, 2025

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ sawka
❌ Copilot
You have signed the CLA already but the status is still pending? Let us recheck it.

Co-authored-by: sawka <2722291+sawka@users.noreply.github.com>
Copilot AI changed the title [WIP] Update build to remove embedded documentation site Remove embedded docsite, always use live docs.waveterm.dev Oct 31, 2025
Copilot AI requested a review from sawka October 31, 2025 23:56
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 4, 2025

Walkthrough

This pull request removes embedded docsite functionality across the entire codebase. Changes include deleting the docsite:build:embedded Taskfile task and its build script dependencies, eliminating the Electron main process docsite initialization module (emain/docsite.ts), removing the IPC-based docsite URL retrieval API (getDocsiteUrl), replacing dynamic docsite URL logic in the help view with a static constant, and deleting the Go backend module (pkg/docsite/docsite.go) that served static docsite files. The electron builder configuration no longer unpacks docsite files, and web server route registration is updated to exclude the docsite path.

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:

  • Verify no orphaned references to getDocsiteUrl() remain elsewhere in the codebase (grep/search for remnants)
  • Confirm the hardcoded docsiteUrl constant introduced in helpview.tsx is the intended replacement behavior and that its value is correct
  • Check that the removal of the docsite route in pkg/web/web.go doesn't break any remaining navigation or routing logic
  • Ensure the TypeScript type definitions in frontend/types/custom.d.ts are now consistent with the removed ElectronApi method
  • Validate that the electron builder configuration changes don't affect packaging or distribution of other resources

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and accurately summarizes the main change: removing the embedded docsite and switching to use the live docs.waveterm.dev instead.
Description check ✅ Passed The description is well-written and directly related to the changeset, providing comprehensive context about backend, frontend, and build configuration changes.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch copilot/remove-embedded-docs

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sawka sawka marked this pull request as ready for review November 4, 2025 18:06
@sawka
Copy link
Member

sawka commented Nov 4, 2025

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 4, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between a5fb38e and 1d69c02.

📒 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 initDocsite import and call is consistent with the PR objectives to eliminate embedded docsite functionality. The relocation of the initIpcHandlers import 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, and fireAndForget properly 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 docsiteUrl directly 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=app returns a 200 status code and is accessible. The hardcoded URL supports the PR objective to always use live documentation, with the ?ref=app parameter enabling tracking.

Comment on lines 126 to 132
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>
);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

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.

@sawka sawka merged commit a23bbc0 into main Nov 4, 2025
8 of 10 checks passed
@sawka sawka deleted the copilot/remove-embedded-docs branch November 4, 2025 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants