UN-3308 [MISC] Add plugin-gated banner mount in PageLayout#2029
UN-3308 [MISC] Add plugin-gated banner mount in PageLayout#2029athul-rs wants to merge 6 commits into
Conversation
The MarketplacePendingBanner component (cloud plugin, #1532 in unstract-cloud) was exported but mounted nowhere, so the 'your marketplace purchase is being confirmed' state never showed. Mount it at the top of PageLayout using the established plugin-gated dynamic import — OSS builds without the plugin leave it unmounted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Summary by CodeRabbit
WalkthroughAdds a shared ChangesMarketplace Plugin Integration with Dynamic Import
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
| Filename | Overview |
|---|---|
| frontend/src/helpers/pluginLoader.js | New zero-dependency helper that centralises isModuleMissing; adds the 'Optional plugin not available' Vite stub match missing from the old useMainAppRoutes.js version, fixing spurious console errors in OSS production builds. |
| frontend/src/layouts/page-layout/PageLayout.jsx | Adds a TLA-gated marketplace banner slot above Outlet; follows the established plugin-gating pattern with correct isModuleMissing error classification and a safe undefined-guard before rendering. |
| frontend/src/routes/Router.jsx | Swaps the isModuleMissing import from useMainAppRoutes.js to the new helpers/pluginLoader.js; no logic changes. |
| frontend/src/routes/useMainAppRoutes.js | Removes the local isModuleMissing definition and imports it from helpers/pluginLoader.js; all catch blocks now also benefit from the added 'Optional plugin not available' match. |
Sequence Diagram
sequenceDiagram
participant Vite as Vite bundler
participant PL as PageLayout.jsx (TLA)
participant PLdr as helpers/pluginLoader.js
participant Mkt as plugins/marketplace (or stub)
Vite->>PL: initialise module (TLA)
PL->>PLdr: "import { isModuleMissing }"
PLdr-->>PL: function returned
PL->>Mkt: await import()
alt Plugin present
Mkt-->>PL: MarketplacePendingBanner exported
PL->>PL: MarketplacePendingBanner assigned
else Plugin absent
Mkt-->>PL: throws Error optional plugin not available
PL->>PLdr: isModuleMissing(err) true
PL->>PL: MarketplacePendingBanner stays undefined
else Plugin broken
Mkt-->>PL: throws unexpected error
PL->>PLdr: isModuleMissing(err) false
PL->>PL: console.error import failed
end
PL-->>Vite: module ready
Reviews (6): Last reviewed commit: "UN-3308 Recognize Vite optional-plugin s..." | Re-trigger Greptile
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/layouts/page-layout/PageLayout.jsx`:
- Around line 21-25: The current empty catch in PageLayout.jsx swallows all
import errors for the marketplace plugin; change it to catch the error (e.g.,
catch (err)) and only suppress it when the error indicates the module is missing
(same contract used in Router.jsx and useMainAppRoutes.js — check err.code ===
'MODULE_NOT_FOUND' or err.message for "Cannot find module"); for any other error
log the full error (or rethrow) so runtime/syntax failures in the plugin aren’t
silently ignored, and only set MarketplacePendingBanner when the import
succeeds.
🪄 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: 533c278f-52bf-4a0e-a39a-839a8142acca
📒 Files selected for processing (1)
frontend/src/layouts/page-layout/PageLayout.jsx
Mirror Router.jsx: stay quiet for the expected plugin-absent OSS case, log unexpected plugin failures so a broken plugin can't silently remove the pending-purchase UX in cloud builds. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PageLayout importing it from useMainAppRoutes created an import cycle (useMainAppRoutes -> PageLayout -> useMainAppRoutes). Move the helper to helpers/pluginLoader.js so Router, useMainAppRoutes, and PageLayout all import it cycle-free. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The layout-side change is a generic plugin-gated mount slot; keep the cloud feature's product narrative out of the OSS code comments.
jaseemjaskp
left a comment
There was a problem hiding this comment.
Automated PR Review Toolkit pass (code-review / silent-failure / type-design / test-coverage / comment / simplify), scoped to the 4 changed files. The extraction is clean and consistent with the existing top-level-await import plugin pattern, and the previously-raised "catch swallows unexpected failures" concern is resolved. Two new, higher-value findings are posted inline on pluginLoader.js; lower-priority/design notes are summarized in the PR thread rather than posted inline to avoid clutter.
The shipped bundle resolves an absent optional plugin to a stub that throws 'Optional plugin not available' (vite.config.js optionalPluginImports), not a fetch/MODULE_NOT_FOUND error — so the OSS 'missing plugin' case was misclassified and every OSS load logged the console.error the helper was meant to suppress. Match that message; also generalize the stale de-register-the-route comment to cover the PageLayout banner mount. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Frontend Lint Report (Biome)✅ All checks passed! No linting or formatting issues found. |
|



What
PageLayoutfor an optional status banner contributed by themarketplaceplugin, when present.isModuleMissinginto a zero-dependencyhelpers/pluginLoader.js(it previously lived inuseMainAppRoutes.js, which would have created an import cycle withPageLayout);Router.jsxanduseMainAppRoutes.jsnow import it from there.Why
Router.jsx/useMainAppRoutes.js(UN-3308 [MISC] Register marketplace landing routes (plugin-gated) #2022 pattern), but there was no hook point for plugin-contributed UI inside the persistent page chrome. The banner component ships with the plugin; the layout only needs a gated slot.How
Router.jsx,TopNavBar):await import("../../plugins/marketplace")inside try/catch withisModuleMissing— missing plugin is the expected quiet case, any other failure is logged.<Outlet />; the component self-manages visibility and renders nothing when it has nothing to show.Can this PR break any existing features? If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)
undefined, and nothing renders — verified withvite buildboth without the plugin and with it present. TheisModuleMissingmove is a pure relocation (same implementation, all consumers updated).Relevant Docs
Router.jsx)Related Issues or PRs
Dependencies Versions / Env Variables
Notes on Testing
vite buildgreen with and without the plugin directory present; biome clean.Screenshots
Checklist
I have read and understood the Contribution Guidelines.
🤖 Generated with Claude Code