refactor: migrate identity module to TS by jaissica12 · Pull Request #1248 · mParticle/mparticle-web-sdk

jaissica12 · 2026-04-13T21:45:44Z

Background

The identity.js module was one of the remaining JavaScript files in the SDK. As part of the ongoing TypeScript migration effort (SDKE-1103), it needed to be converted to TypeScript to improve type safety and maintainability. This is a direct migration with no behavior, API, or breaking changes.

What Has Changed

Renamed src/identity.js to src/identity.ts with type annotations added to all functions, parameters, and variables
Corrected pre-existing type mismatches in identity.interfaces.ts, identityApiClient.ts, persistence.interfaces.ts, store.ts, and mp-instance.ts where interfaces didn't match actual runtime behavior
Updated test files to align with corrected interface types
No runtime logic, public API, or deprecated behavior was changed — compiled output is functionally identical

Testing

All existing Karma + Mocha integration tests pass
All existing Jest unit tests pass
Manual end-to-end testing performed via rokt-web-playground with local SDK build, covering:
- SDK initialization and identify callback (httpCode 200)
- Login with multiple identity types (email, customerid, facebook, mobile_number)
- Modify with no-op (httpCode 400 — expected) and actual changes (httpCode 200)
- Logout → anonymous MPID transition (ephemeral, is_logged_in: false)
- Anonymous user: page views, commerce events, user attributes all fire with correct anonymous MPID and empty identities
- Re-login from anonymous state restores known MPID with full identity set
- User attribute set/remove with forwarder callbacks (Rokt Kit)
- Session end/start across identity transitions
- All event batches uploaded successfully (202) with correct MPID and identity payloads

Screenshots/Video

Checklist

I have performed a self-review of my own code.
I have made corresponding changes to the documentation.
I have added tests that prove my fix is effective or that my feature works.
I have tested this locally.

Additional Notes

The main self = this pattern in the Identity constructor (~31 usages) was intentionally left as-is for a follow-up refactor to arrow functions, to keep this PR focused on the JS→TS migration only.
var → const/let replacements and removal of unnecessary type assertions were included as part of SonarCloud compliance.

Reference Issue (For employees only. Ignore if you are an outside contributor)

Closes https://go/j/SDKE-1103

sonarqubecloud · 2026-05-12T21:37:18Z

Quality Gate passed

Issues
72 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
3.0% Duplication on New Code

See analysis details on SonarQube Cloud

…in identity

cursor · 2026-05-20T19:58:48Z

PR Summary

Medium Risk
Touches identify/login/logout/modify, persistence, and forwarder hooks across a large file; risk is mainly regression despite the stated no-behavior-change goal and existing test coverage.

Overview
This PR migrates the core identity implementation from JavaScript to TypeScript (identity.js → identity.ts) by adding types across identity APIs, request builders, user helpers, and response parsing, while keeping the public behavior the same.

Interface and wiring updates align types with runtime usage: identity request payloads use request_timestamp_ms, Context, and IKnownIdentities; preprocess results expose cleanedIdentities; modify flows type createIdentityChanges as an array; alias callbacks use IAliasCallback; IIdentity / IIdentityApiClient / store and persistence signatures are tightened (e.g. optional knownIdentities on modify, mpid on store, first/last seen times as numbers).

Small defensive tweaks include optional kit logOut, checking typeof forwarder.logOut === 'function', safer validation logging, and test updates for the corrected types. var → const/let is included for lint compliance without intended logic changes.

^{Reviewed by Cursor Bugbot for commit d408003. Bugbot is set up for automated code reviews on this repo. Configure here.}

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 8f4c21b. Configure here.}

alexs-mparticle · 2026-05-21T14:28:32Z

    UserIdentities,
-    IdentityCallback,
 } from '@mparticle/web-sdk';
+import { IdentityCallback } from './identity-user-interfaces';


Clean up the imports

alexs-mparticle · 2026-05-21T14:29:02Z

    identityApiData: IdentityApiData,
    identityMethod: string,
-    knownIdentities: IKnownIdentities,
+    knownIdentities: IKnownIdentities | UserIdentities,


I'm not sure if these overlap cleanly

Good catch. IKnownIdentities | UserIdentities collapses to plain UserIdentities at the type level (since IKnownIdentities extends UserIdentities), so the union added zero safety and actually hid device_application_stamp

alexs-mparticle · 2026-05-21T14:29:52Z

    environment: Environment;
    request_id: string;
-    request_timestamp_unixtime_ms: number;
+    request_timestamp_ms: number;


Why are we renaming this?

Yeah, it's really a typo fix on the interface. The SDK has always been sending request_timestamp_ms, the interface was just declaring the different key name

alexs-mparticle · 2026-05-21T14:30:51Z

        previousIdentities: UserIdentities,
-        newIdentitie: UserIdentities
-    ): IIdentityAPIIdentityChangeData;
+        newIdentities: UserIdentities


How long has this typo been here? 😓

alexs-mparticle · 2026-05-21T14:31:39Z

    ): IUserAttributeChangeEvent;
    createUserIdentityChange(
-        identityType: SDKIdentityTypeEnum,
+        identityType: SDKIdentityTypeEnum | string,


I don't think we should use string but really limit this to known types

Tightened to SDKIdentityTypeEnum on the interface and narrowed the loop

crisryantan · 2026-05-21T18:10:13Z

                            aliasRequest.destinationMpid
                    );
-                    var aliasRequestMessage = mpInstance._Identity.IdentityRequest.createAliasNetworkRequest(
+                    const aliasRequestMessage = mpInstance._Identity.IdentityRequest.createAliasNetworkRequest(


This cast hides a real mismatch and is worth fixing before merge.

createAliasNetworkRequest returns the network wire shape:

{ request_id, request_type: 'alias', environment, data: { destination_mpid, source_mpid, start_unixtime_ms, end_unixtime_ms, device_application_stamp } }

But IAliasRequest is:

{ destinationMpid, sourceMpid, startTime, endTime, scope? }

Those are completely different shapes. The as IAliasRequest cast tells the compiler "trust me, it's an IAliasRequest" and from there sendAliasRequest(aliasRequest: IAliasRequest) is happy. At runtime sendAliasRequest just does JSON.stringify(aliasRequest) (identityApiClient.ts:113) so the actual wire format is still right, but the types are lying.

Agreed, the cast is hiding a real type drift. Already addressed in fix/SDKE-1103-alias-network-request-type branch

crisryantan · 2026-05-21T18:11:50Z

        callback: IdentityCallback,
        method: IdentityAPIMethod
    ): IdentityPreProcessResult;
+    createAliasNetworkRequest(aliasRequest: IAliasRequest): object;


Tied to the earlier comment. These three methods all return object, which is barely better than any. The migration goal is type safety, so this is a missed opportunity.

If you do above for createAliasNetworkRequest, the same pattern applies here, declare proper interfaces for the native shapes and use them. convertToNative returning object | void is especially confusing because void is meaningless as a return type union with anything else (it means "ignore the return value"), and the actual code returns undefined explicitly. IdentityNativeRequest | undefined would be clearer.

Happy to defer to a follow-up if you want to keep this PR scoped, but please file a ticket so it does not slip.

Agreed, object is barely better than any, and object | void is genuinely confusing semantics.
Picking up the two alias-related ones on fix/SDKE-1103-alias-network-request-type:
createAliasNetworkRequest: : object → : IAliasNetworkRequest
convertAliasToNative: : object → : INativeAliasRequest (new interface — { DestinationMpid, SourceMpid, StartUnixtimeMs, EndUnixtimeMs, Scope? })
Creating a separate ticket (https://go/j/SDKE-1204) for convertToNative since it's a different code path (identity native bridge, not alias), will switch to INativeIdentityRequest | undefined per your suggestion

crisryantan · 2026-05-21T18:13:25Z

                            ) {
-                                if (forwarder.logOut) {
-                                    forwarder.logOut(evt);
+                                const fwd = forwarder as unknown as Record<string, Function>;


Why the double-cast to Record<string, Function>? That tells the compiler "this thing is a string-keyed bag of arbitrary functions" which is the loosest possible cast and erases all type information from forwarder.

I checked ConfiguredKit in forwarders.interfaces.ts does not have a logOut method. So either:

The old JS code (if (forwarder.logOut)) was always-falsy defensive code that never actually fired. If that is the case, this whole block can be deleted.

There is a logOut method present at runtime on some kit implementations that the ConfiguredKit interface does not describe. If that is the case, add logOut?: (evt: SDKEvent) => void to ConfiguredKit and the cast disappears.

Option 2, logOut is real at runtime (MockForwarder defines it and the CBT test asserts logOutCalled === true after logout). Added logOut?: (evt: SDKEvent) => void to ConfiguredKit and dropped the cast.

sonarqubecloud · 2026-05-25T21:07:44Z

Quality Gate passed

Issues
23 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
2.9% Duplication on New Code

See analysis details on SonarQube Cloud

jaissica12 force-pushed the refactor/SDKE-1103-migrate-identity-module-to-ts branch from 54ee61d to 6013f98 Compare May 12, 2026 21:20

refactor: migrate identity module to TS

1cdacb3

jaissica12 force-pushed the refactor/SDKE-1103-migrate-identity-module-to-ts branch from 6013f98 to 1cdacb3 Compare May 12, 2026 21:36

replace var with const/let, remove unnecessary casts and unused self …

8f4c21b

…in identity

jaissica12 marked this pull request as ready for review May 20, 2026 19:58

jaissica12 requested a review from a team as a code owner May 20, 2026 19:58

cursor Bot reviewed May 20, 2026

View reviewed changes

Comment thread src/identity.ts

split comma-separated declaration

85a0f3c

alexs-mparticle requested changes May 21, 2026

View reviewed changes

crisryantan reviewed May 21, 2026

View reviewed changes

tighten identity type annotations per PR review

d408003

Conversation

jaissica12 commented Apr 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Background

What Has Changed

Testing

Screenshots/Video

Checklist

Additional Notes

Reference Issue (For employees only. Ignore if you are an outside contributor)

Uh oh!

sonarqubecloud Bot commented May 12, 2026

Quality Gate passed

Uh oh!

cursor Bot commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

crisryantan May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud Bot commented May 25, 2026

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jaissica12 commented Apr 13, 2026 •

edited

Loading

cursor Bot commented May 20, 2026 •

edited

Loading

crisryantan May 21, 2026 •

edited

Loading