Skip to content

Queue calls while awaiting load when using the NPM package #428

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
tsheaff opened this issue Apr 8, 2022 · 9 comments
Closed

Queue calls while awaiting load when using the NPM package #428

tsheaff opened this issue Apr 8, 2022 · 9 comments

Comments

@tsheaff
Copy link

tsheaff commented Apr 8, 2022

I'm loading from NPM rather than through the snippet.

I may be misunderatanding how AnalyticsBrowser.load() works, and I see reference in the docs to the library enqueuing calls before the script loads, but according to the example in the README, the analytics instance is optional until await AnalyticsBrowser.load({ ... }) is completed. This means any analytics?.track(...), analytics?.identify(...) etc. will be no-ops while awaiting. This seems quite bad since there will now be race conditions between our initial events like page views, versus the time for the AnalyticsBrowser.load to complete.

Am I misunderstanding things? How can I get all of my calls to analytics to work, even those that run immediately after calling await AnalyticsBrowser.load but before that response has come back? Do I have to wrap the lib and implement enqueing myself?

cc @chrisradek @pooyaj @danieljackins

@pooyaj
Copy link
Contributor

pooyaj commented Apr 8, 2022

@tsheaff The npm version (unlike the CDN/snippet based solution) requires you to implement queuing yourself. Although the implementation should be straightforward. Can you check the response on this issue and see if that solves your issue: #374

@tsheaff
Copy link
Author

tsheaff commented Apr 8, 2022

Ah OK thanks for the fast reply @pooyaj!

That's unfortunate. I did see that queueing in the snippet. Unfortunately that doesn't work in Typescript. It seems like this could be built into the library. For example:

const [analytics] = AnalyticsBrowser.configure(...); // same options as `.load(...)`
await analytics.load();

Then analytics.track(...) etc can all work instantly, with the queueing behavior being something that moves inside the library and then is flushed when .load() finished. So loaded vs not is transparent to the caller.

Here's a solution that works for just 3 of the methods on analytics objects, but is fully typesafe. It does require importing from within the package tho. A sanity-check would be fantastic @pooyaj 🙏

import { Analytics, AnalyticsBrowser } from '@segment/analytics-next';
import { EventParams, PageParams, UserParams } from '@segment/analytics-next/dist/pkg/core/arguments-resolver';

type EnqueuedSegmentCall =
{
  type: 'track';
  args: EventParams;
} |
{
  type: 'page';
  args: PageParams;
} |
{
  type: 'identify';
  args: UserParams;
};

class Analytics {
  private analytics: Analytics | undefined;

  constructor() {
    this.load();
  }

  private async load() {
    const writeKey = process.env.REACT_APP_SEGMENT_WRITE_KEY;
    const cdnURL = process.env.REACT_APP_SEGMENT_CDN_PROXY;
    if (!writeKey) throw new Error('Segment Write Key Cannot Be Undefined');
    if (!cdnURL) throw new Error('Segment CDN URL Cannot Be Undefined');

    const [analytics] = await AnalyticsBrowser.load({ writeKey, cdnURL });
    this.analytics = analytics;

    this.flushQueue();
  }

  private enqueuedCalls: EnqueuedSegmentCall[] = [];

  private flushQueue() {
    this.enqueuedCalls.forEach((call) => {
      switch (call.type) {
        case 'track':
          return this.track(...call.args);
        case 'page':
          return this.page(...call.args);
        case 'identify':
          return this.identify(...call.args);
      }
    });
    this.enqueuedCalls = [];
  }

  track(...args: EventParams) {
    if (!this.analytics) {
      this.enqueuedCalls.push({ type: 'track', args: args });
      return;
    }
    this.analytics.track(...args);
  }

  page(...args: PageParams) {
    if (!this.analytics) {
      this.enqueuedCalls.push({ type: 'page', args: args });
      return;
    }
    this.analytics?.page(...args);
  }

  identify(...args: UserParams) {
    if (!this.analytics) {
      this.enqueuedCalls.push({ type: 'identify', args: args });
      return;
    }
    this.analytics?.identify(...args);
  }
}

export default new Analytics();

This then gives us this very clean interface throughout our calling code:

import analytics from '@analytics/analytics';

analytics.track(...);
analytics.page(...);
analytics.identify(...);

@pooyaj
Copy link
Contributor

pooyaj commented Apr 8, 2022

@tsheaff your solution looks great 🙌 I think we will build queuing mechanism for our npm package in near future as well so you don't have to wrap the library/import internal types

@tsheaff tsheaff changed the title Queue calls for replay after site loads Queue calls when loading from NPM package Apr 8, 2022
@tsheaff
Copy link
Author

tsheaff commented Apr 8, 2022

Ah OK great. I'll keep track of this issue until that's done. Can you leave this open and track that PR against this issue? I've renamed it for more clarity.

@tsheaff
Copy link
Author

tsheaff commented Apr 8, 2022

I think it will require a new interface as I mentioned as well so that you can get your analytics instance without any await

@tsheaff tsheaff changed the title Queue calls when loading from NPM package Queue calls while awaiting load when using the NPM package Apr 8, 2022
@silesky
Copy link
Contributor

silesky commented Apr 20, 2022

@tsheaff this is an awesome suggestion! We're actively working on this feature, I will tag you in the PR.

@silesky
Copy link
Contributor

silesky commented May 19, 2022

Feature merged in #436, should be available next release.

@silesky
Copy link
Contributor

silesky commented May 25, 2022

@silesky silesky closed this as completed May 25, 2022
@dwilt
Copy link

dwilt commented Jun 27, 2022

Thank you so much for implementing this! You saved us a ton of work! It worked great!

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

No branches or pull requests

4 participants