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

New update #38

Merged
merged 13 commits into from Oct 19, 2022
Merged

New update #38

merged 13 commits into from Oct 19, 2022

Conversation

daniellemaxwell
Copy link
Contributor

@daniellemaxwell daniellemaxwell commented Oct 17, 2022

Scope of changes

Upgraded yarn to the latest release.

Fixes SC-10010

Type of change

  • new feature
  • bug fix
  • documentation
  • testing
  • technical debt
  • other (describe)

Acceptance criteria

Describe how reviewers can test this change to be sure that it works correctly. Add a checklist if possible.

Author checklist

  • I have manually tested the change and/or added automation in the form of unit tests or integration tests
  • I have updated the dependencies list
  • I have recompiled and included new protocol buffers to reflect changes I made
  • I have added new test fixtures as needed to support added tests
  • Check this box if a reviewer can merge this pull request after approval (leave it unchecked if you want to do it yourself)
  • I have moved the associated Shortcut story to "Ready for Review"

Reviewer(s) checklist

  • Any new user-facing content that has been added for this PR has been QA'ed to ensure correct grammar, spelling, and understandability.

@codecov
Copy link

codecov bot commented Oct 17, 2022

Codecov upload limit reached ⚠️

This org is currently on the free Basic Plan; which includes 250 free private repo uploads each rolling month. This limit has been reached and additional reports cannot be generated. For unlimited uploads, upgrade to our pro plan.

Do you have questions or need help? Connect with our sales team today at sales@codecov.io

Copy link
Contributor

@bbengfort bbengfort left a comment

Choose a reason for hiding this comment

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

The container build error was:

error Your lockfile needs to be updated, but yarn was run with --frozen-lockfile

I've tried re-running the container build in case this was a transient error; e.g. sometimes GitHub Actions fails to connect to npm. However, the build failed again. I tried to rebuild the yarn lock file locally but ran into the problem in the comment below.

Could you please talk me through your method of upgrading yarn to fix the security issues?

Additionally, I think we need to update the following dependencies:

  • nth-check to at least 2.0.1 or greater
  • object-path to at least 0.11.8 or later

It probably wouldn't hurt to update all dependencies. Can you use yarn upgrade to see what gets updated?

Comment on lines 1 to 7
nodeLinker: node-modules

plugins:
- path: .yarn/plugins/@yarnpkg/plugin-version.cjs
spec: "@yarnpkg/plugin-version"

yarnPath: .yarn/releases/yarn-3.2.4.cjs
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm getting the following error when I try to run yarn:

node:internal/modules/cjs/loader:959
  throw err;
  ^

Error: Cannot find module '/Users/benjamin/Workspace/go/src/github.com/rotationalio/ensign/web/ensign-landing-page/.yarn/releases/yarn-3.2.4.cjs'
    at Module._resolveFilename (node:internal/modules/cjs/loader:956:15)
    at Module._load (node:internal/modules/cjs/loader:804:27)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)
    at node:internal/main/run_main_module:17:47 {
  code: 'MODULE_NOT_FOUND',
  requireStack: []
}

What steps should I take to install yarn locally? Could we add these steps to the readme?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated yarn to the latest version by running yarn set version stable and I got that from this page: https://yarnpkg.com/getting-started/install

I ran yarn upgrade, but received a message stating that I should run yarn install, so I did that. I just looked at the documentation again and ran yarn upgrade-interactive, after installing another tool, and these are now the packages listed as needing to be updated. I can do that now and then push again.

@testing-library/user-event ----------------- ◉ ^13.5.0 ------ ◯ ^14.4.3 ------
react-router-dom ---------------------------- ◉ ^6.4.1 ------- ◯ ^6.4.2 -------
react-router -------------------------------- ◉ ^6.4.1 ------- ◯ ^6.4.2 -------
web-vitals ---------------------------------- ◉ ^2.1.4 ------- ◯ ^3.0.3 -------

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here's a link where yarn upgrade-interactive is mentioned along with the plugin that I had to install for it to work. https://yarnpkg.com/cli/upgrade-interactive

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I upgraded the dependencies mentioned in my last message. nth-check and object-path were also upgraded. I realized that I missed your message about installing yarn. I ran corepack enable because I have node installed and running yarn set version stable should install the latest version of yarn.

@bbengfort
Copy link
Contributor

@daniellemaxwell ok, so if I remove the .yarnrc.yml file and then run yarn set version stable then I get a new .yarnrc.yml file and the .yarn directory is populated. Unfortunately, the next step is figuring out the plugins :-).

For now, I've deleted the .yarnrc.yml file and updated yarn.lock in an attempt to fix the tests. If that works, I propose that we add .yarnrc.yml to the .gitignore so that local changes don't effect the container build process. If you think it's important to set the yarn version then I can look into updating the container build process.

@daniellemaxwell
Copy link
Contributor Author

@bbengfort After I submitted the configuration module PR without an issue, I had a feeling that the .yarnrc.yml file was causing an issue. The plugins aren't necessary as they were added to run yarn version check and yarn upgrade-interactive while I was trying to figure things out. In this case, setting the version isn't important, so I think that all should be good now.

@bbengfort
Copy link
Contributor

@daniellemaxwell ok cool; I just wanted to try one more thing real quick - I did some research and found this: https://yarnpkg.com/getting-started/qa#which-files-should-be-gitignored -- if we go with the "no zero-installs" approach, then we can commit the yarn release and plugins; if the container builds work I'm happy to go that route.

@bbengfort
Copy link
Contributor

Ok, I've put us back to yarn v1.22.19 since I couldn't figure out the yarn 3 container builds. If you run:

$ yarn set version 1.22.19

You should have the correct yarn version to generate yarn.lock files that the container build process understand.

Copy link
Contributor

@bbengfort bbengfort left a comment

Choose a reason for hiding this comment

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

Once merged we'll take a look at the security alerts and make sure they're resolved!

@bbengfort bbengfort merged commit 7f58eae into main Oct 19, 2022
@bbengfort bbengfort deleted the new-update branch October 19, 2022 21:32
@bbengfort bbengfort mentioned this pull request Oct 19, 2022
13 tasks
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.

None yet

2 participants