feat: Improved queue flushing

[marandaneto]

Problem

Fixes https://github.com/PostHog/posthog-js-lite/issues/176

Changes

• Flushing will only remove from the queue if it has reliably been dealt with
• Only one flush can happen at a time but with an increased maxBatchSize
• Shutting down works as designed, iteratively calling flush until the queue is exhausted or the timeout is reached.

Release info Sub-libraries affected

Bump level

• [ ] Major
• [ ] Minor
• [x] Patch

Libraries affected

• [x] All of them
• [ ] posthog-web
• [ ] posthog-node
• [ ] posthog-react-native

Changelog notes

• Added support for X

[github-actions[bot]]

Size Change: +534 B (+1%)

Total Size: 72.3 kB

Filename	Size	Change
posthog-node/lib/index.cjs.js	19.6 kB	+120 B (+1%)
posthog-node/lib/index.esm.js	19.6 kB	+122 B (+1%)
posthog-web/lib/index.cjs.js	16.6 kB	+146 B (+1%)
posthog-web/lib/index.esm.js	16.5 kB	+146 B (+1%)

_{compressed-size-action}

[marandaneto]

What happens if you flush twice in a row? Feels like we are missing something to block multiple flushes (or at least handle them correctly).

Similarly we might want to check that the shutdown logic would still work if we block multiple flushes at once

True, I assumed the method was sync but there's a then call at the end, and that would lead to race conditions and messing up the queue, good catch. Only 1 flush per time is allowed to avoid such races using the state flag isFlushing. The shutdown logic should be fine because it awaits all pending promises anyway and call flush again. WDYT? can write tests if you think this is enough now.

[marandaneto]

What happens if you flush twice in a row? Feels like we are missing something to block multiple flushes (or at least handle them correctly).

Similarly we might want to check that the shutdown logic would still work if we block multiple flushes at once

The flush method only calls removeItemsFromQueue and it's a sync method and JS is single-threaded. In React Native land we could use useState to be sure and avoid race conditions, but this method isn't async so I guess this is fine?

Happy to jump in and help here as this is a bit more JS-y than mobile-y

yep, made a new comment here, but happy to hear your thoughts since this has node/web impact.

promise-hold approach would work but I wonder if it's really necessary, I'd say it's only fully necessary if we want close to real-time events, where any call to flush or flushAsync cannot be missed and cannot wait the next timer cycle. The only downside is the shutdownAsync, but as mentioned in my comment, even with the current implementation, the Queue isn't fully flushed.