diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts
index 601c4e9b65..0a1597a32e 100644
--- a/src/core/webview/webviewMessageHandler.ts
+++ b/src/core/webview/webviewMessageHandler.ts
@@ -3000,20 +3000,26 @@ export const webviewMessageHandler = async (
}
case "dismissUpsell": {
if (message.upsellId) {
- // Get current list of dismissed upsells
- const dismissedUpsells = getGlobalState("dismissedUpsells") || []
+ try {
+ // Get current list of dismissed upsells
+ const dismissedUpsells = getGlobalState("dismissedUpsells") || []
- // Add the new upsell ID if not already present
- if (!dismissedUpsells.includes(message.upsellId)) {
- const updatedList = [...dismissedUpsells, message.upsellId]
- await updateGlobalState("dismissedUpsells", updatedList)
+ // Add the new upsell ID if not already present
+ let updatedList = dismissedUpsells
+ if (!dismissedUpsells.includes(message.upsellId)) {
+ updatedList = [...dismissedUpsells, message.upsellId]
+ await updateGlobalState("dismissedUpsells", updatedList)
+ }
+
+ // Send updated list back to webview (use the already computed updatedList)
+ await provider.postMessageToWebview({
+ type: "dismissedUpsells",
+ list: updatedList,
+ })
+ } catch (error) {
+ // Fail silently as per Bruno's comment - it's OK to fail silently in this case
+ provider.log(`Failed to dismiss upsell: ${error instanceof Error ? error.message : String(error)}`)
}
-
- // Send updated list back to webview
- await provider.postMessageToWebview({
- type: "dismissedUpsells",
- list: [...dismissedUpsells, message.upsellId],
- })
}
break
}
diff --git a/webview-ui/src/components/common/DismissibleUpsell.tsx b/webview-ui/src/components/common/DismissibleUpsell.tsx
index 0721629c22..b1c0f82001 100644
--- a/webview-ui/src/components/common/DismissibleUpsell.tsx
+++ b/webview-ui/src/components/common/DismissibleUpsell.tsx
@@ -1,11 +1,14 @@
-import { memo, ReactNode, useEffect, useState } from "react"
+import { memo, ReactNode, useEffect, useState, useRef } from "react"
import styled from "styled-components"
import { vscode } from "@src/utils/vscode"
+import { useAppTranslation } from "@src/i18n/TranslationContext"
interface DismissibleUpsellProps {
/** Required unique identifier for this upsell */
- className: string
+ id: string
+ /** Optional CSS class name for styling */
+ className?: string
/** Content to display inside the upsell */
children: ReactNode
/** Visual variant of the upsell */
@@ -84,38 +87,51 @@ const DismissIcon = () => (
)
-const DismissibleUpsell = memo(({ className, children, variant = "banner", onDismiss }: DismissibleUpsellProps) => {
+const DismissibleUpsell = memo(({ id, className, children, variant = "banner", onDismiss }: DismissibleUpsellProps) => {
+ const { t } = useAppTranslation()
const [isVisible, setIsVisible] = useState(true)
+ const isMountedRef = useRef(true)
useEffect(() => {
+ // Track mounted state
+ isMountedRef.current = true
+
// Request the current list of dismissed upsells from the extension
vscode.postMessage({ type: "getDismissedUpsells" })
// Listen for the response
const handleMessage = (event: MessageEvent) => {
+ // Only update state if component is still mounted
+ if (!isMountedRef.current) return
+
const message = event.data
- if (message.type === "dismissedUpsells" && Array.isArray(message.list)) {
+ // Add null/undefined check for message
+ if (message && message.type === "dismissedUpsells" && Array.isArray(message.list)) {
// Check if this upsell has been dismissed
- if (message.list.includes(className)) {
+ if (message.list.includes(id)) {
setIsVisible(false)
}
}
}
window.addEventListener("message", handleMessage)
- return () => window.removeEventListener("message", handleMessage)
- }, [className])
+ return () => {
+ isMountedRef.current = false
+ window.removeEventListener("message", handleMessage)
+ }
+ }, [id])
- const handleDismiss = () => {
- // Hide the upsell immediately
- setIsVisible(false)
-
- // Notify the extension to persist the dismissal
+ const handleDismiss = async () => {
+ // First notify the extension to persist the dismissal
+ // This ensures the message is sent even if the component unmounts quickly
vscode.postMessage({
type: "dismissUpsell",
- upsellId: className,
+ upsellId: id,
})
+ // Then hide the upsell
+ setIsVisible(false)
+
// Call the optional callback
onDismiss?.()
}
@@ -131,8 +147,8 @@ const DismissibleUpsell = memo(({ className, children, variant = "banner", onDis
+ aria-label={t("common:dismiss")}
+ title={t("common:dismissAndDontShowAgain")}>
diff --git a/webview-ui/src/components/common/__tests__/DismissibleUpsell.spec.tsx b/webview-ui/src/components/common/__tests__/DismissibleUpsell.spec.tsx
index 3d81984bb9..78b6c71e90 100644
--- a/webview-ui/src/components/common/__tests__/DismissibleUpsell.spec.tsx
+++ b/webview-ui/src/components/common/__tests__/DismissibleUpsell.spec.tsx
@@ -1,5 +1,5 @@
-import { render, screen, fireEvent, waitFor } from "@testing-library/react"
-import { describe, it, expect, vi, beforeEach } from "vitest"
+import { render, screen, fireEvent, waitFor, act } from "@testing-library/react"
+import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"
import DismissibleUpsell from "../DismissibleUpsell"
// Mock the vscode API
@@ -10,14 +10,32 @@ vi.mock("@src/utils/vscode", () => ({
},
}))
+// Mock the translation hook
+vi.mock("@src/i18n/TranslationContext", () => ({
+ useAppTranslation: () => ({
+ t: (key: string) => {
+ const translations: Record = {
+ "common:dismiss": "Dismiss",
+ "common:dismissAndDontShowAgain": "Dismiss and don't show again",
+ }
+ return translations[key] || key
+ },
+ }),
+}))
+
describe("DismissibleUpsell", () => {
beforeEach(() => {
mockPostMessage.mockClear()
+ vi.clearAllTimers()
+ })
+
+ afterEach(() => {
+ vi.clearAllTimers()
})
it("renders children content", () => {
render(
-
+
Test content
,
)
@@ -27,7 +45,7 @@ describe("DismissibleUpsell", () => {
it("applies the correct variant styles", () => {
const { container, rerender } = render(
-
+
Banner content
,
)
@@ -41,7 +59,7 @@ describe("DismissibleUpsell", () => {
// Re-render with default variant
rerender(
-
+
Default content
,
)
@@ -55,7 +73,7 @@ describe("DismissibleUpsell", () => {
it("requests dismissed upsells list on mount", () => {
render(
-
+
Test content
,
)
@@ -68,7 +86,7 @@ describe("DismissibleUpsell", () => {
it("hides the upsell when dismiss button is clicked", async () => {
const onDismiss = vi.fn()
const { container } = render(
-
+
Test content
,
)
@@ -77,24 +95,24 @@ describe("DismissibleUpsell", () => {
const dismissButton = screen.getByRole("button", { name: /dismiss/i })
fireEvent.click(dismissButton)
- // Check that the component is no longer visible
- await waitFor(() => {
- expect(container.firstChild).toBeNull()
- })
-
- // Check that the dismiss message was sent
+ // Check that the dismiss message was sent BEFORE hiding
expect(mockPostMessage).toHaveBeenCalledWith({
type: "dismissUpsell",
upsellId: "test-upsell",
})
+ // Check that the component is no longer visible
+ await waitFor(() => {
+ expect(container.firstChild).toBeNull()
+ })
+
// Check that the callback was called
expect(onDismiss).toHaveBeenCalled()
})
it("hides the upsell if it's in the dismissed list", async () => {
const { container } = render(
-
+
Test content
,
)
@@ -116,7 +134,7 @@ describe("DismissibleUpsell", () => {
it("remains visible if not in the dismissed list", async () => {
render(
-
+
Test content
,
)
@@ -138,7 +156,7 @@ describe("DismissibleUpsell", () => {
it("applies the className prop to the container", () => {
const { container } = render(
-
+
Test content
,
)
@@ -148,7 +166,7 @@ describe("DismissibleUpsell", () => {
it("dismiss button has proper accessibility attributes", () => {
render(
-
+
Test content
,
)
@@ -157,4 +175,131 @@ describe("DismissibleUpsell", () => {
expect(dismissButton).toHaveAttribute("aria-label", "Dismiss")
expect(dismissButton).toHaveAttribute("title", "Dismiss and don't show again")
})
+
+ // New edge case tests
+ it("handles multiple rapid dismissals of the same component", async () => {
+ const onDismiss = vi.fn()
+ render(
+
+ Test content
+ ,
+ )
+
+ const dismissButton = screen.getByRole("button", { name: /dismiss/i })
+
+ // Click multiple times rapidly
+ fireEvent.click(dismissButton)
+ fireEvent.click(dismissButton)
+ fireEvent.click(dismissButton)
+
+ // Should only send one message
+ expect(mockPostMessage).toHaveBeenCalledTimes(2) // 1 for getDismissedUpsells, 1 for dismissUpsell
+ expect(mockPostMessage).toHaveBeenCalledWith({
+ type: "dismissUpsell",
+ upsellId: "test-upsell",
+ })
+
+ // Callback should only be called once
+ expect(onDismiss).toHaveBeenCalledTimes(1)
+ })
+
+ it("does not update state after component unmounts", async () => {
+ const { unmount } = render(
+
+ Test content
+ ,
+ )
+
+ // Unmount the component
+ unmount()
+
+ // Simulate receiving a message after unmount
+ const messageEvent = new MessageEvent("message", {
+ data: {
+ type: "dismissedUpsells",
+ list: ["test-upsell"],
+ },
+ })
+
+ // This should not cause any errors
+ act(() => {
+ window.dispatchEvent(messageEvent)
+ })
+
+ // No errors should be thrown
+ expect(true).toBe(true)
+ })
+
+ it("handles invalid/malformed messages gracefully", () => {
+ render(
+
+ Test content
+ ,
+ )
+
+ // Send various malformed messages
+ const malformedMessages = [
+ { type: "dismissedUpsells", list: null },
+ { type: "dismissedUpsells", list: "not-an-array" },
+ { type: "dismissedUpsells" }, // missing list
+ { type: "wrongType", list: ["test-upsell"] },
+ null,
+ undefined,
+ "string-message",
+ ]
+
+ malformedMessages.forEach((data) => {
+ const messageEvent = new MessageEvent("message", { data })
+ window.dispatchEvent(messageEvent)
+ })
+
+ // Component should still be visible
+ expect(screen.getByText("Test content")).toBeInTheDocument()
+ })
+
+ it("ensures message is sent before component unmounts on dismiss", async () => {
+ const { unmount } = render(
+
+ Test content
+ ,
+ )
+
+ const dismissButton = screen.getByRole("button", { name: /dismiss/i })
+ fireEvent.click(dismissButton)
+
+ // Message should be sent immediately
+ expect(mockPostMessage).toHaveBeenCalledWith({
+ type: "dismissUpsell",
+ upsellId: "test-upsell",
+ })
+
+ // Unmount immediately after clicking
+ unmount()
+
+ // Message was already sent before unmount
+ expect(mockPostMessage).toHaveBeenCalledWith({
+ type: "dismissUpsell",
+ upsellId: "test-upsell",
+ })
+ })
+
+ it("uses separate id and className props correctly", () => {
+ const { container } = render(
+
+ Test content
+ ,
+ )
+
+ // className should be applied to the container
+ expect(container.firstChild).toHaveClass("styling-class")
+
+ // When dismissed, should use the id, not className
+ const dismissButton = screen.getByRole("button", { name: /dismiss/i })
+ fireEvent.click(dismissButton)
+
+ expect(mockPostMessage).toHaveBeenCalledWith({
+ type: "dismissUpsell",
+ upsellId: "unique-id",
+ })
+ })
})