@alexbeletsky opened this Pull Request on March 4th 2016 Contributor

We are using Piwik together with browserify and would like to be able to use require('./piwik.js') to get the module. Unfortunately, it's not possible since Piwik is not assigned to a global namespace with transformations like deglobalify.

This change would explicitly assign Piwik to window, that fixes the issue above.

  • [ ] Todo after merge: Build minified piwik.js
@tsteur commented on March 6th 2016 Owner

That's awesome :+1: I noticed our Tracker tests fail because of JSLint see https://travis-ci.org/piwik/piwik/jobs/113611238#L780. Likely we need to add Piwik here: https://github.com/piwik/piwik/blob/2.16.0/js/piwik.js#L959

Also I noticed there are some more references to Piwik eg https://github.com/piwik/piwik/blob/2.16.0/js/piwik.js#L6490-L6493 . Should we update these as well? I hope it won't break anything re exposing Piwik as an AMD module?

Note: After merge we need to build minified piwik.js

@alexbeletsky commented on March 7th 2016 Contributor

@tsteur Thanks :+1: I've added Piwik to config section.. Regarding https://github.com/piwik/piwik/blob/2.16.0/js/piwik.js#L6490-L6493 , my assumption it is fine there, since local Piwik instance is referred.

@tsteur commented on March 7th 2016 Owner

Sweet, thank you for this! :+1:

@mattab commented on March 17th 2016 Owner

Hi @alexbeletsky not sure, but maybe it would be possible to add an automated test to check the notatation require('./piwik.js') works? maybe around these lines in our JS test suite: https://github.com/piwik/piwik/blob/master/tests/javascript/index.php#L548

@alexbeletsky commented on March 22nd 2016 Contributor

@mattab it's hard to say to me, since I'm not that aware of that framework you use for testing.

But, I have to add following: if you use browserify and require('./piwik.js') it won't work properly. What I did is the possibility to use browserify with deglobalify transformation, so in code it looks like,

const Piwik = require('./piwik.js', 'Piwik');

Sure, it would be good idea to make Piwik as real UMD module, it requires bit more work. But could be good next logical step.

This Pull Request was closed on March 7th 2016
Powered by GitHub Issue Mirror