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

replace bower (and libs/) with npm #12067

Open
Findus23 opened this Issue Sep 16, 2017 · 10 comments

Comments

Projects
None yet
3 participants
@Findus23
Member

Findus23 commented Sep 16, 2017

(I think this issue is really low priority but quite important to do one day so I want to start a discussing)

In my opinion keeping all third-party-libraries checked into the main repository is a bad idea. It makes it hard to see which libraries are outdated and have bugs, which may be already fixed upstream, which LICENSEs belong to which files and which files aren't used any more (There are placeholder background images from the first visitor popup lying around in /libs/jquery/images/)

For PHP libraries Piwik is (mostly) using composer to solve this issue, so I'd propose to do the same with javascript (and css) libraries and npm.

While bower also helps, it doesn't support as many libraries and even the bower team recommends switching to npm.

Since version 5 npm also supports a lockfile similar to composer which would make sure that the CI and the built package would have the same version as tested while developing.

Another advantage would be that we would have a well formated list of used libraries and their licenses (compared to LEGALNOTICE), which may help avoiding incompatible licenses (e.g. version 3 of jQuery.dotdotdot)

see also #6469, #12021

@Findus23

This comment has been minimized.

Show comment
Hide comment
@Findus23

Findus23 Sep 16, 2017

Member

An example package.json:

{
  "name": "piwik",
  "version": "1.0.0",
  "description": "the leading free/libre analytics platform",
  "main": "piwik.js",
  "directories": {
    "test": "tests"
  },
  "repository": {
    "type": "git",
    "url": "git+https://github.com/piwik/piwik.git"
  },
  "author": "Piwik.org <hello@piwik.org>",
  "license": "GPL-3.0+",
  "bugs": {
    "url": "https://github.com/piwik/piwik/issues"
  },
  "dependencies": {
    "jquery-ui": "1.10.4",
    "jquery": "~2.2.0",
    "angular": "~1.6.0",
    "angular-sanitize": "~1.6.0",
    "angular-animate": "~1.6.0",
    "angular-cookies": "~1.6.0",
    "angular-mocks": "~1.6.0",
    "ng-dialog": "~1.3.0",
    "html5shiv": "~3.7.0",
    "mousetrap": "~1.4.0",
    "sprintf-js": "~1.0.0",
    "jscrollpane": "~2.0.0",
    "jquery-mousewheel": "~3.1.12",
    "jquery-placeholder": "~2.0.7",
    "jquery.dotdotdot": "~1.7.2",
    "jquery.scrollto": "~1.4.13",
    "chroma-js": "~0.6.0",
    "visibilityjs": "~1.2.1"
  },
  "keywords": [
    "piwik",
    "web",
    "analytics"
  ],
  "homepage": "http://piwik.org"
}
➜  ~/public_html/piwik git:(npm) ✗ npm outdated 
Package             Current  Wanted  Latest  Location
chroma-js             0.6.3   0.6.3   1.3.4  piwik
jquery                2.2.4   2.2.4   3.2.1  piwik
jquery-placeholder    2.0.7   2.0.7   2.3.1  piwik
jquery-ui            1.10.4  1.10.4  1.12.1  piwik
jquery.dotdotdot      1.7.4   1.7.4   3.0.1  piwik
jquery.scrollto      1.4.14  1.4.14   2.1.2  piwik
mousetrap             1.4.6   1.4.6   1.6.1  piwik
ng-dialog             1.3.0   1.3.0   1.4.0  piwik
sprintf-js            1.0.3   1.0.3   1.1.1  piwik
Member

Findus23 commented Sep 16, 2017

An example package.json:

{
  "name": "piwik",
  "version": "1.0.0",
  "description": "the leading free/libre analytics platform",
  "main": "piwik.js",
  "directories": {
    "test": "tests"
  },
  "repository": {
    "type": "git",
    "url": "git+https://github.com/piwik/piwik.git"
  },
  "author": "Piwik.org <hello@piwik.org>",
  "license": "GPL-3.0+",
  "bugs": {
    "url": "https://github.com/piwik/piwik/issues"
  },
  "dependencies": {
    "jquery-ui": "1.10.4",
    "jquery": "~2.2.0",
    "angular": "~1.6.0",
    "angular-sanitize": "~1.6.0",
    "angular-animate": "~1.6.0",
    "angular-cookies": "~1.6.0",
    "angular-mocks": "~1.6.0",
    "ng-dialog": "~1.3.0",
    "html5shiv": "~3.7.0",
    "mousetrap": "~1.4.0",
    "sprintf-js": "~1.0.0",
    "jscrollpane": "~2.0.0",
    "jquery-mousewheel": "~3.1.12",
    "jquery-placeholder": "~2.0.7",
    "jquery.dotdotdot": "~1.7.2",
    "jquery.scrollto": "~1.4.13",
    "chroma-js": "~0.6.0",
    "visibilityjs": "~1.2.1"
  },
  "keywords": [
    "piwik",
    "web",
    "analytics"
  ],
  "homepage": "http://piwik.org"
}
➜  ~/public_html/piwik git:(npm) ✗ npm outdated 
Package             Current  Wanted  Latest  Location
chroma-js             0.6.3   0.6.3   1.3.4  piwik
jquery                2.2.4   2.2.4   3.2.1  piwik
jquery-placeholder    2.0.7   2.0.7   2.3.1  piwik
jquery-ui            1.10.4  1.10.4  1.12.1  piwik
jquery.dotdotdot      1.7.4   1.7.4   3.0.1  piwik
jquery.scrollto      1.4.14  1.4.14   2.1.2  piwik
mousetrap             1.4.6   1.4.6   1.6.1  piwik
ng-dialog             1.3.0   1.3.0   1.4.0  piwik
sprintf-js            1.0.3   1.0.3   1.1.1  piwik
@mattab

This comment has been minimized.

Show comment
Hide comment
@mattab

mattab Sep 18, 2017

Member

In my opinion keeping all third-party-libraries checked into the main repository is a bad idea.

FYI the reason we put all JS files in the repository is so that people can easily use Piwik from GIT: https://piwik.org/faq/how-to-install/faq_18271/
Sure, we could change it and ask people to run the NPM command, but then it would add some new requirements for them, plus it would break any existing Piwik deployed from Git (which there are hundreds).

It makes it hard to see which libraries are outdated and have bugs, which may be already fixed upstream

There are services that read the bower.json and composer files and report any outdated library, so far we have tried this tool: https://gemnasium.com/piwik/piwik (currently doesn't load but may be temporary issue).
There are quite a few other such services; https://alternativeto.net/software/requires-io/ - I would rather use the services to solve this particular challenge of being notified when there are new version of libraries.

Member

mattab commented Sep 18, 2017

In my opinion keeping all third-party-libraries checked into the main repository is a bad idea.

FYI the reason we put all JS files in the repository is so that people can easily use Piwik from GIT: https://piwik.org/faq/how-to-install/faq_18271/
Sure, we could change it and ask people to run the NPM command, but then it would add some new requirements for them, plus it would break any existing Piwik deployed from Git (which there are hundreds).

It makes it hard to see which libraries are outdated and have bugs, which may be already fixed upstream

There are services that read the bower.json and composer files and report any outdated library, so far we have tried this tool: https://gemnasium.com/piwik/piwik (currently doesn't load but may be temporary issue).
There are quite a few other such services; https://alternativeto.net/software/requires-io/ - I would rather use the services to solve this particular challenge of being notified when there are new version of libraries.

@sgiehl

This comment has been minimized.

Show comment
Hide comment
@sgiehl

sgiehl Sep 27, 2017

Member

If it helps to maintain and easily update the libs, I wouldn't mind using npm or similar to maintain all (non php) dependencies. We still could commit those libraries to git, so it can be directly used.

Member

sgiehl commented Sep 27, 2017

If it helps to maintain and easily update the libs, I wouldn't mind using npm or similar to maintain all (non php) dependencies. We still could commit those libraries to git, so it can be directly used.

@Findus23

This comment has been minimized.

Show comment
Hide comment
@Findus23

Findus23 Sep 28, 2017

Member

I agree that this would be a breaking change and probably should be made with Piwik 4.
I am also definitly for using a service like gemnasium. But they would work even better with npm as they are just a nice visualisation of npm outdated (or bower list and composer outdated at the moment)

But I am still quite against checking the resulting node_modules folder into the main git repository. I think it will result in many large commits changing thousands of lines every time angular (or another library) has a bugfix release.

I'd really recommend to do it the same way as PHP dependecies in composer (run npm install on tests and build script, do updates similar to #11780, still remove unwanted directories in the build script

Member

Findus23 commented Sep 28, 2017

I agree that this would be a breaking change and probably should be made with Piwik 4.
I am also definitly for using a service like gemnasium. But they would work even better with npm as they are just a nice visualisation of npm outdated (or bower list and composer outdated at the moment)

But I am still quite against checking the resulting node_modules folder into the main git repository. I think it will result in many large commits changing thousands of lines every time angular (or another library) has a bugfix release.

I'd really recommend to do it the same way as PHP dependecies in composer (run npm install on tests and build script, do updates similar to #11780, still remove unwanted directories in the build script

@Findus23

This comment has been minimized.

Show comment
Hide comment
@Findus23

Findus23 Sep 28, 2017

Member

Another idea would be adding the libraries in /tests/libs as dev-dependecies in the package.json.
This way they could be managed the same way, but can be excluded from the final build with npm install --only=production

UPDATE: I feel like this folder needs a cleanup equally desperately: The java-folder holds 7-9 year old java files without a license or context. And screenshot-testing has an empty node_modules folder

Member

Findus23 commented Sep 28, 2017

Another idea would be adding the libraries in /tests/libs as dev-dependecies in the package.json.
This way they could be managed the same way, but can be excluded from the final build with npm install --only=production

UPDATE: I feel like this folder needs a cleanup equally desperately: The java-folder holds 7-9 year old java files without a license or context. And screenshot-testing has an empty node_modules folder

@mattab

This comment has been minimized.

Show comment
Hide comment
@mattab

mattab Sep 28, 2017

Member

npm is famous for sending a crazy amount of http requests over the network (not sure if this recently improved but it was terrible), so I wouldn't trust this platform, when it comes to quickly release a security release, or forcing users to use npm on their server to use Piwik from git. Composer is a bit different because it's leaner and PHP based, which our users already support for sure.

+1 definitely to switch to npm vs bower (if you conclude it's better), but would still check in the files in git

Member

mattab commented Sep 28, 2017

npm is famous for sending a crazy amount of http requests over the network (not sure if this recently improved but it was terrible), so I wouldn't trust this platform, when it comes to quickly release a security release, or forcing users to use npm on their server to use Piwik from git. Composer is a bit different because it's leaner and PHP based, which our users already support for sure.

+1 definitely to switch to npm vs bower (if you conclude it's better), but would still check in the files in git

@mattab

This comment has been minimized.

Show comment
Hide comment
@mattab

mattab Sep 28, 2017

Member

UPDATE: I feel like this folder needs a cleanup equally desperately:

Yes, you're most welcome to remove anything un-used there 👍 (we iterated a lot in tests over the last 4 years and definitely a few things could be cleaned up)

Member

mattab commented Sep 28, 2017

UPDATE: I feel like this folder needs a cleanup equally desperately:

Yes, you're most welcome to remove anything un-used there 👍 (we iterated a lot in tests over the last 4 years and definitely a few things could be cleaned up)

@Findus23

This comment has been minimized.

Show comment
Hide comment
@Findus23

Findus23 Sep 28, 2017

Member

While npm is still not the fastest package manager it definitly improved. And one factor is that we are mostly using flat dependecies (e.g. jquery) instead of fetching (server-side) node-modules which hundreds of dependencies each.
When disabling every cache, the package.json from above takes 2.3 seconds to install.

Member

Findus23 commented Sep 28, 2017

While npm is still not the fastest package manager it definitly improved. And one factor is that we are mostly using flat dependecies (e.g. jquery) instead of fetching (server-side) node-modules which hundreds of dependencies each.
When disabling every cache, the package.json from above takes 2.3 seconds to install.

@Findus23

This comment has been minimized.

Show comment
Hide comment
@Findus23

Findus23 Oct 12, 2017

Member

There are services that read the bower.json and composer files and report any outdated library, so far we have tried this tool: https://gemnasium.com/piwik/piwik (currently doesn't load but may be temporary issue).

Seems like github now also offers this functionality out of the box (even though it seems a bit like work in progress)

https://github.com/piwik/piwik/network/dependencies

(More Github News)

Member

Findus23 commented Oct 12, 2017

There are services that read the bower.json and composer files and report any outdated library, so far we have tried this tool: https://gemnasium.com/piwik/piwik (currently doesn't load but may be temporary issue).

Seems like github now also offers this functionality out of the box (even though it seems a bit like work in progress)

https://github.com/piwik/piwik/network/dependencies

(More Github News)

@mattab

This comment has been minimized.

Show comment
Hide comment
@mattab

mattab Jan 12, 2018

Member

(slightly out of topic, this article about NPM is a really interesting read: I’m harvesting credit card numbers and passwords from your site. Here’s how.)

Member

mattab commented Jan 12, 2018

(slightly out of topic, this article about NPM is a really interesting read: I’m harvesting credit card numbers and passwords from your site. Here’s how.)

@mattab mattab removed the Lower priority label Sep 13, 2018

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