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.

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

Upgrade to Yarn v2 or switch back to npm #1074

Closed
artlowel opened this issue Mar 24, 2021 · 4 comments · Fixed by #3173
Closed

Upgrade to Yarn v2 or switch back to npm #1074

artlowel opened this issue Mar 24, 2021 · 4 comments · Fixed by #3173

Comments

@artlowel
Copy link
Member

artlowel commented Mar 24, 2021

Yarn 2 was released last year, and yarn 1 will only get security upgrades anymore. I can't seem to find the date when those will end.

Yarn 2 isn't backwards compatible, but the migration process doesn't seem too daunting. More info here.

However we could also consider dropping yarn and going back to npm. The main reasons we switched back in 2017 have been addressed: npm now has built-in support for proper lockfiles, and its performance is supposedly on par with yarn 1. (however the yarn 2 docs promise major performance increases on top of that, even with the option to eliminate the install step altogether). Still, switching to npm is likely more effort than upgrading to yarn 2.

@artlowel artlowel added new feature needs triage New issue needs triage and/or scheduling labels Mar 24, 2021
@tdonohue
Copy link
Member

@artlowel : Thanks for logging this. My inclination is to resolve this in either 7.1 or perhaps 7.0 final (after Beta 5 / Testathon) if we deem it higher priority.

In terms of yarn vs npm, I don't have strong opinions right now (haven't dug enough). It seems like the main pro of npm is we wouldn't need to install anything besides Node (since NPM comes with it). But, the main pro of yarn is that it seems to find ways to continue to be a faster build than npm.

For now, I'm going to move this into 7.1, as it looks like there has been no formal announcement of an End of Life date for Yarn v1 (or Yarn Classic), so there's no requirement for an upgrade soon. However, we definitely should monitor this and see if that message changes.

@tdonohue tdonohue added Estimate TBD and removed needs triage New issue needs triage and/or scheduling labels Mar 24, 2021
@ghost
Copy link

ghost commented Nov 18, 2021

@artlowel I am curious, why do you think this is the case? "switching to npm is likely more effort than upgrading to yarn 2."

Shouldn't npm just work. If npm does not work, I feel there is something wrong. Find and replace of yarn to npm seems rather trivial.

I feel the use of yarn should be a choice of the developer.

@artlowel
Copy link
Member Author

I was thinking about things like resolutions that don't work in exactly the same way between the two, or changes to the CI build which has some code to deal with the yarn cache, updating the docs, …

However if you say it's not that hard to support both npm and yarn, and let the user decide, I'm all for it.

@alanorth
Copy link
Contributor

alanorth commented Jul 8, 2024

The argument to switch back to npm is for simplifying the project. I agree with the comment above stating that any issues with npm that we don't have with yarn point to some underlying problem that should be solved.

As an experiment I tried to switch to npm (v10.7.0 / Node.js v20.14.0) in a clean DSpace 8.0 working directory:

$ rm -rf node_modules
$ sed -i 's/yarn/npm/g' package.json
$ npm install

The first problem was:

$ npm install 
npm error code ERESOLVE
npm error ERESOLVE unable to resolve dependency tree
npm error
npm error While resolving: [email protected]
npm error Found: @angular/[email protected]
npm error node_modules/@angular/animations
npm error   @angular/animations@"^17.3.11" from the root project
npm error
npm error Could not resolve dependency:
npm error peer @angular/animations@">=13.0.0 <14" from @kolkov/[email protected]
npm error node_modules/@kolkov/ngx-gallery
npm error   @kolkov/ngx-gallery@"^2.0.1" from the root project
npm error
npm error Fix the upstream dependency conflict, or retry
npm error this command with --force or --legacy-peer-deps
npm error to accept an incorrect (and potentially broken) dependency resolution.
npm error
npm error
npm error For a full report see:
npm error /home/aorth/.npm/_logs/2024-07-08T09_52_53_993Z-eresolve-report.txt

npm error A complete log of this run can be found in: /home/aorth/.npm/_logs/2024-07-08T09_52_53_993Z-debug-0.log

So yarn is quietly working around this legitimate dependency issue in @kolkov/[email protected].

If I force the installation it continues, but hits another error with a yarn-specific link syntax for local dependencies:

$ npm install --legacy-peer-deps
npm error code EUNSUPPORTEDPROTOCOL
npm error Unsupported URL Type "link:": link:./lint/dist/src/rules/html

npm error A complete log of this run can be found in: /home/aorth/.npm/_logs/2024-07-08T10_08_29_847Z-debug-0.log

It seems we should be able to use file: dependencies in npm.

After fixing a few other minor issues like this the application builds and runs fine. I will submit a pull request.

@alanorth alanorth mentioned this issue Jul 8, 2024
11 tasks
@github-project-automation github-project-automation bot moved this from 📋 To Do to ✅ Done in DSpace 9.0 Release Sep 6, 2024
@tdonohue tdonohue added this to the 9.0 milestone Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

3 participants