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

Downgrade pnpm so Dependabot can read the lockfile #1507

Merged

Conversation

fhenrich33
Copy link
Contributor

@fhenrich33 fhenrich33 commented Sep 4, 2024

Dependabot isn't issuing alerts with pnpm v9 lockfile format. See dependabot/dependabot-core#10534

Downgrading to latest pre v9 lockfile `pnpm release until v9 is supported: https://github.com/pnpm/pnpm/releases/tag/v8.15.9

Next steps:

  • Make a task to track revisiting v9 compatibility.
  • Run Check for updates after this PR is merged to check for Dependabot security alerts that we missed due to the aforementioned issue.

image

This also address the following Dependabot PRs:

#1506
#1502
#1488
#1485

@fhenrich33 fhenrich33 changed the title Downgrade pnpm so dependabot can read lockfile Downgrade pnpm so Dependabot can read lockfile Sep 4, 2024
@fhenrich33 fhenrich33 changed the title Downgrade pnpm so Dependabot can read lockfile Downgrade pnpm so Dependabot can read the lockfile Sep 4, 2024
Copy link

codecov bot commented Sep 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.60%. Comparing base (0493992) to head (d534537).
Report is 35 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1507      +/-   ##
==========================================
+ Coverage   85.59%   85.60%   +0.01%     
==========================================
  Files         232      232              
  Lines       21710    21708       -2     
  Branches     1916     1942      +26     
==========================================
+ Hits        18582    18583       +1     
+ Misses       3088     3085       -3     
  Partials       40       40              
Flag Coverage Δ
backend 99.07% <ø> (ø)
frontend 82.36% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@fhenrich33
Copy link
Contributor Author

Copy link
Member

@HaGuesto HaGuesto left a comment

Choose a reason for hiding this comment

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

Thanks @fhenrich33

@HaGuesto
Copy link
Member

HaGuesto commented Sep 7, 2024

Should we include this in the production deploy on Monday or not @fhenrich33 ?

@fhenrich33
Copy link
Contributor Author

Should we include this in the production deploy on Monday or not @fhenrich33 ?

Let's hold off and test it a bit more before we pull the trigger to be on the safe side.

@jamescrowley
Copy link
Contributor

jamescrowley commented Sep 11, 2024

@fhenrich33 looks good (and running locally fine for me), thanks! Just a note that if folks don't downgrade pnpm (using corepack use [email protected]), they will accidentally upgrade the lock file again, so we're going to have to keep an eye out for that.

I had hoped that

package-manager-strict-version=true

in .npmrc would at least limit fall out if pnpm 9 is installed (supported since v9.2 to enforce the packageManager version), but I couldn't get it working

@fhenrich33
Copy link
Contributor Author

@fhenrich33 looks good (and running locally fine for me), thanks! Just a note that if folks don't downgrade pnpm (using corepack use [email protected]), they will accidentally upgrade the lock file again, so we're going to have to keep an eye out for that.

I had hoped that

package-manager-strict-version=true

in .npmrc would at least limit fall out if pnpm 9 is installed (supported since v9.2 to enforce the packageManager version), but I couldn't get it working

Let's keep a close look at the following PRs to the frontend, and revisit the pnpm issue in the Dependabot tracker. I think it's the best move for now, IMO. @jamescrowley @HaGuesto @pylipp

@fhenrich33 fhenrich33 merged commit 1c7ce82 into master Sep 11, 2024
11 checks passed
@fhenrich33 fhenrich33 deleted the downgrade-pnpm-until-dependabot-fix-lockfile-v9-bug branch September 11, 2024 06:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants