refactor: Migrate Instance Manager Tests to TypeScript #953
Conversation
alexs-mparticle
added
wait to merge
refactor
alexs-mparticle
force-pushed
the
refactor/SQDSDKS-6771-mp-instance-manager-tests
branch
from
254cdd0 to
d092629
Compare
rmi22186
left a comment
rmi22186
left a comment
There was a problem hiding this comment.
tiny nits, otherwise, looks great!
src/sdkRuntimeModels.ts
Outdated
| setDeviceId(deviceId: string): void; | ||
| setSessionAttribute(key: string, value: string): void; | ||
| getInstance(): MParticleWebSDK; // TODO: Create a new type for MParticleWebSDKInstance | ||
| getInstance(instanceName?: string): MParticleWebSDK; // TODO: Create a new type for MParticleWebSDKInstance |
There was a problem hiding this comment.
nice catch on missing argument here!
add an mp work item for this instead please.
There was a problem hiding this comment.
I think we should possibly do this as part of https://go.mparticle.com/work/SQDSDKS-4804
src/sdkRuntimeModels.ts
Outdated
| ProductActionType: SDKProductActionType; | ||
| generateHash(value: string): string; | ||
| isIOS?: boolean; | ||
| sessionManager: Pick<ISessionManager, 'getSession'>; |
There was a problem hiding this comment.
can we also add a work item, or perhaps add to the V3 item about making the public API for this better? perhaps we only need to put getSession on the instance and not require a customer to use mp.sessionManager.getSession(). not a decision, but just something to flag given it's just 1 method.
There was a problem hiding this comment.
| 'Other5', | ||
| 'Other6', | ||
| 'Other7', | ||
| 'Other8', | ||
| 'Other9', | ||
| 'Other10', | ||
| 'MobileNumber', | ||
| 'PhoneNumber2', | ||
| 'PhoneNumber3', |
alexs-mparticle
force-pushed
the
refactor/SQDSDKS-6771-mp-instance-manager-tests
branch
from
0d6a2a7 to
1cf2faf
Compare
|
alexs-mparticle
merged commit 802b639
into
refactor/ts-migration-blackout-2024
Instructions
developmentSummary
Testing Plan
Reference Issue (For mParticle employees only. Ignore if you are an outside contributor)