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

Development: Add peer dependency overrides to avoid NPM resolve errors #5227

Merged
merged 1 commit into from
Jun 11, 2022

Conversation

bassner
Copy link
Member

@bassner bassner commented Jun 9, 2022

Checklist

General

Motivation and Context

Some of you might have noticed that our CI Pipelines building the client on GitHub actions started to fail in all PRs randomly.

This is due to the release of Node.js v16.15.1 which GH Actions gradually started to use on their runners. Compared to 16.15.0, this included a bump of NPM from 8.5.5 to 8.11.0:
Screenshot 2022-06-09 at 18 53 31

Our dependency tree for the client has conflicts due to outdated or even ancient dependencies that are hard to replace. They depend on older versions of other dependencies, causing NPM to fail when installing without --force:

  • @swimlane/ngx-data-table: Last update on 2021-09-14 - Depends on rxjs@6.6.3 while we have rxjs@7.5.5
  • showdown-katex: Last update on 2020-03-12 - Depends on showdown@1.9.1 while we have showdown@2.1.0.

Previously, NPM was fine with letting us install the broken dependencies once with --force, generating a package-lock.json, and live on from then on. This apparently changed with NPM 8.6.0: Now all builds are failing, even if the package-lock.json already exists, when not using the --force flag. I've always considered this to be a feature, but apparently it was a "bug" that just got fixed, defeating one of the biggest purposes of lock files (imho) and randomly breaking thousands of production CI pipelines in a minor Node.JS patch 👍 🚀 ✨

Description

As it wouldn't be wise to always use --force in light of future dependency updates we do, this PR adds overrides to the conflicting packages to force them onto the versions of dependencies that we use. These version have worked up until now, but we now have to take special care and spoon-feed NPM that we really want that. 🙃

@bassner bassner self-assigned this Jun 9, 2022
@bassner bassner requested a review from a team as a code owner June 9, 2022 17:12
@bassner bassner temporarily deployed to artemistest5 June 9, 2022 17:20 Inactive
Copy link
Contributor

@Strohgelaender Strohgelaender left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approval since the Client-GitHub-Actions seem to work on this PR.

Copy link
Contributor

@Dominik-Weinzierl Dominik-Weinzierl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me 👍 i guess you can mark this as ready for merge

Copy link
Contributor

@ge65cer ge65cer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for applying this fix! 🙂

@krusche krusche added this to the 5.8.5 milestone Jun 11, 2022
@krusche krusche merged commit 48d009b into develop Jun 11, 2022
@krusche krusche deleted the chore/add-dependency-overrides branch June 11, 2022 22:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants