refactor: update forwarder module to TS#1260
Conversation
PR SummaryMedium Risk Overview Expands forwarder/kit interfaces to include Reviewed by Cursor Bugbot for commit 15106a8. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
| import { Batch } from '@mparticle/event-models'; | ||
| import { IMParticleUser, ISDKUserIdentity } from './identity-user-interfaces'; | ||
| import KitBlocker from './kitBlocking'; | ||
| import { Dictionary } from './utils'; |
There was a problem hiding this comment.
Clean up these imports
| } | ||
| if ( | ||
| !self.isEnabledForUserAttributes( | ||
| !(this.isEnabledForUserAttributes as Function)( |
There was a problem hiding this comment.
Why do we need as Function?
|
|
||
| // Check user attribute filtering rules | ||
| clonedEvent.UserAttributes = KitFilterHelper.filterUserAttributes( | ||
| clonedEvent.UserAttributes = (KitFilterHelper.filterUserAttributes as Function)( |
There was a problem hiding this comment.
Same here. This feels like we're trying to bypass type safety.
| } | ||
|
|
||
| for (const forwarder of mpInstance._Store.activeForwarders) { | ||
| for ( |
There was a problem hiding this comment.
Is there a reason we needed to change this to a standard for loop?
|
|
||
| if (result) { | ||
| mpInstance.Logger.verbose(result); | ||
| mpInstance.Logger.verbose(result as string); |
There was a problem hiding this comment.
Isn't this already a string?
| // sideloaded kits, this will need to be refactored. | ||
| this.processSideloadedKits = function(mpConfig) { | ||
| this.processSideloadedKits = function( | ||
| mpConfig: IConfigResponse & { sideloadedKits?: { kitInstance: { register: Function; name: string }; filterDictionary: IKitFilterSettings }[] } |
There was a problem hiding this comment.
If we don't have a type for this, let's make one.
| endSession(): void; | ||
| init(apiKey: string, config: SDKInitConfig, instanceName?: string): void; | ||
| _getActiveForwarders(): ConfiguredKit[]; | ||
| _getActiveForwarders(): MPForwarder[]; |
There was a problem hiding this comment.
Let's be careful with this because MPForwarder and ConfiguredKit might have specific "contracts".
| name: string | ||
| ): Dictionary<string> | null; | ||
| filterUserIdentities?( | ||
| userIdentitiesObject: Record<string, string>, |
There was a problem hiding this comment.
I think we may have this type defined for this.
2 participants
Background
What Has Changed
src/forwarders.jstosrc/forwarders.tswith full type annotations on all methods, parameters, and return typessrc/forwarders.interfaces.tswith corrected and expanded interface definitions to support the conversionsrc/sdkRuntimeModels.tswith type adjustments needed for forwarder-related modelssrc/store.tswith type refinements required by the forwarder moduleScreenshots/Video
Checklist
Additional Notes
Reference Issue (For employees only. Ignore if you are an outside contributor)