Use npm to manage front end dependencies

[wil93]

[landscape-bot]

Repository health decreased by 0.05% when pulling c980443 on wil93:use_bower_plz into 4f11113 on cms-dev:master.

• 2 new problems were found (including 0 errors and 2 code smells).
• No problems were fixed.

[stefano-maggiolo]

What about the bower.json? Do we need to write some name and version there? Can't we just use it for the dependencies?

Having a custom name and version is kind of silly (I think we should use cms-server (path to bower.json) and cms' version, but I'd prefer avoid it entirely. What is it used for?

---

Reviewed 50 of 50 files at r1. Review status: all files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.

---

^{cms/server/bower.json, line 7 [r1] (raw file):} Shouldn't jquery be here?

---

Comments from the review on Reviewable.io

[wil93]

The name is mandatory, while the version can be deleted. I'm not sure about "cms-server", since server it's more of a "backend" term. I don't mind changing it, though :stuck_out_tongue:

P.S. something that should be added is probably ./prerequisites.py web (included in the install command) to run bower install.

---

Review status: all files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.

---

^{cms/server/bower.json, line 7 [r1] (raw file):} If I recall correctly (it's been a long time :stuck_out_tongue:) CMS includes jquery just because it's needed for jqplot and for bootstrap. I mean: CMS does not use jquery directly, thus it's enough to just depend on jqplot-bower and bootstrap (which will both pull jquery as dependency).

---

Comments from the review on Reviewable.io

[wil93]

FWIW, I think we should hang on this a bit, since now it seems that npm is to be preferred to bower (even for client-side dependencies). Furthermore, using bower implies having npm installed, so bower is just redundant at that point.

[wil93]

I updated this PR. The changes are these:

• not using bower anymore, just npm

• since npm is on ubuntu, I just added it on the apt-get install list in the documentation (however, I think it would be reasonable to start writing a CHANGELOG file so that it's easy to keep track of the new dependencies and whatnot)

• the package.json file (unlike bower.json) doesn't require to specify a name, so I just added the dependencies

• the bootstrap dependency didn't offer a 2.x version

  Valid install targets:
  4.0.0-alpha.2, 3.3.6, 3.3.5, 3.3.4, 3.3.2, 3.3.1, 3.3.0, 3.2.0, 3.1.1, 0.0.2, 0.0.1
  

  so, I used the github:name/repo#commit syntax in the package.json file (maybe we should start porting BS2 to BS3, though...)

[wil93]

I rebased this PR, @stefano-maggiolo want to take a look? :smile:

[codecov[bot]]

Codecov Report

Merging #459 into master will increase coverage by 0.19%. The diff coverage is n/a.

@@            Coverage Diff            @@
##           master    #459      +/-   ##
=========================================
+ Coverage      62%   62.2%   +0.19%     
=========================================
  Files         229     229              
  Lines       16521   16521              
=========================================
+ Hits        10244   10277      +33     
+ Misses       6277    6244      -33

Flag	Coverage Δ
#functionaltests	45.97% <ø> (+0.24%)	:arrow_up:
#unittests	43.28% <ø> (+0.02%)	:arrow_up:

Impacted Files	Coverage Δ
cms/db/filecacher.py	77.7% <0%> (ø)	:arrow_up:
cms/io/service.py	69.51% <0%> (ø)	:arrow_up:
cms/grading/Job.py	88.62% <0%> (ø)	:arrow_up:
cms/grading/Sandbox.py	68.59% <0%> (+0.36%)	:arrow_up:
cms/server/admin/handlers/base.py	68.47% <0%> (+0.67%)	:arrow_up:
cms/db/util.py	51.11% <0%> (+0.74%)	:arrow_up:
cms/db/fsobject.py	81.25% <0%> (+0.78%)	:arrow_up:
cms/io/triggeredservice.py	85.26% <0%> (+1.05%)	:arrow_up:
cms/service/EvaluationService.py	67.28% <0%> (+1.06%)	:arrow_up:
cms/service/ProxyService.py	58.51% <0%> (+1.06%)	:arrow_up:
... and 6 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 9da2d14...20dc163. Read the comment docs.

[stefano-maggiolo]

I think the main issue with this is forcing people that just want to run CMS to have npm installed just to download the JS libraries - ideally, npm or whatever package manager should only be used by developers, and the libraries bundled in the targz we produce for the release. This can be done with this approach, I think it just need to change the setup command to move the libraries from node_modules to their place in the tree (where now there are symlinks).

Anyway, I've been working a bit on a full modernization of the frontend, admittedly only somehow ready for RWS (CWS and AWS make it more difficult with the intertwined templates and JS, and templated JS...), but the idea would be to use yarn, webpack, babel, typescript (and maybe in the future react). I'll try to prepare a PR for that, so that we can discuss pros and cons.

I expect the main aspect to be assessed will be "can we really pull off my approach for AWS and CWS with our constrained manpower? If not, this simpler approach would at least solve the problem of having ancient libraries copypasted in our repo.

Maybe we could also consider a mixed approach (mine for RWS, yours for A|CWS).

[wil93]

I'm curious about the rws rewrite, please open a PR 😁

On Sun, 9 Dec 2018, 23:32 Stefano Maggiolo <[email protected] wrote:

I think the main issue with this is forcing people that just want to run CMS to have npm installed just to download the JS libraries - ideally, npm or whatever package manager should only be used by developers, and the libraries bundled in the targz we produce for the release. This can be done with this approach, I think it just need to change the setup command to move the libraries from node_modules to their place in the tree (where now there are symlinks).

Anyway, I've been working a bit on a full modernization of the frontend, admittedly only somehow ready for RWS (CWS and AWS make it more difficult with the intertwined templates and JS, and templated JS...), but the idea would be to use yarn, webpack, babel, typescript (and maybe in the future react). I'll try to prepare a PR for that, so that we can discuss pros and cons.

I expect the main aspect to be assessed will be "can we really pull off my approach for AWS and CWS with our constrained manpower? If not, this simpler approach would at least solve the problem of having ancient libraries copypasted in our repo.

Maybe we could also consider a mixed approach (mine for RWS, yours for A|CWS).

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/cms-dev/cms/pull/459#issuecomment-445578233, or mute the thread https://github.com/notifications/unsubscribe-auth/ABOc8bKfTyqeiYkOHzi0czXHdGfeMIPaks5u3Y9-gaJpZM4FmfQK .

[andreyv]

yarn, webpack, babel, typescript (and maybe in the future react)

Will this configuration be still boostrappable on Ubuntu using standard repository packages (e.g., without having to add PPAs or run curl | sh)?

[stefano-maggiolo]

Good point @andreyv - yarn requires a PPA. I think npm does too, and more in general I'm not aware of any JS package manager available in Ubuntu by default. Unless we find one, my preferences are in this order:

1. ask developers to install a ppa
2. keep outdated, minified js in our repo
3. ask users of cms to install a ppa

[wil93]

FWIW, ubuntu has npm in the standard repository, though it's an outdated version: https://packages.ubuntu.com/bionic/npm

I think it's reasonable to ask CMS contributors to use PPAs, and using ubuntu's npm to fetch dependencies for normal CMS users.

[stefano-maggiolo]

According to github 3.5.2 is from Dec '15 :) (also more worrysome is that it's not even the latest of the 3.5 series...)

If we choose to go with the approach of this PR, I would strongly support including the JS libraries in the targz, also because it's not much work.

[wil93]

Including the JS libraries in the targz is OK for me, but I would still like to have some way to fetch them automatically after cloning the repository (apart from "making a release" of it everytime, assuming there is a command for that).

The point is: our workflow is always revolving around the CMS git repository, never the targz. We usually just fetch the git repository (possibly apply some patch) and then install it, so it would be crucial to have some way (read: like I did here, in the prerequisites.py script) to fetch the JS dependencies.

On Mon, 10 Dec 2018 at 10:06, Stefano Maggiolo [email protected] wrote:

According to github 3.5.2 is from Dec '15 :) (also more worrysome is that it's not even the latest of the 3.5 series...)

If we choose to go with the approach of this PR, I would strongly support including the JS libraries in the targz, also because it's not much work.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/cms-dev/cms/pull/459#issuecomment-445741305, or mute the thread https://github.com/notifications/unsubscribe-auth/ABOc8a9EEjKwCrXTiVeV_u6dvV13adotks5u3iQWgaJpZM4FmfQK .

[andreyv]

As a user and an occasional committer I would appreciate if all dependencies were available in standard repositories, as it currently is. This makes it easier to set up and maintain machines for CMS. Ubuntu's npm is not at the latest version, but should work nonetheless.

However, I trust the developer choice on this matter :)

[stefano-maggiolo]

@wil93 Sure, the way I see it is that we will have a script to fetch the dependencies, that anybody can use to "start developing" or to make a release. But if you download the targz (which people do, apparently) you don't have to use it.

@andreyv I'm afraid that that's what we're trying to move away from ... having the source file of libraries in the repo is quite unelegant and slightly more likely to keep them out of date. Maybe we could start doing occasional and unofficial "dev releases" so that you can still live at master and not have to use npm?

[andreyv]

That just sounds bothersome and not particularly useful to developers, so I don't think that is good.

I meant that npm actually is in the standard repositories, so if the build process could use that (like this PR does), then it would be nice. But if developers opt for another solution, then it's fine too.

[stefano-maggiolo]

Here's my branch for comparison: https://github.com/stefano-maggiolo/cms/commits/yarnrws_for_wil93 - the infrastructure is just the first 4 commits, the others are me playing with typescript (turns out, to have some idiomatic code we would need to change a lot of RWS' javascript).

I will send an email to the mailing list to summarize pros and cons and we can discuss there how to proceed.

[stefano-maggiolo]

In case you aren't subscribed: https://groups.google.com/forum/#!topic/contestms-discuss/4_J2oDl6KpI