feat(extensions): implement dynamic UI rendering for extension panels#1953
feat(extensions): implement dynamic UI rendering for extension panels#1953
Conversation
- Modify extension build to use IIFE format with globals instead of ESM - Add esbuild plugin to resolve external dependencies to window globals - Create extension-globals.ts to expose React, ReactDOM, jsx-runtime, and @hypr/ui components - Initialize globals in main.tsx before app renders - Implement loadExtensionUI function in registry.ts to dynamically load extension scripts - Update TabContentExtension to show loading state and load extension UI on mount - Use convertFileSrc to convert filesystem paths to URLs for script loading Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
✅ Deploy Preview for hyprnote-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for hyprnote ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughAdds runtime extension UI loading: app initializes extension globals, panels request UI via a new loadExtensionUI flow that fetches and executes IIFE extension bundles (which set __hypr_panel_exports), registers default exports as components, and manages per-panel load state and errors. Changes
Sequence DiagramsequenceDiagram
participant Panel as Extension Panel (index.tsx)
participant Registry as Extension Registry (registry.ts)
participant Fetch as Tauri/File Fetch
participant Window as Window Globals
participant Extension as Extension IIFE Bundle
Panel->>Registry: request component for extensionId
Registry->>Registry: is component registered?
alt not registered
Registry-->>Panel: indicate loading
Panel->>Panel: set loadState="loading"
Registry->>Fetch: fetch entry_path via convertFileSrc
Fetch->>Extension: load/execute IIFE script
Extension->>Window: read globals (__hypr_react, __hypr_ui, etc.)
Extension->>Window: set __hypr_panel_exports (bundle exports)
Registry->>Window: read __hypr_panel_exports
Registry->>Registry: extract default export
Registry->>Registry: registerExtensionComponent(extensionId, component)
Registry-->>Panel: resolve true
Panel->>Panel: set loadState="loaded" and render component
else registered
Registry-->>Panel: return component immediately
Panel->>Panel: render component
end
alt load error
Panel->>Panel: set loadState="error" and show fallback message
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/desktop/src/extension-globals.ts (1)
19-28: Keep__hypr_uikeys aligned with the build-time externals mappingThe keys:
window.__hypr_ui = { "components/ui/button": Button, "components/ui/card": Card, };must stay in lockstep with the subpaths computed in
extensions/build.mjs(e.g.@hypr/ui/components/ui/button→"components/ui/button"). If new UI modules are exposed to extensions, they will need matching entries here; otherwise imports will resolve toundefinedat runtime.It may be worth centralizing these subpath strings into a small shared constant module used by both the build script and this initializer to avoid subtle mismatches.
extensions/build.mjs (1)
48-50: IIFE/globalName wiring matches runtime usage; tighten externals handling to avoid silent failuresThe switch to:
format: "iife", globalName: "__hypr_panel_exports",correctly lines up with the runtime code that reads
window.__hypr_panel_exportsinregistry.ts. The externals plugin also matches the globals initialized inextension-globals.ts, which is good.Two things to consider tightening:
- Silent stubbing for unknown
hypr-globalpathsreturn { contents: "module.exports = {}", loader: "js" };This will happily succeed the build but give extensions an empty module at runtime if:
- A new
@hypr/uior@hypr/utilspath is added but not wired intowindow.__hypr_ui/window.__hypr_utils, or- An import is misspelled.
It’s usually preferable for those to be hard build failures. For example, you could
throwhere or at leastconsole.errorand fail the build so issues surface early.
@hypr/utilssubpaths always mapped to rootif (args.path.startsWith("@hypr/utils")) { return { contents: "module.exports = window.__hypr_utils", loader: "js", }; }This assumes extensions will only ever import the root
@hypr/utils. If someone imports a subpath (e.g.@hypr/utils/foo), they’ll still get the root module, which may be confusing.If subpath imports are not intended, you might restrict the resolve filter to
^@hypr/utils$and let other paths fail, or add explicit handling similar to@hypr/ui.Both changes would make extension builds fail fast instead of producing subtle runtime issues.
Also applies to: 53-124
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/desktop/src/components/main/body/extensions/index.tsx(4 hunks)apps/desktop/src/components/main/body/extensions/registry.ts(3 hunks)apps/desktop/src/extension-globals.ts(1 hunks)apps/desktop/src/main.tsx(2 hunks)extensions/build.mjs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
If there are many classNames with conditional logic, usecn(import from@hypr/utils). It is similar toclsx. Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/desktop/src/main.tsxapps/desktop/src/components/main/body/extensions/registry.tsapps/desktop/src/components/main/body/extensions/index.tsxapps/desktop/src/extension-globals.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Agent implementations should use TypeScript and follow the established architectural patterns defined in the agent framework
Agent communication should use defined message protocols and interfaces
Files:
apps/desktop/src/components/main/body/extensions/registry.tsapps/desktop/src/extension-globals.ts
🧠 Learnings (1)
📚 Learning: 2025-11-24T16:32:19.706Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:32:19.706Z
Learning: Applies to **/*.{ts,tsx} : Use `motion/react` instead of `framer-motion`.
Applied to files:
apps/desktop/src/components/main/body/extensions/index.tsx
🧬 Code graph analysis (3)
apps/desktop/src/main.tsx (1)
apps/desktop/src/extension-globals.ts (1)
initExtensionGlobals(19-29)
apps/desktop/src/components/main/body/extensions/registry.ts (2)
apps/desktop/src/types/extensions.d.ts (1)
ExtensionViewProps(3-6)extensions/hello-world/ui.tsx (1)
ExtensionViewProps(13-16)
apps/desktop/src/components/main/body/extensions/index.tsx (2)
apps/desktop/src/components/main/body/extensions/registry.ts (3)
getExtensionComponent(27-34)getPanelInfoByExtensionId(71-76)loadExtensionUI(112-174)apps/desktop/src/components/main/body/index.tsx (1)
StandardTabWrapper(349-368)
⏰ 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). (7)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
- GitHub Check: fmt
- GitHub Check: ci (linux, depot-ubuntu-22.04-8)
- GitHub Check: ci (linux, depot-ubuntu-24.04-8)
- GitHub Check: ci (macos, depot-macos-14)
🔇 Additional comments (3)
apps/desktop/src/extension-globals.ts (1)
9-16: Clarify lifecycle/optionality ofwindow.__hypr_*globalsThese properties are declared as required on
Window, but only become available afterinitExtensionGlobals()runs. For tests or any code that might touch them before bootstrap, this can be surprising.Consider either:
- Marking them as optional (
__hypr_react?: typeof React, etc.) and narrowing at call sites, or- Adding a brief doc comment on
initExtensionGlobals()stating it must run before any extension script executes.This helps avoid accidental early access in future refactors.
apps/desktop/src/main.tsx (1)
20-21: Bootstrap ordering for extension globals looks reasonableCalling
initExtensionGlobals()at startup, right afterinitWindowsPlugin(), ensures thewindow.__hypr_*globals are ready before any extension UI tries to load, and it runs only once per webview.Just make sure no extension-related code can run before
main.tsxexecutes (e.g., alternative entry points) so those globals are always initialized first.Also applies to: 89-91
apps/desktop/src/components/main/body/extensions/index.tsx (1)
1-3: Imports and motion/react usage look aligned with guidelinesUsing
motion/reactandcnhere is consistent with the repo’s conventions and the documented preference overframer-motion. The registry imports also neatly centralize extension loading concerns instead of duplicating logic in the UI layer.Also applies to: 17-21
…ading Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
- Add clean, install commands to build script - Add colored logging and better error handling - Add extensions.mdx documentation for developers - Update package.json with new scripts Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
extensions/build.mjs (2)
243-285: Install helpers work; consider propagating failures and scoping copied filesThe install flow correctly validates
extension.json, mirrors the extension into the computed app-data extensions dir, and logs the final target path. Two optional refinements:
installAll()ignores the boolean result frominstallExtension, so a missing/broken extension still yields exit code 0. Mirroring thebuildAll()pattern (track asuccessflag and exit 1 on any failure) would make failures more visible in CI/scripts.copyRecursivecopies the entire extension directory, includingnode_modulesand source files. For dev this is acceptable, but you may want to consider excluding obvious heavy directories (e.g.,node_modules) or restricting toextension.json,dist/, and asset folders if that’s sufficient for runtime.
6-8: CLI argument parsing and command handling are good; legacy mode could be a bit stricterParsing
process.argv.slice(2)intocommand/extensionName, handlingbuild|clean|install|help, and exiting with 1 on build failures is all well-structured, and themain().catchguard gives you a clean failure mode.One minor UX edge case: in the
defaultbranch, any non-flag first arg is treated as a legacy “extension name” (e.g.,node build.mjs my-ext). That preserves backwards compatibility, but also means a typo likenode build.mjs biuldtries to build a non-existent extension instead of immediately flagging an unknown command. You might optionally gate this legacy path on the extension actually existing (or on an explicit flag) and otherwise print usage.Also applies to: 287-341
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/content/docs/developers/8.extensions.mdx(1 hunks)extensions/build.mjs(2 hunks)extensions/package.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/web/content/docs/developers/8.extensions.mdx
🧰 Additional context used
🧬 Code graph analysis (1)
extensions/build.mjs (1)
apps/desktop/src/components/main/body/extensions/registry.ts (1)
getExtensionsDir(59-65)
⏰ 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). (8)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
- GitHub Check: ci (linux, depot-ubuntu-24.04-8)
- GitHub Check: ci (linux, depot-ubuntu-22.04-8)
- GitHub Check: fmt
- GitHub Check: ci (macos, depot-macos-14)
- GitHub Check: ci (macos, macos-14)
🔇 Additional comments (6)
extensions/package.json (1)
7-10: Scripts correctly wired to the new multi-command CLI
build,build:hello-world,clean, andinstall:devall delegate cleanly tobuild.mjswith the expected subcommands, matching the CLI usage inbuild.mjs. No issues from a tooling or ergonomics standpoint.extensions/build.mjs (5)
1-4: Importingosfor platform-specific extension paths is appropriateUsing
os.platform()/os.homedir()alongsidepathandfsto compute the extensions directory is the right choice for this Node-based CLI; nothing to fix here.
201-224: Extension discovery and aggregated build result behavior look correct
getExtensionDirs()sensibly filters to non-dot, non-node_modulesdirectories that contain anextension.json, andbuildAll()aggregates their build results into a single success flag used by the CLI for exit codes. This is a clean and predictable setup.
226-241: Clean helpers are straightforward and safe
cleanExtensiononly removes<name>/distwhen present and logs clearly;cleanAllfans this out over discovered extensions. No correctness or UX issues here.
9-55: I'll help you verify the concerns raised in the review comment. Let me search the codebase to understand the scope of the issue and check for potential sync problems between the CLI and runtime.
<function_calls>
#!/bin/bashSearch for app ID and extensions dir references
echo "=== Searching for com.hyprnote.dev ==="
rg -n "com.hyprnote.dev" -C 2echo ""
echo "=== Searching for getExtensionsDir ==="
rg -n "getExtensionsDir" -C 2echo ""
echo "=== Searching for extensions path patterns ==="
rg -n ".local.*share|AppData.*Roaming|Library.*Application Support" -C 2 | head -50
</function_calls>
57-199: globals contract is correctly aligned; fallback behavior remains a latent fragilityThe
@hypr/uisubpath extraction in the plugin correctly derives"components/ui/button"and"components/ui/card"from imports, which precisely match the keys defined inwindow.__hypr_uiinapps/desktop/src/extension-globals.ts. The example extension (extensions/hello-world/ui.tsx) imports only from these two components, confirming current alignment.However, the original review concern stands: the
onLoadfallback returns an empty object for any unrecognized@hypr/uisubpath (e.g., if an extension tries to import@hypr/ui/components/ui/accordionbefore it's exported in extension-globals). This masks what should be a build-time failure and defers the error to runtime. While currently not manifesting (only button and card are exported and used), this pattern creates technical debt as more UI components are added.The subpath extraction logic and globals initialization are sound for the present state, but consider adding explicit validation or warnings in the plugin's
onLoadhandler when accessing undefined keys inwindow.__hypr_uito catch integration mismatches earlier.
Summary
This PR implements the missing UI rendering part for extension panels, completing the work started in PRs #1938, #1945, and #1949. Previously, extension panels were registered but showed "UI not loaded" because there was no mechanism to actually load and render the extension's React components.
The solution uses a globals-based approach where:
__hypr_react,__hypr_jsx_runtime,__hypr_ui, etc.) before any extension loadsconvertFileSrc, injected into the DOM, and the exported component is registeredKey changes:
loadExtensionUI()function that fetches, injects, and registers extension componentsTabContentExtensionto show loading state and trigger dynamic loading$APPDATA/**to asset protocol scope to allow loading extension scripts from the app data directoryUpdates since last revision
clean,installcommands with colored logging and better error handling. Theinstallcommand copies extensions to the platform-specific app data directory for development.apps/web/content/docs/developers/8.extensions.mdxwith comprehensive guide covering extension structure, manifest format, runtime scripts, panel UIs, available globals, and development workflow.cleanandinstall:devscripts for easier development workflow.Tested locally
The extension UI rendering was verified working on Linux:
View original video (rec-1b9c32910eeb41b1bf7b7ba4de6566a8-edited.mp4)
Review & Testing Checklist for Human
$APPDATA/**asset protocol scope addition is acceptable - this allows the app to serve files from the app data directoryextension-globals.ts; extensions using other @hypr/ui components will fail silently8.extensions.mdxfor correctness and completenessTest Plan
ONBOARDING=0 pnpm -F desktop tauri devcd extensions pnpm install pnpm build:hello-world pnpm install:devNotes
document.head.appendChild(script)) is used instead of dynamic import because the extension scripts need access to the host app's React instanceextension-globals.tsinstallcommand in build.mjs handles macOS, Linux, and Windows paths but Windows was not testedLink to Devin run: https://app.devin.ai/sessions/1fe16a177a9a48a48c1d09e80c93bdc1
Requested by: yujonglee (@yujonglee)