diff --git a/.changeset/eip1193-remove-listener-noop.md b/.changeset/eip1193-remove-listener-noop.md new file mode 100644 index 00000000000..1e2cfde5ad0 --- /dev/null +++ b/.changeset/eip1193-remove-listener-noop.md @@ -0,0 +1,5 @@ +--- +"thirdweb": patch +--- + +Fix: `EIP1193.toProvider()`'s `removeListener` is no longer a no-op. Previously, `removeListener` discarded the unsubscribe function returned by `wallet.subscribe()`, so listeners registered via `provider.on(...)` (e.g. `accountsChanged`, `chainChanged`, `disconnect`) could never actually be detached — they kept firing after callers (such as wagmi connectors) believed they had unsubscribed. `removeListener` now tracks and invokes the correct unsubscribe function per `(event, listener)` pair. diff --git a/packages/thirdweb/src/adapters/eip1193/to-eip1193.test.ts b/packages/thirdweb/src/adapters/eip1193/to-eip1193.test.ts index 87f1a9ec561..82106363ed6 100644 --- a/packages/thirdweb/src/adapters/eip1193/to-eip1193.test.ts +++ b/packages/thirdweb/src/adapters/eip1193/to-eip1193.test.ts @@ -190,6 +190,49 @@ describe("toProvider", () => { ).resolves.toEqual([]); }); + test("removeListener should detach a listener registered via on", () => { + const provider = toProvider({ + chain: ANVIL_CHAIN, + client: TEST_CLIENT, + wallet: mockWallet, + }); + + const listener = vi.fn(); + provider.on("accountsChanged", listener); + + emitter.emit("accountsChanged", [mockAccount.address]); + expect(listener).toHaveBeenCalledTimes(1); + + provider.removeListener("accountsChanged", listener); + + emitter.emit("accountsChanged", [mockAccount.address]); + expect(listener).toHaveBeenCalledTimes(1); + }); + + test("removeListener fully detaches a listener registered twice via on", () => { + const provider = toProvider({ + chain: ANVIL_CHAIN, + client: TEST_CLIENT, + wallet: mockWallet, + }); + + const listener = vi.fn(); + // register the same listener reference for the same event twice, then + // confirm a single removeListener call fully detaches it (the underlying + // wallet emitter dedupes by callback reference via a Set, so this must + // not require two removeListener calls). + provider.on("accountsChanged", listener); + provider.on("accountsChanged", listener); + + emitter.emit("accountsChanged", [mockAccount.address]); + expect(listener).toHaveBeenCalledTimes(1); + + provider.removeListener("accountsChanged", listener); + + emitter.emit("accountsChanged", [mockAccount.address]); + expect(listener).toHaveBeenCalledTimes(1); + }); + test("should use custom connect override when provided", async () => { const walletWithoutAccount = { ...mockWallet, diff --git a/packages/thirdweb/src/adapters/eip1193/to-eip1193.ts b/packages/thirdweb/src/adapters/eip1193/to-eip1193.ts index 1c831b35c1b..2692b30ee5e 100644 --- a/packages/thirdweb/src/adapters/eip1193/to-eip1193.ts +++ b/packages/thirdweb/src/adapters/eip1193/to-eip1193.ts @@ -58,10 +58,30 @@ export type ToEip1193ProviderOptions = { export function toProvider(options: ToEip1193ProviderOptions): EIP1193Provider { const { chain, client, wallet, connectOverride } = options; const rpcClient = getRpcClient({ chain, client }); + // tracks the unsubscribe fn returned by wallet.subscribe for each (event, listener) + // pair so removeListener can actually detach it, per the EIP-1193 contract. + const unsubscribes = new Map< + unknown, + // biome-ignore lint/suspicious/noExplicitAny: matches EIP1193Provider's loose typing + Map<(params: any) => any, () => void> + >(); return { - on: wallet.subscribe, - removeListener: () => { - // should invoke the return fn from subscribe instead + on: (event, listener) => { + const unsubscribe = wallet.subscribe(event, listener); + let listeners = unsubscribes.get(event); + if (!listeners) { + listeners = new Map(); + unsubscribes.set(event, listeners); + } + listeners.set(listener, unsubscribe); + }, + removeListener: (event, listener) => { + const listeners = unsubscribes.get(event); + const unsubscribe = listeners?.get(listener); + if (unsubscribe) { + unsubscribe(); + listeners?.delete(listener); + } }, request: async (request) => { switch (request.method) {