BUG : All the methods that are called before the `load` method might never get fired

[duannx]

Describe the bug When using the SDK via the npm package, the methods that are called before the load method might never get fired

To Reproduce Let's say we initiate the analytics like this:

const Analytics = {
  instance: null,
  async init() {
    this.instance = await import('rudder-sdk-js')
    // When we did import the SDK, all the methods that are added to `window.rudderanalytics` will be 
    // immediately run but the load method is not called yet. So all the methods will be canceled.
    this.instance.load(WRITE_KEY,DATA_PLANE_URL )
  },
  trackBeforeLoad(methodName: string, ...args: any) {
    if (!window.rudderanalytics) {
      window.rudderanalytics = []
    }
    window.rudderanalytics.push(
      [methodName].concat(Array.prototype.slice.call(args))
    )
  },
  track(...args: Parameters<typeof RudderStackAnalytics.track>) {
    if (!this.instance) return this.trackBeforeLoad('track', ...args)
    return this.instance.track(...args)
  },
}

// Suppose we use the analytics like this
// This method will not be fired
Analytics.track('test')
await Analytics.init()
// This method is fired as expected
Analytics.track('test 2')

In the above scenario, the test method will not be fired because the analytics is not loaded yet. The reason is that the SDK currently guarantees the load call by using window.rudderanalytics. If we call the load method directly, other methods that are called before it will be ignored.

Expected behavior All the methods that are called before the load method should be fired regardless of the way we call the load method

Suggestion A workaround is adding the load method to window.rudderanalytics like this

window.rudderanalytics.push(['load', WRITE_KEY,DATA_PLANE_URL])

But it is not the natural way when we use an npm package. It only makes sense when we use the SDK from a CDN. Because all the methods check if (!this.loaded) return; before running, we should put the execution of all the methods after the loaded property is set like this:

this.loaded = true
processDataInAnalyticsArray(this)

This will guarantee the execution of all methods that are called before the load method and provide more future-proof.

Additional Information:

• SDK installation: NPM
• NPM Version: 2.15.0

[duannx]

Hi @saikumarrs, please take a look at this 🙏🏻

[saikumarrs]

Hi @duannx. Thanks for raising this issue. I took a brief look at it and agree with your suggestions. We made this particular change long ago and don't remember the full context of the reasoning behind it. Please be rest assured as @MoumitaM and I dig into this deeper next week and come back to you.

[duannx]

Hi @saikumarrs, thank you for your response. If you find a solution suitable for newbie contributors, I'd be happy to work on it.

[saikumarrs]

@duannx We've raised a PR to fix the issue. We'd greatly appreciate it if you could test and confirm they work.

[duannx]

@saikumarrs Thank you guys for raising the PR. I tested and found that the original issue is resolved. But I found an edge case that some methods will be ignored when the SDK has to load the polyfill. I already leave a review on the PR (mistakenly by another account, sorry about that)

[MoumitaM]

@duannx, Can you elaborate on the edge case? cause it is working fine even when the SDK is loading the polyfill.

[duannx]

@MoumitaM I've left a comment that elaborates on the edge case. Let's move the conversation to the PR

[saikumarrs]

The linked PR with the relevant fix has been merged. Hence, closing the issue.