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 the SDK’s identity/login/modify/logout and persistence interfaces; while intended as a type-only migration, small signature/shape tweaks (e.g., request field names and optional params) could surface integration mismatches at compile/runtime boundaries.

Overview
Migrates the core Identity module from JS to TypeScript (identity.ts), adding explicit types across identity flows (identify/login/logout/modify, user object methods, aliasing, and search) and replacing many untyped var usages with typed const/let.

Adjusts several public/internal type contracts to match runtime behavior, including identity request shapes (context now Context | null, request_timestamp_ms naming), broader identity/attribute value typing, identity caching callback types, and updated client/persistence/store/mp-instance interfaces (e.g., sendIdentityRequest accepting modify requests and optional knownIdentities, persistence firstSeen/lastSeen types, and _IdentityAPIClient typed as IIdentityApiClient). Tests are updated accordingly.

^{Reviewed by Cursor Bugbot for commit 85a0f3c. 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.}

cursor · 2026-05-20T20:04:39Z

+                    const aliasRequestMessage = mpInstance._Identity.IdentityRequest.createAliasNetworkRequest(
                        aliasRequest
-                    );
+                    ) as IAliasRequest;


Incorrect type cast masks alias network request shape

Medium Severity

The return value of createAliasNetworkRequest (which has fields like request_id, request_type, api_key, data) is cast as IAliasRequest, but IAliasRequest has a completely different shape (destinationMpid, sourceMpid, startTime, endTime). This incorrect assertion propagates through sendAliasRequest, whose parameter is also typed as IAliasRequest. While no runtime breakage occurs today because the object is only JSON-stringified, the misleading type could cause silent undefined values if anyone later accesses aliasRequest.destinationMpid inside sendAliasRequest trusting the declared type.

Additional Locations (1)

src/identityApiClient.ts#L35-L38

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

Will address this in follow-up branch fix/SDKE-1103-alias-network-request-type by introducing IAliasNetworkRequest interface and updates sendAliasRequest + tests to use it.

sonarqubecloud · 2026-05-21T01:50:57Z

Quality Gate passed

Issues
25 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

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

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?

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

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.

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.

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.

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

split comma-separated declaration

85a0f3c

alexs-mparticle requested changes May 21, 2026

View reviewed changes

crisryantan reviewed May 21, 2026

View reviewed changes

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!

cursor Bot May 20, 2026

Choose a reason for hiding this comment

Incorrect type cast masks alias network request shape

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud Bot commented May 21, 2026

Quality Gate passed

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!

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