@diosmosis opened this Pull Request on February 9th 2019 Member

Still a WIP.

NOTE: This will require updates to premium plugins.

TODO:

  • [ ] get tests to pass
  • [ ] update individual libraries and get tests to pass
  • [ ] BC for bower_components folder
  • [ ] run tests of premium plugins and get them to pass

Fixes #12961
Fixes #12067

@Findus23 commented on February 10th 2019 Member

@diosmosis Thanks for starting this work
See also #12067 for an old example package.json

It also seems like this doesn't really solve #12961 as jQuery <3 is still used.

I'd also pin jquery.dotdotdot to this exact version, so that no one unintentionally updates it to the latest version that isn't GPL compatible

@diosmosis commented on February 10th 2019 Member

Currently just getting tests to pass w/ npm libs, some of whom required an update (eg, materializecss < 1.0 includes some node dependencies which we can't include in the repo, & some repos don't have the versions we use available). Then I'll upgrade libs and get tests to pass again.

Thanks for the warning about dotdotdot!

@tsteur commented on February 10th 2019 Member

@diosmosis fyi changing the bower_components folder will be a BC break. They are used in various of our extra plugins and premium features. I assume also other plugins use it. Ideally we would keep the same directory structure and change it in Matomo 4 if possible.

Also just to confirm we still commit the files to the repository right? Just making sure as it really does make installing and updating easier as you wouldn't need node.js to install it and makes sure everyone is using the same version. I would even go so far and commit the vendor directory eventually. Personally, I rarely run composer install after a branch change and things might not always be tested properly because of this. Just makes things easier and more consistent.

@diosmosis commented on February 10th 2019 Member

fyi changing the bower_components folder will be a BC break. They are used in various of our extra plugins and premium features. I assume also other plugins use it. Ideally we would keep the same directory structure and change it in Matomo 4 if possible.

Unfortunately, npm doesn't support specifying a different node_modules folder (though yarn does, so npm might in the future). I think for this we'd have to either change premium plugins to use the correct folder based on the matomo version, or give up until Matomo 4.

Also just to confirm we still commit the files to the repository right?

Yes, so far no complications. If a library decides to include dependencies then it could get annoying, but I don't think it would be a real blocker.

@tsteur commented on February 10th 2019 Member

Unfortunately, npm doesn't support specifying a different node_modules folder (though yarn does, so npm might in the future). I think for this we'd have to either change premium plugins to use the correct folder based on the matomo version, or give up until Matomo 4.

Was thinking a symlink but reckon that might not be working on all systems. I reckon we could otherwise - if needed - have a little to do the npm update and manually move node_modules to bower_components as we commit it anyway? may be bit too annoying though.

It's likely not only premium features that use it but all plugins. Ideally we check the plugins on the marketplace to see if any of them use it (I think we used to have a search engine for that but don'T remember URL or if it still works). Most plugins won't be on the marketplace though...

Actually just thought of a better solution: We can rewrite the path in the asset manager and it'll be no problem :)

@diosmosis commented on February 10th 2019 Member

I reckon we could otherwise - if needed - have a little to do the npm update and manually move node_modules to bower_components as we commit it anyway? may be bit too annoying though.

This could be put into a command and a test added to ReleaseCheckListTest (since we have the list of files we use in the getJavaScriptFiles hook).

Actually just thought of a better solution: We can rewrite the path in the asset manager and it'll be no problem :)

I see you beat me to it :) I was thinking of creating symlinks when merging assets, but using a map of bower_components => node_modules links works better. Will do this.

Powered by GitHub Issue Mirror