-
Notifications
You must be signed in to change notification settings - Fork 130
Bundle Monaco Editor workers #1237
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
Motivation: Central Dogma should be used in an isolated network environment without accessing the internet. However, Monaco Editor dynamically loads workers from internet. See line#1236 for details. Modifications: - Use `monaco-editor-webpack-plugin` to load Monaco Editor resources with WebPack. - Use `loader` from '@monaco-editor/react' to load resources locally instead of CDN - Lazily loading `monaco-editor` in the client side only to avoid accessing the `window` object on the server. Result: - You can now use Central Dogma UI without internet access. - Closes line#1236
📝 WalkthroughWalkthroughThis PR bundles the Monaco Editor locally by extending Next.js webpack configuration with MonacoWebpackPlugin and updating dependencies, then adds loading guards in FileEditor and NewFile components to handle Monaco initialization before rendering. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
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 |
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: 3
🧹 Nitpick comments (1)
webapp/src/dogma/features/file/MonacoLoader.ts (1)
24-46: Prefer explicit typing overanyfor better type safety.Line 26 uses
anyfor the monaco instance, which bypasses TypeScript's type checking and could lead to runtime errors.🔎 Recommended fix to use proper Monaco types
+import type * as monacoType from 'monaco-editor'; + export const useLocalMonaco = () => { - //eslint-disable-next-line @typescript-eslint/no-explicit-any - const [monaco, setMonaco] = useState<any>(null); + const [monaco, setMonaco] = useState<typeof monacoType | null>(null);This provides better IntelliSense and catches type errors at compile time.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
webapp/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
webapp/next.config.jswebapp/package.jsonwebapp/src/dogma/common/components/editor/FileEditor.tsxwebapp/src/dogma/features/file/MonacoLoader.tswebapp/src/dogma/features/file/NewFile.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
webapp/src/dogma/features/file/NewFile.tsx (2)
webapp/src/dogma/features/file/MonacoLoader.ts (1)
useLocalMonaco(24-46)webapp/src/dogma/common/components/Loading.tsx (1)
Loading(3-8)
webapp/src/dogma/common/components/editor/FileEditor.tsx (2)
webapp/src/dogma/features/file/MonacoLoader.ts (1)
useLocalMonaco(24-46)webapp/src/dogma/common/components/Loading.tsx (1)
Loading(3-8)
⏰ 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). (10)
- GitHub Check: build-ubuntu-latest-jdk-17-min-java-11
- GitHub Check: build-macos-latest-jdk-25
- GitHub Check: build-ubuntu-latest-jdk-25-snapshot
- GitHub Check: build-ubuntu-latest-jdk-21
- GitHub Check: build-windows-latest-jdk-25
- GitHub Check: build-ubuntu-latest-jdk-11
- GitHub Check: lint
- GitHub Check: build-ubuntu-latest-jdk-17-coverage
- GitHub Check: flaky-tests
- GitHub Check: docker-build
🔇 Additional comments (3)
webapp/src/dogma/features/file/MonacoLoader.ts (1)
5-22: LGTM: Worker paths align with webpack configuration.The worker URLs correctly match the filename pattern configured in
next.config.jsline 24 (static/[name].worker.js). The check for existingwindow.MonacoEnvironmentprevents overwriting if already set.webapp/src/dogma/features/file/NewFile.tsx (1)
36-37: LGTM: Loading guard correctly prevents premature rendering.The implementation correctly waits for Monaco to load before rendering the editor form. This pattern is consistent with
FileEditor.tsxand prevents errors from accessing Monaco before it's initialized.Also applies to: 143-146
webapp/src/dogma/common/components/editor/FileEditor.tsx (1)
30-31: LGTM: Loading guard ensures Monaco is available before rendering.The loading guard correctly prevents the component from rendering until Monaco is initialized. This pattern is consistent with the implementation in
NewFile.tsxand prevents runtime errors from premature Monaco API access.Also applies to: 111-114
| setMonaco(monacoInstance); | ||
| }) | ||
| .catch((err) => { | ||
| console.error('Failed to load local monaco-editor:', err); |
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.
Noted that if the editor is stuck at <Loading />, checking the console might help debug issues
minwoox
left a comment
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.
👍

Motivation:
Central Dogma should be used in an isolated network environment without accessing the internet. However, Monaco Editor dynamically loads workers from internet. See #1236 for details.
Modifications:
monaco-editor-webpack-pluginto load Monaco Editor resources with WebPack.loaderfrom '@monaco-editor/react' to load resources locally instead of CDNmonaco-editorin the client side only to avoid accessing thewindowobject on the server.Result: