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.

Sign up for GitHub

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

Jump to bottom

Just use npm instead of yarn #394

Closed
lukasjuhrich opened this issue Jul 26, 2020 · 5 comments
Closed

Just use npm instead of yarn #394

lukasjuhrich opened this issue Jul 26, 2020 · 5 comments
Labels
architectural Architectural changes: things that usually don't have an impact for the average pycroft end user. prio:high

Comments

@lukasjuhrich
Copy link
Collaborator

lukasjuhrich commented Jul 26, 2020 •
edited

The switch to yarn was a good decision in the context of replacing Bower. However, currently it does not seem to be necessary to add another layer of abstraction onto npm. If I'm not mistaken, all the packages we need (in particular, bootstrap-table) are packaged for npm as well. Also, a bug in yarn that does not seem to be fixed at any time completely breaks our CI.

To reiterate: yarn…

  • adds complexity which is not needed anymore
  • has been breaking our CI for a few years
@lukasjuhrich
Copy link
Collaborator Author

CC @sebschrader

@lukasjuhrich lukasjuhrich added architectural Architectural changes: things that usually don't have an impact for the average pycroft end user. prio:high labels Jul 26, 2020
@lukasjuhrich
Copy link
Collaborator Author

Marking as high-priority because having a working CI again would be immeasurably helpful.

@MarauderXtreme
Copy link
Member

To speed things up in the CI the package-lock.json should be put in the tree.
With npm ci there is no package resolving but only the installation of defined packages that can also be sped up by effectively using caches.

@lukasjuhrich
Copy link
Collaborator Author

Also a good read: https://engineering.mixmax.com/blog/to-yarn-and-back-again-npm/

lukasjuhrich added a commit that referenced this issue Jul 27, 2020
@lukasjuhrich
Switch from yarn to npm
9ee6054 
Refs #394
lukasjuhrich added a commit that referenced this issue Jul 27, 2020
@lukasjuhrich
Replace yarn usages with NPM usages
b2adb3e 
Refs #394
lukasjuhrich added a commit that referenced this issue Jul 27, 2020
@lukasjuhrich
Switch from yarn to npm
b5ca7a6 
Refs #394
lukasjuhrich added a commit that referenced this issue Jul 27, 2020
@lukasjuhrich
Replace yarn usages with NPM usages
566a7b9 
Refs #394
lukasjuhrich added a commit that referenced this issue Jul 27, 2020
@lukasjuhrich
Rename 20_yarn entry hook
386e755 
Refs #394
lukasjuhrich added a commit that referenced this issue Jul 27, 2020
@lukasjuhrich
Update webpack due to parser bug
df92448 
see webpack/webpack#8509.
Refs #394
@lukasjuhrich
Copy link
Collaborator Author

lukasjuhrich commented Jul 27, 2020 •
edited

A quick change to npm appears to run into issues with docker-compose -f docker-compose.test.yml run --rm test-app webpack --mode production:
See https://travis-ci.org/github/agdsn/pycroft/builds/712164310#L1538

For instance:

ERROR in /opt/pycroft/app/node_modules/watchpack/lib/chokidar.js

Module build failed (from /opt/pycroft/app/node_modules/babel-loader/lib/index.js):

SyntaxError: /opt/pycroft/app/node_modules/watchpack/lib/chokidar.js: 'return' outside of function (4:1)

…which appears to be caused by watchpak using a top-level return, which is not ES6-Module-compatible. However, I have no Idea how that could have been introduced just by not using yarn anymore.
Any help is greatly appreciated.

lukasjuhrich added a commit that referenced this issue Jul 27, 2020
@lukasjuhrich
Switch from yarn to npm
439869c 
Fixes #394

This entails

- Removing `yarn` as an angine
- Adding the `package-lock.json`
- Replacing yarn usages with NPM usages
- making `webpack` known to npm as a build script
- Renaming `20_yarn` entry hook → `20_npm`
- Updating webpack (which is technically not doing anything due to
  minor compat matching)
lukasjuhrich added a commit that referenced this issue Jul 27, 2020
@lukasjuhrich
Use npm ci instead of npm install
5d28a9a 
Refs #394

Because we have the `package-lock.json` checked in, `npm ci` is
favorable.  If one desires to add or update a package, the intended
workflow is to change the `package.json`, execute `npm install
locally, and check in the appropriate changes in the lock file.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
architectural Architectural changes: things that usually don't have an impact for the average pycroft end user. prio:high
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@MarauderXtreme @lukasjuhrich