Amplitude and Madkudu integrations break with SystemJS
robmosca opened this issue · 8 comments
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.
I've got the same issue for the bugsnag integration. What was your solution?
Unfortunately, disabling the integrations... :(
For amplitude we moved to server side integration. Madkudu we just disabled it.
The fix should be as easy as changing the following lines:
in this way
var umd = typeof window.define === 'function' && window.define.amd;to
var umd = typeof window.define === 'function' && window.define.amd && window.require;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.
@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? 🤔
You're right! Nice catch!
I'll open a PR with the suggested solution
Yeah, I also tested it in Chrome overriding the code of the integration with the proposed change and it seems to work. 👍🏽
Thanks @GhassenRjab!
@robmosca the PR has been merged. I think a new version has been released