Heartbeat ak

[ak--47]

this PR adds a new .heartbeat() method to the SDK. it aggregates values client-side and sends a single event on flush:

  // Record 3 heartbeat events  
  mixpanel.heartbeat('watch video', 'video-abc-123');
  mixpanel.heartbeat('watch video', 'video-abc-123'); // 10 sec later
  mixpanel.heartbeat('watch video', 'video-abc-123'); // 10 sec later

  // Wait for autoflush or flush manually
  mixpanel.heartbeat().flush(); // → mixpanel.track('watch video', {
  //   $duration: 30, 
  //   $heartbeats: 3, 
  //   $contentId: "video-abc-123"
  // })

this is useful for streaming or buffering events (video + audio players) where you don't want to send each heartbeat to mixpanel, and you only care about the total aggregated "watch time" or "listen time".

therefore, heartbeat() can safely be called in a loop, and solves a common challenge of summarizing events before sending to mixpanel.

this PR also adds some vscode quality-of-life features to repo, like "one click tests" etc...

[ak--47]

customer use case thread: https://mixpanel.slack.com/archives/C08CJKNNURW/p1749733041454079

internal docs: https://www.notion.so/mxpnl/Heartbeat-Events-212e0ba9256280e883dbed10e3b31e87

[ak--47]

converting to draft state following feedback from @tdumitrescu and @jakewski to simplify the module's API and maybe just make it a method like: mixpanel.heartbeat(event, contentId, props, config)

[ak--47]

the new heartbeat API will go from 6 public methods and 5+ configuration options down to 1 method + 2 options 👍

(importantly, we will not expose a "flush" interface and just do timeout or page unload based-flushing)

/**
     * @function heartbeat
     * @memberof mixpanel
     * @param {String} eventName The name of the event to track
     * @param {String} contentId Unique identifier for the content being tracked
     * @param {Object} [props] Properties to aggregate with existing data
     * @param {Object} [options] Configuration options
     * @param {Number} [options.timeout] Timeout in milliseconds (default 30000)
     * @param {Boolean} [options.forceFlush] Force immediate flush after aggregation
* */

[ak--47]

transitioning back to ready for review @tdumitrescu

we have greatly simplified the API; it's one method mixpanel.heartbeat() ... only 2 auto flushing strategies: timeout + page unload.

unit tests + integration tests should be comprehensive. thanks for the guidance!

[jakewski]

the mock lib you have here seems a bit too exhaustive. isn't this test case literally testing against the code you wrote up L203? hard to tell which part of the heartbeat functionality in the main lib is getting tested

MixpanelLib is hard to test in the node env because it's pretty heavily tied to various browser APIs (not that it can't be done, but we've always favored browser tests to help catch real-world scenarios) - probably why you needed this super comprehensive mock right?

that being said - having unit tests are super reliable and easy to iterate on. maybe there's a way we can encapsulate the heartbeat functionality and inject the dependencies it needs from MixpanelLIb - e.g. make heartbeat a separate module that MixpanelLib uses under the hood, and allow that module to be tested in this file.

// MixpanelLib
this.heartbeat = new MixpanelHeartBeat({ // this is imported a different file
  onFlushEvent: function (ev, props) { ... },
  persistence: this.persistence...
})

then we can mock out those args, and test actual business logic in here. less LOCs in mixpanel-core is a plus - what do you think?

[ak--47]

@jakewski thank you so much for the notes!

• mock was created this way to ensure that any config changes / GDPR are respected and also that the track calls are correct

• strong agree that less LOC in mixpanel-core is better, but i imagined as a separate file this starts to feel like a plugin rather than just convenience method

• is there an example of this type dependency injection elsewhere in the repo i could borrow from?

[jakewski]

1. I'm not sure that's true - the mock is reimplementing a lot of functionality so you'd basically be rewriting the mock when you need to do a config change
2. feeling like a plugin to us as the developers is a good thing. it isolates this functionality so that it's easy to find and allows us to test the business logic without worrying about the rest of the lib. to the end user it can look exactly the same
3. good examples are autocapture, session recording

[tdumitrescu]

I don't know that persisting heartbeat data in the cookie is a great move here. If this system really needs browser persistence, we can probably hook into the existing batch queue utils with some modification, which support indexeddb and localstorage. That might be another overall approach too: make heartbeats a special queue in the request batcher, an event which doesn't get flushed on a regular schedule, and every time a new heartbeat happens, the queued event is rewritten. @jakewski any thoughts?

[jakewski]

oh yeah this persistence can be cookie or localstorage depending on config right - I agree that cookie is weird for this.

I like the idea of a new RequestBatcher for heartbeat events specifically, but trying to rewrite the event for each heartbeat might be difficult and go against the design of the queue.

maybe we can queue up all heartbeats, and do the aggregation only once it's time to flush? I think that design would have some benefits on its own and potentially just use indexeddb off the bat to make sure there's no storage limit issues

[tdumitrescu]

Yeah, that would work well - queue heartbeats like any other event, but consolidate them when flushing. (Hardest part would be working out deduplication in case you need to re-send; maybe the summary event's Insert ID would be deterministic based on the heartbeats it's rolling up?)

[ak--47]

removing persistence for now. i forgot about cookie persistence pattern. such a gotcha.

i think we can persistence back if it's actually requested (i.e. the 'best effort' flush on unload is not good enough)

[tdumitrescu]

Is that every 30 seconds or after 30 seconds of inactivity?

Maybe I've missed it/misunderstood, but I'm surprised this method doesn't have an option for the SDK to repeat heartbeats on a regular schedule, so you don't have to keep calling heartbeat over and over yourself e.g.

const videoHeartbeat = mixpanel.heartbeat(
  'podcast_listen', 'episode_123',
  {platform: 'mobile'},
  {interval_seconds: 5},
);

// then when video stops:
videoHeartbeat.stop();

(I haven't thought through that signature too much ^^^ but something like that I imagine would let you just call start/stop methods and not worry about managing your own timers)

[ak--47]

heard. 🤔

[ak--47]

@jakewski + @tdumitrescu

thank you both for the thoughtful discussion. i had some time to work through this today, and i decided:

• we don't need persistence for heartbeat() ... it tries to autoflush on page exit (because if the end-user leaves the page, they are no longer listening/watching/etc...) so we actually don't need to persist anything (which greatly simplifies the surface area of this feature)

• we no longer mock the entire lib in ./tests/unit/heartbeat.js ... so way less LOC ... and now we are essentially trying to see if heartbeat() calls track() correctly (@jakewski does this address your feedback properly? good mocks are not my strong suit 😅)

i still have to think about @tdumitrescu's comment on providing explicit .start() and .stop() methods (like we do with session recording API) which makes sense to me, however the core reason i designed the current API is because i understand media players in the wild offer dead simple loop for "user is still watching" and so i kinda designed the module as "it's ok to call this in that loop"...

the simplest example would be the timeupdate API for <video> elements:

<video id="my-video" width="640" height="360" controls>
    <source src="your-video-file.mp4" type="video/mp4">
</video>

<script>
    const videoElement = document.getElementById('my-video');
    const videoId = 'product_demo_123';
    videoElement.addEventListener('timeupdate', () => {
        if (!videoElement.paused) {
            mixpanel.heartbeat('video_watch', videoId);
        }
    });
</script>

this pattern is well-supported by video.js and vimeo's player SDK

however, if i get @tdumitrescu's point properly, this approach likely requires boilerplate to work with other third party plugins like youtube's iframe API:

// this is what we don't want
let heartbeatInterval;

function onYouTubeIframeAPIReady() {
    new YT.Player('player', {
        videoId: 'foo', 
        events: {
            'onStateChange': onPlayerStateChange
        }
    });
}

function onPlayerStateChange(event) {
    const videoId = 'foo';

    // If the video is now playing, start the heartbeat loop
    if (event.data == YT.PlayerState.PLAYING) {
        // Clear any existing interval to be safe
        clearInterval(heartbeatInterval);
        
        heartbeatInterval = setInterval(() => {
            console.log('YouTube heartbeat...');
            mixpanel.heartbeat('video_watch', videoId);
        }, 5000); // oy!
    } 
    else {
        clearInterval(heartbeatInterval);
    }
}

in this case it would be easier to just:

 if (event.data == YT.PlayerState.PLAYING) {
        mixpanel.heartbeat.start('video_watch', videoId);
    }
    else {     
        mixpanel.heartbeat.stop('video_watch', videoId);
    }

and call it a day... but then we have to stub .heartbeat.start() and .stop() in the snippet, right?

(( would it be ok to support both APIs? is it a problem for heartbeat() to both be a method and have props that are methods heartbeat.start() ? ))

thoughts?

(also why are int tests failing even though i didn't make any changes to them in the last commit?!)