Skip to content

Conversation

@jaissica12
Copy link
Contributor

Instructions

  1. PR target branch should be against development
  2. PR title name should follow this format: https://github.com/mParticle/mparticle-workflows/blob/main/.github/workflows/pr-title-check.yml
  3. PR branch prefix should follow this format: https://github.com/mParticle/mparticle-workflows/blob/main/.github/workflows/pr-branch-check-name.yml

Summary

Testing Plan

  • Was this tested locally? If not, explain why.
  • {explain how this has been tested, and what, if any, additional testing should be done}

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

}
}

if (forwarderSettings.recordHeatmapData != null) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like there's primarily 2 patterns the settings are following here, either if something is a boolean or if it's a number.

I think you can probably do something like

const boolSessionReplaySettings = ['recordHeatmapData', 'autocapture', etc, etc]
const numberSessionReplaySettings = ['recordIdleTimeoutMs', 'recordMaxMs', etc, etc]

boolSessionReplaySettings.forEach(function(boolSetting) {
   if (boolSetting != null) { initOptions[boolSetting] =  boolSetting === 'True'}
})

// something similar for the numberSessionReplaySettings

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Used dictionaries with { key, mappedKey } since forwarderSettings use camelCase (recordHeatmapData), but Mixpanel's API expects snake_case (record_heatmap_data).

Comment on lines 76 to 78
!isNaN(sessionPercent) &&
sessionPercent >= 0 &&
sessionPercent <= 100
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would argue we should just have Mixpanel do the validation on their side and not worry about it on our side.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

@jaissica12 jaissica12 requested a review from rmi22186 December 19, 2025 15:27
@jaissica12 jaissica12 force-pushed the feat/SDKE-663-update-kit-to-support-session-replay-integration branch from 5f3b1de to f2c2ed0 Compare December 19, 2025 16:03

// Process string settings
stringSettings.forEach(function (setting) {
if (forwarderSettings[setting.key]) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason why you're not checkign != null here?

If this was missed, we could make a simple utility function to make this a little readable.

function hasValue(obj, key) {
    return obj[key] != null;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't add != null for string settings as it would allow empty strings like recordMaskTextSelector: '' to pass through to Mixpanel, which would break test like "should ignore empty string values for privacy selectors".

Comment on lines 119 to 125
var numericValue = parseInt(
forwarderSettings[setting.key],
10
);
if (!isNaN(numericValue)) {
initOptions[setting.mappedKey] = numericValue;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra credit: Maybe we can extract this logic into a utility function to improve readability.

function parseIntSafe(value) {
    var n = parseInt(value, 10);
    return isNaN(n) ? undefined : n;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants