Skip to content
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

[RequireJS] Compatibility issue with xml2json library #1726

Closed
davidgarry opened this issue Dec 29, 2016 · 2 comments
Closed

[RequireJS] Compatibility issue with xml2json library #1726

davidgarry opened this issue Dec 29, 2016 · 2 comments
Assignees
Milestone

Comments

@davidgarry
Copy link
Contributor

Hello,

I am currently implementing dashjs 2.4 instead of dashjs 2.3, but I have an error to load dashjs library. As all my website use requirejs, I load dashjs with the requirejs "shim" option (as it doesn't support AMD).

All was ok with 2.3, but with 2.4, the xml2json external library has been updated and it now use AMD. So, X2JS variable is not defined on the window object :

(function (root, factory) {
     if (typeof define === "function" && define.amd) {
         define([], factory);
     } else if (typeof exports === "object") {
         module.exports = factory();
     } else {
         root.X2JS = factory();
     }
 }(this, function () {

Commit : 5aa8931

Would you be able to standardize that, by using amd for all modules, or by removing amd support on xml2json library ?

Regards,
David

@sopinon
Copy link
Contributor

sopinon commented Jan 3, 2017

+1
had the same issue.. Would be great if this could be fixed in next release.

@davemevans
Copy link
Contributor

This was added in order to keep as close to the original xml2json as possible. Since we have forked the library anyway, that code can be removed and an export statement used as previously.

Does davemevans@0590591 solve your problem?

@davemevans davemevans added this to the v2.4.1 milestone Jan 11, 2017
LloydW93 pushed a commit that referenced this issue Jan 11, 2017
Fix #1726 - remove explicit amd support from xml2json fork
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants