Skip to content

Amplitude and Madkudu integrations break with SystemJS #761

Description

@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.

Activity

  1. telmaantunes commented on Jun 23, 2023

    @telmaantunes

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

  2. robmosca commented on Jun 23, 2023

    @robmosca
    Author

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

  3. robmosca commented on Jun 27, 2023

    @robmosca
    Author

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

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

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

    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;
  4. GhassenRjab commented on Jul 11, 2023

    @GhassenRjab
    Contributor

    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.

  5. robmosca commented on Jul 11, 2023

    @robmosca
    Author

    @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? 🤔

  6. GhassenRjab commented on Jul 11, 2023

    @GhassenRjab
    Contributor

    You're right! Nice catch!

    I'll open a PR with the suggested solution

  7. robmosca commented on Jul 11, 2023

    @robmosca
    Author

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

  8. GhassenRjab commented on Jul 31, 2023

    @GhassenRjab
    Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions