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

Update nuxt, @nuxt/vue-app, @nuxt/types dependencies #3247

Merged
merged 5 commits into from
Nov 8, 2023
Merged

Update nuxt, @nuxt/vue-app, @nuxt/types dependencies #3247

merged 5 commits into from
Nov 8, 2023

Conversation

khushi-kothari
Copy link
Contributor

@khushi-kothari khushi-kothari commented Oct 25, 2023

Fixes

Fixes #3242 by @obulat

Description

Updated nuxt dependency in frontend/package.json and updated the lock file, and commited the changes.

  • nuxt, @nuxt/vue-app, @nuxt/types: from 2.17.0 to ^2.17.2 [changes]
Screenshot 2023-10-25 at 7 43 50 PM

Testing Instructions

Checklist

  • My pull request has a descriptive title (not a vague title likeUpdate index.md).
  • My pull request targets the default branch of the repository (main) or a parent feature branch.
  • My commit messages follow best practices.
  • My code follows the established code style of the repository.
  • I added or updated tests for the changes I made (if applicable).
  • I added or updated documentation (if applicable).
  • I tried running the project locally and verified that there are no visible errors.
  • I ran the DAG documentation generator (if applicable).

Developer Certificate of Origin

Developer Certificate of Origin
Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

@khushi-kothari khushi-kothari requested a review from a team as a code owner October 25, 2023 12:51
@openverse-bot openverse-bot added the 🚦 status: awaiting triage Has not been triaged & therefore, not ready for work label Oct 25, 2023
@obulat obulat added 🟨 priority: medium Not blocking but should be addressed soon 💻 aspect: code Concerns the software code in the repository 🧰 goal: internal improvement Improvement that benefits maintainers, not users 🧱 stack: frontend Related to the Nuxt frontend and removed 🚦 status: awaiting triage Has not been triaged & therefore, not ready for work labels Oct 25, 2023
Copy link
Contributor

@obulat obulat left a comment

Choose a reason for hiding this comment

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

Great start!

You will need to run just node-install at the root of the repository. This will update the lock file, pnpm-lock.yaml. After you commit this file, the linting should be fixed.

@khushi-kothari
Copy link
Contributor Author

khushi-kothari commented Oct 25, 2023

just node-install

@obulat After running just node-install, pnpm-lock.yaml was not updated. Can you point out what is the issue here?

Screenshot 2023-10-25 at 9 34 19 PM

@obulat
Copy link
Contributor

obulat commented Oct 25, 2023

@obulat After running just node-install, pnpm-lock.yaml was not updated. Can you point out what is the issue here?

I really don't understand why your lock file is updated, @khushi-kothari 😕

When I ran it locally on your branch, I got the following:

 j node-install                                                                                                                                                   ✔  8s  18:06:56 
pnpm i
Scope: all 4 workspace projects
readPackage: typescript@^4.9.5 => typescript@^4.9.5 in dependencies of @openverse/eslint-plugin
frontend                                 |  WARN  deprecated [email protected]
frontend                                 |  WARN  deprecated [email protected]
frontend                                 |  WARN  deprecated [email protected]
frontend                                 |  WARN  deprecated [email protected]
frontend                                 |  WARN  deprecated [email protected]
frontend                                 |  WARN  deprecated [email protected]
frontend                                 |  WARN  deprecated [email protected]
frontend                                 |  WARN  deprecated [email protected]
frontend                                 |  WARN  deprecated [email protected]
frontend                                 |  WARN  deprecated [email protected]
frontend                                 |  WARN  deprecated [email protected]
frontend                                 |  WARN  deprecated [email protected]
frontend                                 |  WARN  deprecated [email protected]
frontend                                 |  WARN  deprecated [email protected]
frontend                                 |  WARN  deprecated [email protected]
frontend                                 |  WARN  deprecated [email protected]
frontend                                 |  WARN  deprecated [email protected]
frontend                                 |  WARN  deprecated [email protected]
frontend                                 |  WARN  deprecated @npmcli/[email protected]
frontend                                 |  WARN  deprecated @npmcli/[email protected]
frontend                                 |  WARN  deprecated [email protected]
frontend                                 |  WARN  deprecated @babel/[email protected]
Packages: +258 -121
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------------------------------------------
Progress: resolved 2478, reused 2469, downloaded 0, added 1, done
node_modules/.pnpm/[email protected]_463pqhi6eauw3mjqcyq4fuonrm/node_modules/nuxt: Running postinstall script, done in 578ms
. postinstall$ pnpm --filter ./packages/* run build
│ No projects matched the filters in ".../openverse"
└─ Done in 338ms
Done in 20.1s
just frontend/run i18n:en
pnpm run i18n:en

> @openverse/[email protected] i18n:en .../openverse/frontend
> pnpm i18n:get-translations --en-only


> @openverse/[email protected] i18n:get-translations .../openverse/frontend
> node src/locales/scripts/get-translations "--en-only"

Successfully saved English translation to en.json.
just frontend/run i18n:copy-test-locales
pnpm run i18n:copy-test-locales

> @openverse/[email protected] i18n:copy-test-locales .../openverse/frontend
> cp -r test/locales/* src/locales/

.../openverse  nuxt *66 !1  git status                                                                                                                                                   ✔  22s  18:07:56 
On branch nuxt
Your branch is up to date with 'khushi-kothari/nuxt'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
        modified:   pnpm-lock.yaml

no changes added to commit (use "git add" and/or "git commit -a")

(I redacted the openverse file paths)

I committed the lock file and pushed it to your branch.

Other options to try are running pnpm install --filter=frontend or running just install from the root (openverse/), but I'm not sure they would work better.

@obulat
Copy link
Contributor

obulat commented Oct 25, 2023

Hm, it looks like the vue packages need to be updated in the same PR after all, @khushi-kothari

@khushi-kothari
Copy link
Contributor Author

Ok @obulat that is the next task and am updating vue dependencies. Why are some of the CI tests failing in the current PR?

@khushi-kothari
Copy link
Contributor Author

khushi-kothari commented Oct 25, 2023

Thanks @obulat for adding lock file, I will look into what is causing this issue so next updates can be committed smoothly.

@obulat
Copy link
Contributor

obulat commented Oct 25, 2023

Ok @obulat that is the next task and am updating vue dependencies. Why are some of the CI tests failing in the current PR?

The CI tests are failing because the nuxt updates have updated one of the vue packages, and now there is a version mismatch and Nuxt cannot be built. So, the Vue dependencies should actually be updated in this PR instead of a separate PR.

@khushi-kothari
Copy link
Contributor Author

khushi-kothari commented Oct 25, 2023

@obulat Updated vue dependencies. Still 2 CI tests are failing.
Lock file updated this time and all changes added in the last commit that I added.
Screenshot 2023-10-25 at 10 59 08 PM

@obulat
Copy link
Contributor

obulat commented Oct 25, 2023

Weirdly, the lock file didn't fully match the updates in package.json, so the CI failed (you could see the messages about frozen lock file). I ran just install and committed the updated lock file. Let's see if the CI passes now :)

@khushi-kothari
Copy link
Contributor Author

Hey @obulat I have added all tasks as individual PRs. Do you need any more help from my side on this issue #3242?

@khushi-kothari khushi-kothari requested a review from obulat October 26, 2023 05:56
@obulat
Copy link
Contributor

obulat commented Oct 26, 2023

Hey @obulat I have added all tasks as individual PRs. Do you need any more help from my side on this issue #3242?

Hi, @khushi-kothari ! Thank you for creating the individual PRs. Could you please update them so that they only update the dependencies listed in the corresponding PR description?

For instance, Update focus-trap dependency should only update focus-trap, but if you look at the files changed, you can see that it updates all of these packages:

  • "@nuxtjs/i18n": "^7.3.1",
  • "@nuxtjs/sentry": "^7.5.0",
  • "@popperjs/core": "^2.11.8",
  • "@tailwindcss/typography": "^0.5.10",
  • "clipboard": "^2.0.11", "uuid": "^9.0.1",
  • "vue": "^2.7.15",
  • "@types/uuid": "^9.0.6",
  • "autoprefixer": "^10.4.16",
  • "tailwindcss": "^3.3.3",
  • "vue-demi": "^0.14.6",
  • "vue-server-renderer": "^2.7.15",
  • "vue-template-compiler": "^2.7.15",

@obulat obulat marked this pull request as draft October 26, 2023 06:52
@obulat
Copy link
Contributor

obulat commented Oct 26, 2023

@khushi-kothari, for this particular PR, we will need to either debug the way that Storybook is run in the CI now that Webpack added a patch for SSL, and Nuxt has removed its own patch (which is causing the CI to fail currently), or remove the smoke test from the CI. I'll ask other maintainers and write more here later.

@obulat obulat mentioned this pull request Oct 26, 2023
9 tasks
@khushi-kothari
Copy link
Contributor Author

Hey @obulat I think as it is linting problem always there is some issue while tracking whitespace changes. I locally have the following change for some reason it is not tracked while commiting.
Screenshot 2023-10-26 at 8 03 55 PM
local ss:
Screenshot 2023-10-26 at 7 27 50 PM

But while viewing it remotely, it is like this :
Screenshot 2023-10-26 at 7 29 06 PM

I followed following steps in this PR :

  1. Checked out the lock file from main to remove all unrelated changes.
    git checkout main -- pnpm-lock.yaml
  2. Ran just node-install to update the lock file.
  3. Committed the updated lock file to the branch and pushed the changes.

Yet, linting error is thrown.

@khushi-kothari khushi-kothari changed the title Updated nuxt, @nuxt/vue-app, @nuxt/types dependencies Update nuxt, @nuxt/vue-app, @nuxt/types dependencies Oct 26, 2023
@obulat
Copy link
Contributor

obulat commented Oct 26, 2023

Hey @obulat I think as it is linting problem always there is some issue while tracking whitespace changes. I locally have the following change for some reason it is not tracked while commiting.

I am not sure what's happening with the whitespace changes and why are they not tracked...
The lockfile changes, on the other hand, are not simply whitespace changes. In several PRs you can see that the Vue packages are still touched in the lock files, even though you reverted the Vue changes to the package.json file.
I could update the lock files for you if the process I described doesn't work for you locally:

I followed following steps in this PR :

Checked out the lock file from main to remove all unrelated changes.
git checkout main -- pnpm-lock.yaml
Ran just node-install to update the lock file.
Committed the updated lock file to the branch and pushed the changes.

@khushi-kothari
Copy link
Contributor Author

khushi-kothari commented Oct 26, 2023

Hey @obulat I think as it is linting problem always there is some issue while tracking whitespace changes. I locally have the following change for some reason it is not tracked while commiting.

I am not sure what's happening with the whitespace changes and why are they not tracked... The lockfile changes, on the other hand, are not simply whitespace changes. In several PRs you can see that the Vue packages are still touched in the lock files, even though you reverted the Vue changes to the package.json file. I could update the lock files for you if the process I described doesn't work for you locally:

I followed following steps in this PR :
Checked out the lock file from main to remove all unrelated changes.
git checkout main -- pnpm-lock.yaml
Ran just node-install to update the lock file.
Committed the updated lock file to the branch and pushed the changes.

Ya that will be very helpful @obulat as the process you described isn't working for me locally.

I hope my contributions were helpful. As my first experience to open sourcing, I could say I will continue doing it and will grow in the process. Thankyou for being the kindest mentor and guiding me through this issue very calmly.

@obulat
Copy link
Contributor

obulat commented Oct 26, 2023

Ya that will be very helpful @obulat as the process you described isn't working for me locally.

I hope my contributions were helpful. As my first experience to open sourcing, I could say I will continue doing it and will grow in the process. Thankyou for being the kindest mentor and guiding me through this issue very calmly.

Thank you for your contributions, @khushi-kothari, they are very helpful! Hope you can keep contributing with Openverse ❤️
It is often that dependency or local set up problems are more difficult than coding :)

@obulat obulat mentioned this pull request Nov 6, 2023
8 tasks
@obulat obulat marked this pull request as ready for review November 6, 2023 09:07
@obulat obulat requested a review from a team as a code owner November 6, 2023 09:07
@obulat obulat requested review from dhruvkb and removed request for AetherUnbound November 7, 2023 04:04
@@ -21,6 +21,7 @@
"prod:playwright": "pnpm i18n:copy-test-locales && pnpm prod",
"storybook": "nuxt storybook --port=54000",
"storybook:build": "nuxt storybook build",
"storybook:build-for-docs": "NODE_OPTIONS=--openssl-legacy-provider nuxt storybook build",
Copy link
Contributor

Choose a reason for hiding this comment

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

This script is run in GitHub CI runner (not Docker), and without openssl-legacy-provider it fails.

Signed-off-by: Olga Bulat <[email protected]>
@obulat obulat mentioned this pull request Nov 8, 2023
8 tasks
Copy link
Member

@dhruvkb dhruvkb left a comment

Choose a reason for hiding this comment

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

LGTM!

@dhruvkb dhruvkb merged commit 3e62256 into WordPress:main Nov 8, 2023
46 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💻 aspect: code Concerns the software code in the repository 🧰 goal: internal improvement Improvement that benefits maintainers, not users 🟨 priority: medium Not blocking but should be addressed soon 🧱 stack: frontend Related to the Nuxt frontend
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Update frontend dependencies
4 participants