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

chore: upgrade to yarn 2 (#5279) #11006

Closed
wants to merge 24 commits into from

Conversation

nsinghal12
Copy link

This PR contains ui code for yarn upgrade to yarn 2

Note on DCO:

If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the Details link next to the DCO action for instructions on how to resolve this.

Checklist:

  • Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
  • The title of the PR states what changed and the related issues number (used for the release note).
  • I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue.
  • I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
  • Does this PR require documentation updates?
  • I've updated documentation as required by this PR.
  • Optional. My organization is added to USERS.md.
  • I have signed off all my commits as required by DCO
  • I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
  • My build is green (troubleshooting builds).

@crenshaw-dev crenshaw-dev changed the title PR for issue#5279 chore: upgrade to yarn 2 (#5279) Oct 20, 2022
@crenshaw-dev
Copy link
Member

Looks like we'll have to disable the Snyk integration to get this to work. https://github.com/snyk/cli/issues/1518

@crenshaw-dev
Copy link
Member

Not sure what this means:

#39 [argocd-ui 6/6] RUN HOST_ARCH=amd64 NODE_ENV='production' NODE_ONLINE_ENV='online' NODE_OPTIONS=--max_old_space_size=8192 yarn build
#39 1.258 Usage Error: Couldn't find the node_modules state file - running an install might help (findPackageLocation)

emirot and others added 11 commits October 20, 2022 10:00
* test: add unit test for server version

Signed-off-by: emirot <emirot.nolan@gmail.com>

* test: add unit test for server version

Signed-off-by: emirot <emirot.nolan@gmail.com>

* test: add unit test for server version

Signed-off-by: emirot <emirot.nolan@gmail.com>

* tests: update cmd dependencies

Signed-off-by: emirot <emirot.nolan@gmail.com>

Signed-off-by: emirot <emirot.nolan@gmail.com>
Signed-off-by: Niti Singhal <gupta.sweet.niti@gmail.com>
With Kiali v1.57.1 an additional status condition was added:
```
    - lastTransitionTime: '2022-10-14T11:56:24Z'
      message: ''
      reason: ''
      status: 'False'
      type: Failure
```

Based on the discussion in kiali/kiali#5560 this should not lead to a degraded health state.

This will no longer return Degraded as a catch-all and use the `type` and `status` fields of the condition to determine the CR health.

Signed-off-by: Allex Veldman <allexveldman+github@gmail.com>

Signed-off-by: Allex Veldman <allexveldman+github@gmail.com>
Signed-off-by: Niti Singhal <gupta.sweet.niti@gmail.com>
* issue-10592 Wrap errors with message

Signed-off-by: Apoorva Mahabaleshwara <apoorvamahabaleshwara@Apoorvas-MBP.attlocal.net>
Signed-off-by: Apoorva Mahabaleshwara <apoorvambhat@gmail.com>

* issue-10592 remove unwanted error  wrappers

Signed-off-by: Apoorva Mahabaleshwara <apoorvamahabaleshwara@Apoorvas-MBP.attlocal.net>
Signed-off-by: Apoorva Mahabaleshwara <apoorvambhat@gmail.com>

* chore: fix  error wrapper messages

Signed-off-by: Apoorva Mahabaleshwara <apoorvambhat@gmail.com>

Signed-off-by: Apoorva Mahabaleshwara <apoorvamahabaleshwara@Apoorvas-MBP.attlocal.net>
Signed-off-by: Apoorva Mahabaleshwara <apoorvambhat@gmail.com>
Co-authored-by: Apoorva Mahabaleshwara <apoorvamahabaleshwara@Apoorvas-MBP.attlocal.net>
Signed-off-by: Niti Singhal <gupta.sweet.niti@gmail.com>
…d replicas to ReplicaSet (argoproj#10613)

* Misc UI Improvements: sort by created at in resource list view, add message to AnalysisRun and replicas to Replicaset

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>

* Address PR comments

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>

* No underscore needed in created_at. Add space between icon and message in health details for non-controlled resources

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>

* Guard section

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>
Co-authored-by: Remington Breeze <remington@breeze.software>
Signed-off-by: Niti Singhal <gupta.sweet.niti@gmail.com>
Signed-off-by: Niti Singhal <gupta.sweet.niti@gmail.com>
Signed-off-by: Niti Singhal <gupta.sweet.niti@gmail.com>
Signed-off-by: Niti Singhal <gupta.sweet.niti@gmail.com>
Signed-off-by: Niti Singhal <gupta.sweet.niti@gmail.com>
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
* chore: fix e2e

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

* more config

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

* global

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Signed-off-by: Niti Singhal <gupta.sweet.niti@gmail.com>
Signed-off-by: Niti Singhal <gupta.sweet.niti@gmail.com>
Signed-off-by: Niti <gupta.sweet.niti@gmail.com>
Signed-off-by: Niti <gupta.sweet.niti@gmail.com>
nsinghal12 and others added 8 commits October 21, 2022 09:10
Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Signed-off-by: Niti <gupta.sweet.niti@gmail.com>
Signed-off-by: Niti Singhal <gupta.sweet.niti@gmail.com>
Signed-off-by: Niti Singhal <gupta.sweet.niti@gmail.com>
Signed-off-by: Niti Singhal <gupta.sweet.niti@gmail.com>
Signed-off-by: Niti Singhal <gupta.sweet.niti@gmail.com>
Signed-off-by: Niti Singhal <gupta.sweet.niti@gmail.com>
@nsinghal12
Copy link
Author

Hi @crenshaw-dev I tried multiple things today to try fix the uuid/v1 error:

  1. Adding uuid/v1 as a dev dependency
  2. Adding uuid as a packageExtension in .yarnrc.yml
  3. Try allowing yarn to update lock file
  4. Adding yarn set version berry and yarn install --immutable to step on installing dependencies

But the code is unable to resolve the package.

As I see uuid/v1 is not a package in NPM registry. It is the uuid package from which v1.js is being imported. I believe we will need to modify the argo-ui source code to switch from CommonJS require to ES import statements. (refer https://github.com/argoproj/argo-ui/blob/master/src/components/form-field/form-field.tsx#L7)

I haven't yet found guidance in my Google search on how to resolve CJS level-2 dependencies. Any advice on where to look, or how to resolve without modifying argo-ui ?

@crenshaw-dev crenshaw-dev added hacktoberfest hacktoberfest-accepted Accepted Hacktoberfest contribution labels Oct 27, 2022
@crenshaw-dev
Copy link
Member

@keithchong I think this PR is very close to being done. Can you take a look at the build failures? I don't know yarn well enough to have any ideas.

@keithchong
Copy link
Contributor

Hi @crenshaw-dev, I haven't actually migrated to yarn 2 before. Did you follow the steps here? https://yarnpkg.com/getting-started/migration#step-by-step

@blakepettersson
Copy link
Member

I believe we will need to modify the argo-ui source code to switch from CommonJS require to ES import statements. (refer https://github.com/argoproj/argo-ui/blob/master/src/components/form-field/form-field.tsx#L7)

It was a while since I've worked with yarn, but IIRC yarn 2 requires all dependencies to be explicitly be stated in package.json and from what I can see it is not.

It could be a good opportunity to modify argo-ui to ES import, and to perhaps upgrade uuid to a newer version?

@blakepettersson
Copy link
Member

@nsinghal12 @keithchong @crenshaw-dev I think the required changes are in argoproj/argo-ui#367.

blakepettersson added a commit to blakepettersson/argo-cd that referenced this pull request May 19, 2023
Related to argoproj#11006, all dependencies that are used within the app needs
to be expressed in package.json, in case we ever want to use yarn berry
or pnpm.

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>
crenshaw-dev added a commit that referenced this pull request May 28, 2023
* chore: set explicit package.json deps

Related to #11006, all dependencies that are used within the app needs
to be expressed in package.json, in case we ever want to use yarn berry
or pnpm.

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* fix: downgrade history dependency

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

---------

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>
Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
@blakepettersson
Copy link
Member

blakepettersson commented Jun 4, 2023

@nsinghal12 if you have time you can try to merge master and see if it builds now.

@nsinghal12
Copy link
Author

@nsinghal12 if you have time you can try to merge master and see if it builds now.

Hello @blakepettersson I merged master and updated this branch but looks like build is failing at install node dependencies in integration tests.

@blakepettersson
Copy link
Member

@nsinghal12 I think your yaml.lock file is malformed (it seems to contain Git conflict diffs). I'd suggest removing it and then regenerating the lockfile.

@nsinghal12
Copy link
Author

@nsinghal12 I think your yaml.lock file is malformed (it seems to contain Git conflict diffs). I'd suggest removing it and then regenerating the lockfile.

I think you meant yarn.lock file. I deleted and regenrated the file again and pushed it. But looks like same issue.

@crenshaw-dev
Copy link
Member

@nsinghal12 are you sure you added the new yarn.lock and pushed it? GitHub is still showing it as deleted.

@crenshaw-dev
Copy link
Member

Oh, I see. Make sure you run yarn install from inside the ui directory.

@blakepettersson
Copy link
Member

I think you meant yarn.lock file.

@nsinghal12 yes, that was a silly typo 😄

As @crenshaw-dev said, the yarn.lock should be regenerated by running yarn install in the ui/ dir.

yyzxw pushed a commit to yyzxw/argo-cd that referenced this pull request Aug 9, 2023
* chore: set explicit package.json deps

Related to argoproj#11006, all dependencies that are used within the app needs
to be expressed in package.json, in case we ever want to use yarn berry
or pnpm.

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* fix: downgrade history dependency

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

---------

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>
Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
@crenshaw-dev
Copy link
Member

Closing for now as this needs more work.

tesla59 pushed a commit to tesla59/argo-cd that referenced this pull request Dec 16, 2023
* chore: set explicit package.json deps

Related to argoproj#11006, all dependencies that are used within the app needs
to be expressed in package.json, in case we ever want to use yarn berry
or pnpm.

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* fix: downgrade history dependency

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

---------

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>
Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest hacktoberfest-accepted Accepted Hacktoberfest contribution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants