Amplitude and Madkudu integrations break with SystemJS

[robmosca]

When trying to initialize Amplitude Classic or Madkudu in a website using SystemJS, the initialization fails with the following message:

analytics.min.js:1 Madkudu TypeError: window.require is not a function

This is due to the fact that the condition on this line is true for SystemJS, but SystemJS does not define window.require (see here and here).

Maybe to the condition on line 13 a check should be added for window.require to be defined.

[telmaantunes]

I've got the same issue for the bugsnag integration. What was your solution?

[robmosca]

Unfortunately, disabling the integrations... :(
For amplitude we moved to server side integration. Madkudu we just disabled it.

[robmosca]

The fix should be as easy as changing the following lines:

analytics.js-integrations/integrations/amplitude/lib/index.js

Line 19 in 548c10c

var umd = typeof window.define === 'function' && window.define.amd;

analytics.js-integrations/integrations/bugsnag/lib/index.js

Line 15 in 548c10c

var umd = typeof window.define === 'function' && window.define.amd;

analytics.js-integrations/integrations/madkudu/lib/index.js

Line 13 in 548c10c

var umd = typeof window.define === 'function' && window.define.amd;

in this way

var umd = typeof window.define === 'function' && window.define.amd;

to

var umd = typeof window.define === 'function' && window.define.amd && window.require;

[GhassenRjab]

Hey @robmosca,

I think replacing

var umd = typeof window.define === 'function' && window.define.amd;

with

var umd = typeof window.define === 'function' && window.define.amd && window.require;

won't fix the issue.

var umd will have a falsy value and the test on line 44 won't pass. Hence, MadKudu.js won't be loaded.

I am looking at the SystemJS documentation in order to see how we can import MadKudu.js here.

[robmosca]

@GhassenRjab: the test on line 44 won't pass and MadKudu.js will instead be loaded via the traditional way (script element injection) at line 59.

The test at line 44 is just to use RequireJS instead of load() (hence the return statement on line 56).

Or maybe I misunderstand the code? 🤔

[GhassenRjab]

You're right! Nice catch!

I'll open a PR with the suggested solution

[robmosca]

Yeah, I also tested it in Chrome overriding the code of the integration with the proposed change and it seems to work. 👍🏽
Thanks @GhassenRjab!

[GhassenRjab]

@robmosca the PR has been merged. I think a new version has been released