Skip to content

Commit 095a2af

Browse files
CassioMGclaude
andcommitted
fix(icons): compare every render-affecting prop in AssetIcon's memo
AssetIcon's custom memo comparator only compared the icon map and the two security flags, so a surviving instance whose asset changed while the icon map stayed deeply equal kept the previous asset's logo or loading state on screen. That gap predates this branch — the comparator already ignored the code the native check used to depend on — and now that the check also reads the issuer, both halves of the asset's identity have to be compared. Compare code, issuer, icon and the shape flags alongside the icon map; the retry callback stays excluded because its identity is unstable and it does not affect the render. Two rerender tests pin an identity change over an equal icon map in both directions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 5e8526b commit 095a2af

2 files changed

Lines changed: 65 additions & 0 deletions

File tree

extension/src/popup/components/account/AccountAssets/__tests__/AssetIcon.test.tsx

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,4 +45,57 @@ describe("AssetIcon", () => {
4545
FETCHED_ICON,
4646
);
4747
});
48+
49+
// The component is memoised with a custom comparator. When only the asset's
50+
// identity changes on a surviving instance — the icon map staying deeply
51+
// equal — the comparator must still let the render through, or the previous
52+
// asset's logo stays on screen.
53+
describe("re-rendering a surviving instance with a new asset identity", () => {
54+
const wrap = (props: {
55+
code: string;
56+
issuerKey?: string;
57+
assetIcons: Record<string, string>;
58+
}) => (
59+
<Wrapper state={{}} routes={["/"]}>
60+
<AssetIcon {...props} />
61+
</Wrapper>
62+
);
63+
64+
it("re-renders when the native asset becomes a classic asset using the native code", () => {
65+
const { rerender } = renderIcon({ code: "XLM", assetIcons: {} });
66+
expect(screen.getByAltText("XLM logo")).toHaveAttribute(
67+
"src",
68+
BUNDLED_LOGO,
69+
);
70+
71+
rerender(
72+
wrap({ code: "XLM", issuerKey: XLM_CODED_ISSUER, assetIcons: {} }),
73+
);
74+
75+
// Same (empty) icon map, different identity: no bundled logo any more —
76+
// the icon now has to be looked up, so the loading state shows.
77+
expect(screen.queryByAltText("XLM logo")).not.toBeInTheDocument();
78+
expect(
79+
screen.getByTestId("AccountAssets__asset--loading"),
80+
).toBeInTheDocument();
81+
});
82+
83+
it("re-renders when a classic asset using the native code becomes the native asset", () => {
84+
const { rerender } = renderIcon({
85+
code: "XLM",
86+
issuerKey: XLM_CODED_ISSUER,
87+
assetIcons: {},
88+
});
89+
expect(
90+
screen.getByTestId("AccountAssets__asset--loading"),
91+
).toBeInTheDocument();
92+
93+
rerender(wrap({ code: "XLM", assetIcons: {} }));
94+
95+
expect(screen.getByAltText("XLM logo")).toHaveAttribute(
96+
"src",
97+
BUNDLED_LOGO,
98+
);
99+
});
100+
});
48101
});

extension/src/popup/components/account/AccountAssets/index.tsx

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,11 +63,23 @@ interface AssetIconProps {
6363
isModal?: boolean;
6464
}
6565

66+
// Skip a re-render only when nothing that affects the rendered icon changed.
67+
// `assetIcons` is compared deeply because callers build it per render; the
68+
// retry callback is left out because its identity is unstable and it does not
69+
// affect what is rendered. Every other prop feeds the render — the asset's
70+
// identity (code + issuer) decides between the bundled native logo, a Soroban
71+
// placeholder and an icon lookup — so each has to be compared.
6672
const shouldAssetIconSkipUpdate = (
6773
prevProps: AssetIconProps,
6874
nextProps: AssetIconProps,
6975
) =>
7076
isEqual(prevProps.assetIcons, nextProps.assetIcons) &&
77+
prevProps.code === nextProps.code &&
78+
prevProps.issuerKey === nextProps.issuerKey &&
79+
prevProps.icon === nextProps.icon &&
80+
prevProps.isLPShare === nextProps.isLPShare &&
81+
prevProps.isSorobanToken === nextProps.isSorobanToken &&
82+
prevProps.isModal === nextProps.isModal &&
7183
prevProps.isSuspicious === nextProps.isSuspicious &&
7284
prevProps.isMalicious === nextProps.isMalicious;
7385

0 commit comments

Comments
 (0)