fix: EventEmitter memory leak

[jaredperreault-okta]

No description provided.

[jaredperreault-okta]

This listener bound on every Credential instance, but filter to only fire on the specific Credential. It's incorrectly space in method space. Moved this logic to static section where other listeners are bound

[jaredperreault-okta]

The tags_updated trigger moved from observeToken

[jaredperreault-okta]

Added a method to clean resources used by Credential instances. Since the tags_updated listener was moved to static space, the only instance-level listener is token_did_refresh of the OAuth2 instance with Credential. oauth2.dispose clears that listener.

Also added an AbortController instance of each Credential for future proofing of clearing resources

[jaredperreault-okta]

The pattern of clearing this.oauth2.emitter listeners also solves this nested listener which contributed to the memory leak

https://github.com/okta/okta-client-javascript/blob/master/packages/auth-foundation/src/Credential/CredentialCoordinator.ts#L128

[jaredperreault-okta]

calls new .dipose method on Credentials during .remove()

[jaredperreault-okta]

Used within Credential via OAuth2.dispose. Clears active listeners on the EventEmitter to prevent similar leaks

[jaredperreault-okta]

Used within Credential.dispose. Clears active listeners on this.emitter via super.dispose to prevent similar leaks. Also clears the #httpCache

[jaredperreault-okta]

This pattern isn't really utilized yet, but for future-proofing. Listens on a provided AbortSignal (presumably via an AbortController) and clears the event listener bound at the same time as the AbortSignal. Could useful to assist resource clean up of other entities in the future

[jaredperreault-okta]

Need to confirm with testing this section is no longer needed

[jaredperreault-okta]

Per AI suggestion, there is probably no need to broadcast the token value when synchronizing tabs, each tab can access the token value from storage (the source of truth) to save on memory usage

[rchild-okta]

Re-reviewed at b7c2850 against the earlier pass at bb7a297. I couldn't run the suites locally (yarn install fails fetching the larger tarballs in my environment), so this is from reading the branch — CI is the source of truth.

The leak fix looks right. Three independent changes, any one of which would have helped: hoisting the metadata_updated binding out of observeToken() into the static coordinator setter, dispose() wired into remove()/clear(), and dropping the token body from broadcasts. Worth stating explicitly because it's the durable property: after the hoist, the only subscription a Credential holds is to its own oauth2.emitter, so even an un-disposed Credential is self-contained and collectable — the coordinator-emitter leak can't recur through that path.

Resolved since the last pass, each with a test I'd call load-bearing: the already-aborted-signal case in EventEmitter.on (EventEmitter.ts:37-40, test at EventEmitter.spec.ts:122); the signals WeakMap keying, now per-event, with the cross-event contamination test at :176; and cross-tab credential_removed for unmaterialized credentials (spa-platform/.../CredentialCoordinator.ts:142-152, both branches covered at :160-176). The new coordinator spec and the metadata_updated listener-count test close the coverage gaps I flagged.

One thing regressed. TokenStorage.ts:340 and :351 now emit token_removed from handleReadError/handleDecryptionError, and the only subscriber is the broadcast at spa-platform/.../CredentialCoordinator.ts:77-79. So a transient read failure in one tab now fans credential_removed out to every other tab, each disposing its Credential and surfacing the event to the app — while the tab that actually failed keeps its own credential, since nothing subscribes locally. A silent per-tab null became a synchronized cross-tab removal that skips the faulting tab. If the emit is deliberate, the fan-out should be too; otherwise use a non-destructive read there, or suppress the emit when the read originated from a broadcast. Untested either way — the new spec sets encryptAtRest = false (:22).

Refresh echo now costs a decrypt per message. :51-53 rebroadcasts on any local credential_refreshed and the receive branch re-emits at :179, re-triggering it. The echo terminates on Token.isEqual, but the receive path now does tokenStorage.get(id) — read, decrypt, new Token — where it previously took the token off the payload. With T tabs holding the credential that's O(T²) handlings each doing a decrypt per rotation, which partly reintroduces the per-tab allocation that dropping the payload was meant to remove. Simplest fix is to skip the re-emit for broadcast-originated updates.

Smaller items. dispose() is still public and doesn't evict from the dataSource — worth @internal, especially now that APIClient.dispose() splices interceptors (:62-65), so calling it on a live credential also strips its request interceptors. #pendingRefresh/PromiseQueue still aren't cleared alongside #httpCache. credential_added changed to { credential?: Credential, id: string } — credential is optional now and undefined for cross-tab events; that's the breaking change here and it isn't in the CHANGELOG, nor is the tags_updated widening. And the fixture change at BrowserTokenStorage.spec.ts:213-216 masks a real shortID() encoding issue rather than fixing it — I've written that up separately.

Nits: "resourece" in both dispose docs (APIClient.ts:60, client.ts:149), "gargabe"/"sigificant" in Credential.dispose(). Also: the new spec pairs jest.useFakeTimers() with a handler that awaits pause(50) when isFirefox() — passes under jsdom's UA, but hangs rather than fails if that ever flips.

---

_{Analysis produced with Claude Code and not executed locally — see the caveat above.}