feat(portal-plugin-billing): add new billing plugin with subscription management and Stripe integration #526
Conversation
… management and Stripe integration - Introduce a new `@lumeweb/portal-plugin-billing` package for handling subscription billing - Add refine-based UI components and hooks for subscription status and customer portal access - Implement Stripe pricing table embedding for plan selection - Configure Module Federation exposes for billing routes - Register plugin capabilities and dependencies within the portal framework - Update app shell to include the new billing plugin - Add build script and shared module configuration for the new plugin - Refactor dashboard plugin's refine config to use new API utility functions - Update shared-modules configuration to include `@lumeweb/advanced-rest-provider`
|
WalkthroughA new billing plugin is introduced to the portal framework, providing Stripe payment integration, subscription management UI, and account portal functionality. The dashboard plugin is refactored to extract and reuse API URL resolution and data provider setup utilities. Changes
Sequence DiagramsequenceDiagram
participant App as App/Framework
participant BillingPlugin as Billing Plugin
participant DashboardPlugin as Dashboard Plugin
participant AdvancedRest as Advanced REST Provider
rect rgb(200, 230, 255)
Note over App,BillingPlugin: Plugin Initialization
App->>BillingPlugin: initialize(framework)
BillingPlugin->>DashboardPlugin: getCapability("dashboard:refine-config")
DashboardPlugin->>BillingPlugin: dashboardCapability (with apiUrl)
BillingPlugin->>BillingPlugin: store apiUrl
end
rect rgb(230, 200, 255)
Note over App,AdvancedRest: Data Provider Setup
App->>AdvancedRest: setupDataProvider(apiUrl, resources)
AdvancedRest->>AdvancedRest: read jwt from localStorage
AdvancedRest->>AdvancedRest: setAuthToken(jwt)
AdvancedRest->>App: configured dataProvider
end
rect rgb(255, 230, 200)
Note over BillingPlugin,AdvancedRest: Subscription Query
App->>BillingPlugin: useSubscription()
BillingPlugin->>AdvancedRest: GET /account/billing/subscription
AdvancedRest->>App: SubscriptionStatusResponse
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Rationale: The PR introduces an entirely new plugin with ~30 files, but most are straightforward configuration re-exports and type definitions. The more complex pieces—subscription page with conditional rendering, Stripe integration, and dashboard plugin utility extraction—are moderate in density. Changes are primarily additive (new plugin) with focused refactoring in the dashboard plugin's API URL and data provider logic, making this moderately heterogeneous but relatively clear in intent. Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
shared-modules.ts (1)
3-14: The proposed fix is incomplete and will cause version mismatches.The review correctly identifies the singleton conflict, but the suggested override to
"react-router": "7.6.0"is problematic:
- Five packages explicitly pin to
7.5.2(libs/portal-plugin-billing, libs/portal-plugin-template, libs/portal-plugin-ipfs, libs/portal-plugin-dashboard, libs/portal-plugin-abuse-report)- Pinning the override to
7.6.0will violate these fixed constraints, forcing resolution to incompatible versions and defeating the singleton patternTo resolve the conflict, either:
- Update all
"react-router": "7.5.2"to"7.6.0"across the five plugins, then add the override- Or downgrade the app-shell and framework packages from
^7.6.0to^7.5.2The review's current diff only adds the override without addressing the dependent package declarations, leaving the singleton unresolved.
libs/portal-plugin-dashboard/src/capabilities/refineConfig.ts (2)
46-51: Fix route template path (missing leading slash)The “operations/filters” resource template lacks a leading “/”, unlike others. This likely breaks routing.
meta: { dataProviderName: DATA_PROVIDER_NAME, - template: "operations/filters", + template: "/operations/filters", }, name: "operations.filters",
25-55: Guard against using getConfig before initializeIf getConfig is called before initialize sets #apiUrl, setup will run with undefined.
getConfig(existing?: Partial<RefineProps>) { + if (!this.#apiUrl) { + throw new Error("Capability not initialized: apiUrl is missing"); + } const dashboardResources = [ { meta: { template: "/account" }, name: DATA_PROVIDER_NAME, },
🧹 Nitpick comments (10)
package.json (1)
17-17: Build script added — good.Consider adding a matching CI job (or turbo pipeline target) so the package is exercised in PRs.
libs/portal-plugin-billing/tailwind.config.ts (1)
13-17: Verify Tailwind “layers” usage.Tailwind config doesn’t natively recognize a “layers” field; if your build tooling relies on it, fine. Otherwise, this has no effect and can confuse readers. Confirm intent (plugin relying on host CSS, no local layer output).
libs/portal-plugin-billing/package.json (1)
19-27: Align react-router version to resolve incompatible ranges across packages.Verification confirms version mismatch: portal-plugin-billing declares react-router 7.5.2 (exact) while portal-app-shell and multiple frameworks declare ^7.6.0. These incompatible ranges will cause npm to install both versions, creating the dual-install issue mentioned. Aligning to ^7.6.0 or 7.6.0 resolves this. The @refinedev/core suggestion is also valid for explicitness, though ^4.47.0 technically allows 4.57.10 within its caret range.
- "@refinedev/core": "^4.47.0", + "@refinedev/core": "4.57.10", @@ - "react-router": "7.5.2" + "react-router": "7.6.0"libs/portal-plugin-billing/src/index.ts (1)
13-26: Consider using a proper logging framework instead of console.log.The lifecycle methods use
console.logfor logging. For production-grade plugins, consider using a structured logging framework that can be configured for different environments and log levels.Example refactor:
- async destroy(_framework: Framework) { - console.log("Plugin Billing destroyed"); - }, + async destroy(framework: Framework) { + framework.logger?.info("Plugin Billing destroyed"); + },- async initialize(_framework: Framework) { - console.log("Plugin Billing initialized"); - }, + async initialize(framework: Framework) { + framework.logger?.info("Plugin Billing initialized"); + },libs/portal-plugin-billing/src/ui/components/StripePricingTable.tsx (1)
27-35: Consider adding a loading state for better UX.The component renders the
<stripe-pricing-table>element immediately, but the Stripe script may not have loaded yet. This could result in a blank area or a flash of unstyled content.Example implementation:
export const StripePricingTable: React.FC<StripePricingTableProps> = ({ pricingTableId, publishableKey, customerEmail, clientReferenceId }) => { + const [isLoaded, setIsLoaded] = React.useState(false); const [error, setError] = React.useState<string | null>(null); React.useEffect(() => { const existingScript = document.querySelector( 'script[src="https://js.stripe.com/v3/pricing-table.js"]' ); if (existingScript) { + setIsLoaded(true); return; } const script = document.createElement("script"); script.src = "https://js.stripe.com/v3/pricing-table.js"; script.async = true; + script.onload = () => { + setIsLoaded(true); + }; + script.onerror = () => { setError("Failed to load Stripe pricing table. Please refresh the page."); }; document.body.appendChild(script); }, []); if (error) { return <div className="text-red-600">{error}</div>; } + if (!isLoaded) { + return <div>Loading pricing options...</div>; + } + return ( <stripe-pricing-table pricing-table-id={pricingTableId} publishable-key={publishableKey} {...(customerEmail && { "customer-email": customerEmail })} {...(clientReferenceId && { "client-reference-id": clientReferenceId })} /> ); };libs/portal-plugin-billing/src/hooks/useSubscription.ts (1)
6-14: Export the config type and tidy queryOptions usage
- Interface isn’t exported; consumers can’t type the config.
- queryOptions spread has no defaults; the “allow overrides” comment is misleading.
Consider:
- Exporting UseSubscriptionConfig.
- Either remove the comment or add sensible defaults and then spread user options last.
-interface UseSubscriptionConfig { +export interface UseSubscriptionConfig { queryOptions?: UseCustomProps< SubscriptionStatusResponse, unknown, unknown, unknown, SubscriptionStatusResponse >["queryOptions"]; } @@ return useCustom<SubscriptionStatusResponse>({ url: "/account/billing/subscription", method: "get", dataProviderName: DATA_PROVIDER_NAME, - queryOptions: { - // Apply user-provided query options last to allow overrides - ...queryOptions, - }, + queryOptions: { + // sensible defaults; user options override + // staleTime: 30_000, + // refetchOnWindowFocus: false, + ...(queryOptions ?? {}), + }, });Also applies to: 23-27
libs/portal-plugin-billing/src/hooks/useCustomerPortal.ts (1)
6-13: Export types and stabilize mutate
- Exporting the config/result types improves DX for consumers.
- Wrap mutate in useCallback to keep a stable reference.
-interface UseCustomerPortalConfig { +export interface UseCustomerPortalConfig { return_url?: string; mutationOptions?: UseCustomMutationProps< CustomerPortalResponse, HttpError, unknown >["mutationOptions"]; } -interface UseCustomerPortalResult { +export interface UseCustomerPortalResult { mutate: () => void; isLoading: boolean; data?: CustomerPortalResponse; error?: HttpError | null; } @@ - const mutate = () => { - mutation.mutate({ - url: "/account/billing/customer-portal", - method: "post", - values: { - return_url, - }, - dataProviderName: DATA_PROVIDER_NAME, - }); - }; + const mutate = React.useCallback(() => { + mutation.mutate({ + url: "/account/billing/customer-portal", + method: "post", + values: { return_url }, + dataProviderName: DATA_PROVIDER_NAME, + }); + }, [mutation, return_url]);Also applies to: 15-20, 31-40
libs/portal-plugin-billing/src/ui/routes/account.subscriptions.tsx (2)
84-97: Handle identity fetch errors tooCurrently only subscription error is handled. If identity fails, UI proceeds without feedback.
- const { data: identity, isLoading: isIdentityLoading } = useGetIdentity<Identity>(); + const { + data: identity, + isLoading: isIdentityLoading, + error: identityError, + } = useGetIdentity<Identity>(); @@ - if (error) { + if (error || identityError) { return ( <div className="container mx-auto px-4 py-12"> <div className="text-center"> <h2 className="mb-4 text-2xl font-semibold text-red-600"> - Error Loading Subscription + Error Loading Account </h2> <p className="text-muted-foreground"> Unable to load your subscription information. Please try again later. </p>Also applies to: 129-144
146-149: Coerce hasStripeConfig to boolean and drop redundant toString
- hasStripeConfig currently evaluates to a string; coerce to boolean for clarity.
- Identity.id is already a string; no need for toString().
- const hasStripeConfig = - billingMeta?.stripe_pricing_table_id && billingMeta?.stripe_publishable_key; + const hasStripeConfig = Boolean( + billingMeta?.stripe_pricing_table_id && billingMeta?.stripe_publishable_key, + ); @@ - clientReferenceId={identity?.id?.toString()} + clientReferenceId={identity?.id}Also applies to: 181-186
libs/portal-plugin-dashboard/src-lib/util/dataProvider.ts (1)
24-31: Make localStorage access SSR‑safeAccessing localStorage on the server throws. Guard it.
- const token = localStorage.getItem("jwt"); + let token: string | null = null; + try { + if (typeof window !== "undefined" && window.localStorage) { + token = window.localStorage.getItem("jwt"); + } + } catch { + token = null; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (30)
.git-town.toml(1 hunks)apps/portal-app-shell/package.json(1 hunks)apps/portal-app-shell/src/App.tsx(1 hunks)libs/portal-plugin-billing/package.json(1 hunks)libs/portal-plugin-billing/plugin.config.ts(1 hunks)libs/portal-plugin-billing/postcss.config.cjs(1 hunks)libs/portal-plugin-billing/src/capabilities/index.ts(1 hunks)libs/portal-plugin-billing/src/capabilities/refineConfig.ts(1 hunks)libs/portal-plugin-billing/src/hooks/index.ts(1 hunks)libs/portal-plugin-billing/src/hooks/useCustomerPortal.ts(1 hunks)libs/portal-plugin-billing/src/hooks/useSubscription.ts(1 hunks)libs/portal-plugin-billing/src/index.ts(1 hunks)libs/portal-plugin-billing/src/routes.tsx(1 hunks)libs/portal-plugin-billing/src/types/index.ts(1 hunks)libs/portal-plugin-billing/src/types/subscription.ts(1 hunks)libs/portal-plugin-billing/src/ui/components/StripePricingTable.tsx(1 hunks)libs/portal-plugin-billing/src/ui/components/index.ts(1 hunks)libs/portal-plugin-billing/src/ui/index.ts(1 hunks)libs/portal-plugin-billing/src/ui/routes/account.subscriptions.tsx(1 hunks)libs/portal-plugin-billing/src/ui/routes/index.ts(1 hunks)libs/portal-plugin-billing/tailwind.config.ts(1 hunks)libs/portal-plugin-billing/tsconfig.json(1 hunks)libs/portal-plugin-billing/vite.config.ts(1 hunks)libs/portal-plugin-dashboard/src-lib/index.ts(1 hunks)libs/portal-plugin-dashboard/src-lib/util/api.ts(1 hunks)libs/portal-plugin-dashboard/src-lib/util/dataProvider.ts(1 hunks)libs/portal-plugin-dashboard/src-lib/util/index.ts(1 hunks)libs/portal-plugin-dashboard/src/capabilities/refineConfig.ts(3 hunks)package.json(1 hunks)shared-modules.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (8)
libs/portal-plugin-dashboard/src-lib/util/api.ts (1)
libs/portal-framework-core/src/util/portalMeta.ts (1)
getPluginMeta(62-72)
libs/portal-plugin-billing/tailwind.config.ts (1)
libs/portal-framework-ui-core/config/classlist.ts (2)
tailwindSafelist(86-86)tailwindBlocklist(87-87)
libs/portal-plugin-billing/src/hooks/useCustomerPortal.ts (1)
libs/portal-plugin-billing/src/types/subscription.ts (1)
CustomerPortalResponse(9-11)
libs/portal-plugin-billing/src/ui/routes/account.subscriptions.tsx (6)
libs/portal-plugin-billing/src/hooks/useSubscription.ts (1)
useSubscription(16-28)libs/portal-framework-core/src/types/portal.ts (1)
Identity(1-9)libs/portal-plugin-billing/src/hooks/useCustomerPortal.ts (1)
useCustomerPortal(22-48)libs/portal-framework-ui/src/hooks/usePluginMeta.ts (1)
usePluginMeta(5-11)libs/portal-plugin-billing/src/types/subscription.ts (1)
BillingPluginMeta(13-16)libs/portal-plugin-billing/src/ui/components/StripePricingTable.tsx (1)
StripePricingTable(21-45)
libs/portal-plugin-billing/src/hooks/useSubscription.ts (1)
libs/portal-plugin-billing/src/types/subscription.ts (1)
SubscriptionStatusResponse(1-7)
libs/portal-plugin-dashboard/src-lib/util/dataProvider.ts (1)
libs/portal-framework-core/src/util/refineConfig.ts (1)
mergeRefineConfig(32-59)
libs/portal-plugin-dashboard/src/capabilities/refineConfig.ts (2)
libs/portal-plugin-dashboard/src-lib/util/dataProvider.ts (1)
setupDataProvider(49-61)libs/portal-plugin-dashboard/src-lib/util/api.ts (1)
resolveDashboardApiUrl(6-27)
libs/portal-plugin-billing/src/capabilities/refineConfig.ts (2)
libs/portal-plugin-dashboard/src/capabilities/refineConfig.ts (1)
Capability(9-60)libs/portal-framework-core/src/util/refineConfig.ts (1)
mergeRefineConfig(32-59)
🔇 Additional comments (22)
.git-town.toml (1)
1-4: Git Town configuration aligns with the repository's development branch strategy.The configuration correctly maps
maintodevelop, which is consistent with the PR objective where feature work merges intodevelop. The file is properly formatted and references the official Git Town docs.Consider verifying with your team that this represents the complete Git Town configuration needed (e.g., perennialBranches, hostingService settings) and that it aligns with your established git workflow strategy. You may also want to document this in your team's contributing guide if not already present.
shared-modules.ts (1)
7-7: Addition looks good; confirm singleton/eager intent and side effects.Sharing @lumeweb/advanced-rest-provider as singleton/eager is fine if it initializes globals or must be unique. Please confirm the package relies on import-time side effects; otherwise consider removing eager to avoid upfront cost.
apps/portal-app-shell/src/App.tsx (1)
18-20: Potential copy/paste: routing flag sets loadNavigation again.VITE_PORTAL_APP_DISABLE_ROUTING toggles loadNavigation, same as the nav flag. If routing is meant to be disabled, confirm the correct AppComponent option (e.g., a routing toggle) is set.
apps/portal-app-shell/package.json (1)
20-20: LGTM — dependency added coherently with imports and MF share.libs/portal-plugin-billing/postcss.config.cjs (1)
1-1: The current code is already robust and does not need the suggested change.The package
@lumeweb/portal-framework-ui-core(a workspace package) explicitly defines the export"./postcss.config": "./dist/cjs/config/postcss.config.cjs"in its package.json. When the extensionless require path is used, Node.js correctly resolves it via this explicit export mapping. Adding.cjsas suggested would actually break or degrade the resolution, since"./postcss.config.cjs"is not in the exports map—Node.js would no longer use the explicit export and would fall back to filesystem search, which is less reliable. The current implementation already follows best practices for package consumption.libs/portal-plugin-billing/tsconfig.json (1)
3-8: Paths alias is correctly configured and will be resolved by Vite.The framework's
Config()function (imported from@lumeweb/portal-framework-core/vite) includes thevite-tsconfig-pathsplugin, which automatically resolves the"@/*": ["./src/*"]alias defined in the plugin's tsconfig.json. No additional Vite configuration is needed.libs/portal-plugin-billing/src/types/subscription.ts (1)
1-16: LGTM!The type definitions are well-structured and provide clear contracts for subscription status, customer portal responses, and Stripe configuration metadata.
libs/portal-plugin-billing/src/types/index.ts (1)
1-1: LGTM!Standard barrel export pattern for the types module.
libs/portal-plugin-billing/src/ui/routes/index.ts (1)
1-1: LGTM!Standard barrel export pattern for route components.
libs/portal-plugin-dashboard/src-lib/index.ts (1)
3-4: LGTM!The additional re-exports appropriately expand the public API surface to include the new API resolution and data provider utilities.
libs/portal-plugin-billing/src/ui/index.ts (1)
1-2: LGTM!Standard barrel export pattern establishing the public UI surface for the billing plugin.
libs/portal-plugin-billing/src/capabilities/index.ts (1)
1-1: LGTM!Standard barrel export pattern for the capabilities module.
libs/portal-plugin-billing/src/ui/components/index.ts (1)
1-1: LGTM!Standard barrel export pattern for exposing the StripePricingTable component through the components module.
libs/portal-plugin-dashboard/src-lib/util/index.ts (1)
1-2: LGTM!The addition of API and data provider utility exports follows the established barrel export pattern and aligns with the refactoring goal to make these utilities reusable.
libs/portal-plugin-billing/src/index.ts (1)
1-3: LGTM!Public API exports follow the standard barrel export pattern for the billing plugin.
libs/portal-plugin-billing/src/hooks/index.ts (1)
1-2: LGTM!Standard barrel export pattern for exposing the billing hooks through the package public API.
libs/portal-plugin-billing/vite.config.ts (1)
1-13: LGTM!The Vite configuration follows the portal framework plugin pattern correctly, properly delegating to
plugin.config.tsfor core settings and integrating shared modules.libs/portal-plugin-billing/plugin.config.ts (1)
1-15: LGTM!The plugin configuration correctly sets up Module Federation exposes for the billing plugin. The
__dirnamecomputation is the standard pattern for ESM modules, and the configuration properly exposes both the root entry point and the account subscriptions route.libs/portal-plugin-billing/src/routes.tsx (1)
1-16: LGTM!The route definition is properly structured with appropriate navigation metadata. The path, component, and navigation properties are correctly configured for the subscription management feature.
libs/portal-plugin-billing/src/ui/components/StripePricingTable.tsx (2)
10-19: LGTM!The JSX intrinsic element declaration for
stripe-pricing-tableis correctly typed, allowing TypeScript to recognize the Stripe custom element.
37-44: LGTM!The conditional spread syntax for optional attributes is clean and ensures that
customer-emailandclient-reference-idare only added when the corresponding props are provided.libs/portal-plugin-dashboard/src/capabilities/refineConfig.ts (1)
57-59: Verify port preservation in resolveDashboardApiUrlresolveDashboardApiUrl(framework) builds the URL using URL(apiUrl).hostname, which drops the port during construction; use .host to preserve dev ports. Please confirm in libs/portal-plugin-dashboard/src-lib/util/api.ts.
| import { AppComponent, AppComponentProps } from "@lumeweb/portal-framework-ui"; | ||
| import "@fontsource-variable/manrope"; | ||
| import "@lumeweb/portal-framework-ui-core/tailwind.css"; | ||
| import "@lumeweb/advanced-rest-provider"; |
There was a problem hiding this comment.
🧩 Analysis chain
Side‑effect import — ensure it won’t be treeshaken.
If @lumeweb/advanced-rest-provider must run at import time, verify its package.json has sideEffects true (or lists the entry) so bundlers keep it. Otherwise, expose an explicit init and call it here.
🏁 Script executed:
#!/bin/bash
# Find the workspace package.json and print its sideEffects field
PKG=$(rg -n '"name":\s*"@lumeweb/advanced-rest-provider"' -g '**/package.json' | cut -d: -f1 | head -n1)
echo "Package file: $PKG"
jq '.sideEffects // "UNSET"' "$PKG"Length of output: 53
Side‑effect import will be tree‑shaken — add sideEffects declaration or use explicit init.
Verified: @lumeweb/advanced-rest-provider (libs/advanced-rest/package.json) has no sideEffects field. Bundlers will remove this import during build.
Fix one of:
- Add
"sideEffects": true(or list the entry point) to libs/advanced-rest/package.json - Replace the side-effect import with an explicit
initAdvancedRestProvider()call (or equivalent) in this file and export that function for tree-shaking safety
🤖 Prompt for AI Agents
In apps/portal-app-shell/src/App.tsx around line 4, the side-effect import
import "@lumeweb/advanced-rest-provider"; can be tree‑shaken because
libs/advanced-rest/package.json has no sideEffects field; either add
"sideEffects": true (or list the specific entry file path) to
libs/advanced-rest/package.json to prevent removal, or replace the side‑effect
import with an explicit exported initializer from the package (e.g.
export/initAdvancedRestProvider) and call that initializer here so the import is
live and tree‑shake safe; pick one approach and apply it consistently (update
package.json for the package if choosing option 1, or change this file and the
package to expose/init the provider function if choosing option 2).
| status; | ||
| readonly type = "core:refine-config"; | ||
| version: string; | ||
| #apiUrl: string; |
There was a problem hiding this comment.
🧩 Analysis chain
Unused private field #apiUrl.
The #apiUrl field is set during initialization (line 33) but never read or used elsewhere in the class. This suggests either incomplete implementation or unnecessary code.
Looking at the dashboard capability (see relevant code snippets), it uses setupDataProvider(this.#apiUrl, existing, dashboardResources) in its getConfig method. Should the billing capability be doing something similar?
Run this script to check if billing needs its own data provider setup:
🏁 Script executed:
#!/bin/bash
# Search for billing-specific API endpoints or data provider usage
rg -nP --type=ts -C3 'billing.*api|data.*provider.*billing' -g '!node_modules'Length of output: 0
I need to investigate the codebase more thoroughly since the initial pattern search yielded no results. Let me examine the file structure and look for actual usage patterns.
---
🏁 Script executed:
```shell
#!/bin/bash
# View the refineConfig.ts file to understand the class structure and #apiUrl usage
fd -t f "refineConfig.ts" && cat "$(fd -t f 'refineConfig.ts' | head -1)"
Length of output: 1526
🏁 Script executed:
#!/bin/bash
# Search for setupDataProvider usage across the codebase
rg -nP --type=ts 'setupDataProvider' -C5 -g '!node_modules'Length of output: 3208
🏁 Script executed:
#!/bin/bash
# Search for any usage or reference to #apiUrl in the codebase
rg -nP --type=ts '#apiUrl' -C3 -g '!node_modules'Length of output: 11437
🏁 Script executed:
#!/bin/bash
# Look for dashboard capability file to understand the pattern
fd -t f "dashboard.*capability\|.*dashboard.*config" --type f | grep -E '\.(ts|js)$' | head -10Length of output: 0
I need to see the complete billing capability file to verify if #apiUrl is actually used in the getConfig() method.
---
🏁 Script executed:
```shell
#!/bin/bash
# View the complete billing refineConfig.ts file
cat libs/portal-plugin-billing/src/capabilities/refineConfig.ts
Length of output: 1067
Remove unused #apiUrl field or implement data provider setup.
The #apiUrl field is set during initialization (lines 32-37) but never used. The getConfig() method returns mergeRefineConfig(existing, {}) without using it. Other plugins (dashboard, IPFS, core) follow a pattern where #apiUrl is used inside getConfig() to set up data providers—billing should either implement similar functionality or remove the unused field entirely.
🤖 Prompt for AI Agents
In libs/portal-plugin-billing/src/capabilities/refineConfig.ts around line 13,
the private field `#apiUrl` is declared and set during initialization but never
used; either remove the unused `#apiUrl` declaration and its initialization
code, or implement the same data-provider merge pattern used in other plugins:
update getConfig() to call mergeRefineConfig(existing, { dataProvider:
createDataProvider(this.#apiUrl) }) (or the billing-specific provider), ensure
the provider factory is imported/available, and remove dead imports if you
choose to delete the field.
| getConfig(existing?: Partial<RefineProps>) { | ||
| return mergeRefineConfig(existing, {}); | ||
| } |
There was a problem hiding this comment.
Empty configuration passed to mergeRefineConfig.
The getConfig method passes an empty object {} to mergeRefineConfig, which provides no custom data providers or resources. This seems unusual for a capability that retrieves and stores an API URL.
Consider whether billing-specific resources or data providers should be configured here, similar to how the dashboard capability uses setupDataProvider.
🤖 Prompt for AI Agents
In libs/portal-plugin-billing/src/capabilities/refineConfig.ts around lines 18
to 20, getConfig currently calls mergeRefineConfig(existing, {}) which passes an
empty config; replace that second argument with a billing-aware config that
wires up the billing data provider(s)/resources and persists the API URL
(similar to dashboard’s setupDataProvider usage). Concretely, construct and pass
a config object that invokes setupDataProvider or equivalent to register the
billing API URL and any billing resources (resources, dataProvider, authProvider
as needed) before calling mergeRefineConfig(existing, billingConfig).
| React.useEffect(() => { | ||
| const script = document.createElement("script"); | ||
| script.src = "https://js.stripe.com/v3/pricing-table.js"; | ||
| script.async = true; | ||
| document.body.appendChild(script); | ||
| return () => { | ||
| document.body.removeChild(script); | ||
| }; | ||
| }, []); |
There was a problem hiding this comment.
Fix script cleanup to prevent breaking other component instances.
Removing the script element on unmount will break other mounted instances of StripePricingTable. If multiple instances exist or the component remounts quickly, the script removal will cause failures.
Apply this diff to fix the cleanup logic:
export const StripePricingTable: React.FC<StripePricingTableProps> = ({
pricingTableId,
publishableKey,
customerEmail,
clientReferenceId
}) => {
React.useEffect(() => {
+ // Check if script already exists
+ const existingScript = document.querySelector(
+ 'script[src="https://js.stripe.com/v3/pricing-table.js"]'
+ );
+
+ if (existingScript) {
+ return; // Script already loaded, no cleanup needed
+ }
+
const script = document.createElement("script");
script.src = "https://js.stripe.com/v3/pricing-table.js";
script.async = true;
document.body.appendChild(script);
- return () => {
- document.body.removeChild(script);
- };
+
+ // Don't remove the script on unmount since it may be used by other instances
+ // The script is safe to leave in the DOM
}, []);
return (
<stripe-pricing-table
pricing-table-id={pricingTableId}
publishable-key={publishableKey}
{...(customerEmail && { "customer-email": customerEmail })}
{...(clientReferenceId && { "client-reference-id": clientReferenceId })}
/>
);
};Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In libs/portal-plugin-billing/src/ui/components/StripePricingTable.tsx around
lines 27 to 35, the effect always removes the injected Stripe script on unmount
which breaks other mounted instances; change the logic so the component only
removes the script if it was the one that added it (or no other instances
remain). Implement this by checking for an existing script before creating one
(query document for the Stripe script URL), set a marker (e.g.,
script.dataset.addedBy = "StripePricingTable") or maintain a module-level
ref/count of active instances and increment on mount/decrement on unmount, and
only call document.body.removeChild(script) when the count reaches zero or the
dataset marker shows this instance added it; ensure you still append the script
when missing and preserve async attribute.
Add error handling for script loading failures.
The component doesn't handle cases where the Stripe script fails to load (network errors, CSP violations, etc.), which could result in a broken UI with no feedback to the user.
Consider adding error handling:
export const StripePricingTable: React.FC<StripePricingTableProps> = ({
pricingTableId,
publishableKey,
customerEmail,
clientReferenceId
}) => {
+ const [error, setError] = React.useState<string | null>(null);
+
React.useEffect(() => {
const existingScript = document.querySelector(
'script[src="https://js.stripe.com/v3/pricing-table.js"]'
);
if (existingScript) {
return;
}
const script = document.createElement("script");
script.src = "https://js.stripe.com/v3/pricing-table.js";
script.async = true;
+
+ script.onerror = () => {
+ setError("Failed to load Stripe pricing table. Please refresh the page.");
+ };
+
document.body.appendChild(script);
}, []);
+ if (error) {
+ return <div className="text-red-600">{error}</div>;
+ }
+
return (
<stripe-pricing-table
pricing-table-id={pricingTableId}
publishable-key={publishableKey}
{...(customerEmail && { "customer-email": customerEmail })}
{...(clientReferenceId && { "client-reference-id": clientReferenceId })}
/>
);
};Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In libs/portal-plugin-billing/src/ui/components/StripePricingTable.tsx around
lines 27 to 35, the effect that injects the Stripe pricing-table script lacks
error handling so failures (network/CSP) leave the UI broken; modify the effect
to add script.onerror and script.onload handlers: set component state (e.g.,
loading, error) on those callbacks, show a user-friendly fallback or error
message when onerror fires, ensure you also handle timeout cases if needed, and
remove the event handlers during cleanup before removing the script to avoid
leaks.
| throw new Error("Failed to get API base URL"); | ||
| } | ||
|
|
||
| const subdomain = getPluginMeta(framework.meta!, "dashboard", "subdomain"); |
There was a problem hiding this comment.
Unsafe non-null assertion on framework.meta.
The non-null assertion operator bypasses TypeScript's type safety. If framework.meta is undefined, this will throw at runtime.
Apply this diff to add defensive validation:
- const subdomain = getPluginMeta(framework.meta!, "dashboard", "subdomain");
+ if (!framework.meta) {
+ throw new Error("Framework metadata is not available");
+ }
+ const subdomain = getPluginMeta(framework.meta, "dashboard", "subdomain");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const subdomain = getPluginMeta(framework.meta!, "dashboard", "subdomain"); | |
| if (!framework.meta) { | |
| throw new Error("Framework metadata is not available"); | |
| } | |
| const subdomain = getPluginMeta(framework.meta, "dashboard", "subdomain"); |
🤖 Prompt for AI Agents
In libs/portal-plugin-dashboard/src-lib/util/api.ts around line 16, replace the
unsafe non-null assertion on framework.meta by first validating that
framework.meta is defined (e.g., if (!framework?.meta) throw a clear Error or
return a safe default), then call getPluginMeta using the validated object;
alternatively use optional chaining and handle an undefined result from
getPluginMeta so runtime exceptions are avoided and a clear error or fallback
behavior is provided.
| } catch (error) { | ||
| throw new Error(`Failed to construct API URL: ${error.message}`); |
There was a problem hiding this comment.
Error message access may fail if error is not an Error object.
In TypeScript catch blocks, the error type is unknown. Accessing .message directly can throw if the caught value is not an Error instance.
Apply this diff to safely access the error message:
} catch (error) {
- throw new Error(`Failed to construct API URL: ${error.message}`);
+ throw new Error(
+ `Failed to construct API URL: ${error instanceof Error ? error.message : String(error)}`
+ );
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (error) { | |
| throw new Error(`Failed to construct API URL: ${error.message}`); | |
| } catch (error) { | |
| throw new Error( | |
| `Failed to construct API URL: ${error instanceof Error ? error.message : String(error)}` | |
| ); | |
| } |
🤖 Prompt for AI Agents
In libs/portal-plugin-dashboard/src-lib/util/api.ts around lines 24-25, the
catch block assumes error.message exists which can fail for non-Error throws;
update the catch to safely extract the message by checking if error is an
instance of Error and using error.message, otherwise fall back to String(error)
(or JSON.stringify if preferred), then include that safe message in the thrown
Error text so the code never tries to directly access a possibly-undefined
.message property.
| const token = localStorage.getItem("jwt"); | ||
| const acctProvider = dataProvider(apiUrl, true); | ||
|
|
||
| if (token) { | ||
| acctProvider.setAuthToken(token); | ||
| } |
There was a problem hiding this comment.
Security posture: JWT in localStorage
Reading tokens from localStorage encourages storage there, which is XSS‑prone. Prefer HTTP‑only cookies or an in‑memory token with refresh. If constrained, ensure strict CSP and zero DOM XSS sinks.
| const authProvider = existing?.authProvider as | ||
| | AuthProviderWithEmitter | ||
| | undefined; | ||
| if (authProvider) { | ||
| authProvider.on("authCheckSuccess", (params) => { | ||
| acctProvider.setAuthToken(params.token); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Add token cleanup and avoid event listener leaks
- Subscribe adds a long‑lived listener; no corresponding unsubscribe.
- Also clear token on logout to prevent stale auth.
const authProvider = existing?.authProvider as
| AuthProviderWithEmitter
| undefined;
if (authProvider) {
- authProvider.on("authCheckSuccess", (params) => {
- acctProvider.setAuthToken(params.token);
- });
+ const onAuth = (params: { token?: string }) => {
+ if (params?.token) acctProvider.setAuthToken(params.token);
+ };
+ const onLogout = () => {
+ acctProvider.setAuthToken(undefined as any);
+ };
+ authProvider.on("authCheckSuccess", onAuth);
+ authProvider.on?.("logoutSuccess", onLogout);
+ // TODO: Unsubscribe on teardown if .off is available; wire cleanup via caller.
+ // authProvider.off?.("authCheckSuccess", onAuth);
+ // authProvider.off?.("logoutSuccess", onLogout);
}If AuthProviderWithEmitter exposes an off/unsubscribe API, we should call it during capability.destroy(). Please confirm with a quick grep.
🏁 Script executed:
#!/bin/bash
rg -nP -C2 'interface\s+AuthProviderWithEmitter|class\s+.*Emitter|\.on\(|\.off\(' -g '!**/dist/**'Length of output: 285453
🏁 Script executed:
#!/bin/bash
# Search for AuthProviderWithEmitter interface definition
rg -nP 'interface\s+AuthProviderWithEmitter|type\s+AuthProviderWithEmitter' --type=ts --type=tsxLength of output: 27
🏁 Script executed:
#!/bin/bash
# Find AuthProviderWithEmitter definition and check for .off() method
rg -nP 'interface\s+AuthProviderWithEmitter|type\s+AuthProviderWithEmitter|export\s+(?:interface|type)\s+AuthProviderWithEmitter'
# Also search for where it's used/imported in dataProvider
echo "---"
echo "Searching for AuthProviderWithEmitter in dataProvider:"
rg -nP 'AuthProviderWithEmitter' libs/portal-plugin-dashboard/src-lib/util/dataProvider.tsLength of output: 550
🏁 Script executed:
#!/bin/bash
# View the AuthProviderWithEmitter interface definition
rg -A 15 'export interface AuthProviderWithEmitter' libs/portal-framework-auth/src/dataProviders/auth.tsLength of output: 560
Refactor event subscription to prevent listener leak using returned cleanup function
The AuthProviderWithEmitter.on() method returns a cleanup function (not .off()). The current code doesn't capture or call it, causing a listener leak:
const authProvider = existing?.authProvider as
| AuthProviderWithEmitter
| undefined;
if (authProvider) {
- authProvider.on("authCheckSuccess", (params) => {
- acctProvider.setAuthToken(params.token);
+ const unsubscribe = authProvider.on("authCheckSuccess", (params) => {
+ if (params?.token) {
+ acctProvider.setAuthToken(params.token);
+ }
});
+ // TODO: Call unsubscribe() during teardown (e.g., in capability.destroy())
}The .off() method does not exist in the interface. Instead, capture and invoke the cleanup function returned by .on() to prevent memory leaks.
🤖 Prompt for AI Agents
In libs/portal-plugin-dashboard/src-lib/util/dataProvider.ts around lines 31 to
38, the code subscribes to authProvider events but ignores the cleanup function
returned by authProvider.on(), causing a listener leak; capture the returned
cleanup (e.g. const dispose = authProvider.on(...)) and ensure you call
dispose() when the provider changes or when the surrounding object is torn down
(store the dispose reference on the closure/object and call it before
reassigning authProvider or in the provider's cleanup/dispose method) so the
listener is removed properly.
@lumeweb/portal-plugin-billingpackage for handling subscription billing@lumeweb/advanced-rest-providerSummary by CodeRabbit
Release Notes