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

Nuxt 3 migration #411

Closed
6 tasks done
obulat opened this issue Feb 16, 2023 · 36 comments · Fixed by #4257
Closed
6 tasks done

Nuxt 3 migration #411

obulat opened this issue Feb 16, 2023 · 36 comments · Fixed by #4257
Assignees
Labels
🧭 project: thread An issue used to track a project and its progress

Comments

@obulat
Copy link
Contributor

obulat commented Feb 16, 2023

Start Date ETA Project Lead Actual Ship Date
2023-06-05 TBD @obulat TBD

Description

Vue 2 is reaching end-of-life at the end of 20231. We need to update to Nuxt 3 to use Vue 3.

Node 16, which is required to build the frontend app (Nuxt 2 applications can theoretically run on Node >16) reaches end-of-life on September 11th, 2023.

Documents

Issues

Prior Art

Several members of the team have attempted the migration previously but without success. This is one of such attempts: WordPress/openverse-frontend#313
The Nuxt team has said that working on simplifying the migration is one of their priorities for 2023, so hopefully, it will be easier this time.

Footnotes

  1. Vue 2 LTS: https://v2.vuejs.org/lts/

@obulat obulat added the 🧭 project: thread An issue used to track a project and its progress label Feb 16, 2023
@obulat obulat mentioned this issue Feb 18, 2023
2 tasks
@obulat obulat mentioned this issue Mar 20, 2023
7 tasks
@sarayourfriend
Copy link
Collaborator

@obulat Do you know when we'll prioritise this project? Node 16 reaches EOL in September 2023, 4 months from now. It would probably be nice to give ourselves a little breathing room for planning and any major changes that need to be implemented to accommodate the Nuxt 3 transition. As I understand it that is what is keeping us on Node 16, right?

@obulat
Copy link
Contributor Author

obulat commented May 17, 2023

I haven't looked at this migration from the point of view of the Node version, @sarayourfriend . I think we should prioritize it after the current Core UI improvements project is done.

I'm going to be adding some more notes about the things we would need for migration, as pre-planning steps.

Modules

Module name Change needed Links
"@nuxt/typescript-build" Delete as TypeScript is natively supported by Nuxt 3
"@nuxtjs/composition-api/module" Delete - natively supported by Nuxt 3
"@nuxtjs/style-resources" This is for scss variables that we don't use anymore - create an issue to remove it
"@nuxtjs/svg" Not supported, need to replace with nuxtjs/svg-sprite done
"@nuxtjs/svg-sprite" Update to current that supports Nuxt 3
"@nuxtjs/eslint-module" Update to current that supports Nuxt 3 https://github.com/nuxt-modules/eslint
"@pinia/nuxt" Update to current version that supports Nuxt 3
"portal-vue/nuxt" Delete - natively supported by Vue 3 / Nuxt 3
"@nuxtjs/proxy" ?
"@nuxtjs/redirect-module" Define Nitro routeRules in nuxt.config https://nitro.unjs.io/config/#routerules
"@nuxtjs/sentry" ?
"cookie-universal-nuxt" Delete - natively supported by Nuxt 3, useCookie
"vue-plausible" Update to current version that supports Nuxt 3 Note: enabling outbound tracking breaks links with target blank
"@nuxtjs/sitemap" Replace with nuxt-simple-sitemap https://github.com/harlan-zw/nuxt-simple-sitemap
  • Remove @nuxtjs/style-resources

Nuxt i18n changes

The setup changed a little bit
Functional components need to be replaced: i18n has been replaced with i18n-t, and key prop is replaced with keypath

  • Create a wrapper component for the current i18n functional component with the same API as the new component (i18n-t with keypath prop) to minimize the number of changes during Nuxt migration.

  • useContext.app.localePath -> useLocalePath

  • nuxtI18nHead -> useLocaleHead

  • useContext.i18n -> useI18n

Route

route does not need .value anymore

useContext

useContext is deprecated - useNuxtApp for app properties like $sentry, $ua, $plausible. For i18n-related props, see i18n, for $cookies - useCookie composable.

onBeforeRouteEnter

We don't have access to Pinia stores anymore, so we need to rethink how we update the search term we should move the search term update to the middleware in

beforeRouteEnter(to, from, next) {
/**
* The order in which beforeRouteEnter and middleware are called is indeterminate.
* We need to make sure that query `q` exists before checking if it matches
* the store searchTerm.
*/
const searchStore = useSearchStore()
if (to.query.q && to.query.q !== searchStore.searchTerm) {
searchStore.setSearchTerm(to.query.q)
}
next()

  • Replace route guards with route middleware

@sarayourfriend
Copy link
Collaborator

Regarding the modules that can be deleted: that should only happen during the Nuxt 3 migration, not before, right?

@obulat
Copy link
Contributor Author

obulat commented May 27, 2023

Regarding the modules that can be deleted: that should only happen during the Nuxt 3 migration, not before, right?

For most modules the answer is yes, we should only delete them during the Nuxt 3 migration.
There are two exceptions. We've already removed nuxtjs/svg, and we can remove @nuxtjs/style-resources because we are not using it anyways.

@sarayourfriend
Copy link
Collaborator

Do you think it's worth us writing an implementation for this? Just doing the count on how much longer before Node 16 EOL, I think three weeks of planning for this (writing + review) would be on the potentially speedier side. That would give us about 2.5 months for implementation, which would hopefully allow us a small amount of wiggle room to account for unexpected difficulties.

Just thinking of this in light of how difficult and roundabout the changes have been in the past when we've tried and considering the distinct possible approaches (Bridge vs a "re-write" with Nuxt 3 vs something like Vite SSR)... getting ahead of this seems wise to me. If you agree then I'll add it to the community meeting agenda for this week.

@zackkrida
Copy link
Member

zackkrida commented May 29, 2023

Do you think it's worth us writing an implementation for this?

This makes sense to me, especially considering @obulat has already done a lot of the planning research in this comment. A formal plan should definitely help us set expectations in the context of the September 11th EOL for Node 16 (Which I've also updated the project description to mention)

@sarayourfriend
Copy link
Collaborator

👍 Added to the agenda for this week.

@obulat
Copy link
Contributor Author

obulat commented May 30, 2023

I agree with writing the implementation plan for this soon.

Bridge vs a "re-write" with Nuxt 3 vs something like Vite SSR

I think Nuxt i18n does not support bridge, so we can only go with a "re-write" with Nuxt 3.

@obulat obulat moved this from Not Started to In Kickoff in Openverse Project Tracker Jun 5, 2023
@openverse-bot
Copy link
Collaborator

Hi @obulat, this project has not received an update comment in 14 days. Please leave an update comment as soon as you can. See the documentation on project updates for more information.

@productdevbook
Copy link

hi @obulat, check v-plausible nuxt 3 support https://github.com/productdevbookcom/v-plausible

@zackkrida zackkrida moved this from In Kickoff to On Hold in Openverse Project Tracker Jul 5, 2023
@zackkrida
Copy link
Member

This project will be placed On Hold while TypeScript updates are made to the codebase. Those changes will impact the contents of the implementation plan which has also been drafted.

@zackkrida
Copy link
Member

@sarayourfriend thanks for the reminder about Dhruv's local issue, we definitely want to watch out for that.

To clarify, we'll do this using two separate services behind the same domain, and route to one or the other based on the path of the request, at the load balancer level. The goal here is to see whether when we cut the site down to the absolute minimum number of routes, the "best case scenario" for Nuxt 3, if there is still a performance issue.

Yes, I should have been more specific. After the chat I was thinking as a very first step we could simply see how Nuxt 3 behaves with only English first. It would be quite simple to deploy a single service, load test it, and then if the problem is present we'll know we're really in trouble before running any tests of multiple services split at the load-balancer. I'm fine with skipping that though if it doesn't seem worthwhile.

@sarayourfriend
Copy link
Collaborator

sarayourfriend commented Apr 27, 2024

Oh true, and actually we could do that and direct increasing percentages of production traffic to it as well, if we wanted, with a small bit of configuration on the load balancer side. That way we can see how it operates under some production load, without taking out all of production if it fails. If it goes well with English, we can add localizations and see if we can find the "limit"

@obulat
Copy link
Contributor Author

obulat commented Apr 29, 2024

Thank you for the writeup, @zackkrida!

I've updated the Nuxt 3 migration branch: rebased it onto main, removed unnecessary changes such as converting components to script setup syntax or using ofetch instead of axios, and will push it to the repository shortly.

There are several code changes that work in Nuxt 2 that I think we can do before starting the testing:

@sarayourfriend
Copy link
Collaborator

sarayourfriend commented Apr 30, 2024

The nuxt preview service is ready to go in this PR in the infrastructure repository: https://github.com/WordPress/openverse-infrastructure/pull/874

#4227 complements that PR by adding a script to build and push to the nuxt-preview tag. We don't need to merge that branch to main, instead, @obulat please cherry-pick the commit onto the Nuxt 3 migration branch and then close the PR and delete the branch 🙏

main is deployed to https://nuxt-preview.openverse.org, so the service is ready for Nuxt 3 code once you've got the branch ready and used the push-to-nuxt-preview recipe, @obulat.

@zackkrida
Copy link
Member

140 locales have been removed from page generation and the locale chooser as a general improvement to Openverse, but relevant to this project, in this PR: #4224.

@zackkrida zackkrida moved this from ⏸ On Hold to 🚧 In Progress in Openverse Project Tracker Apr 30, 2024
@obulat
Copy link
Contributor Author

obulat commented May 13, 2024

2024-05-13

Most of the preparation work for Nuxt 3 is done (except for #4295, which needs one more review).
Some of the problems in the Nuxt 3 PR (#4357) (fetching of the single result items, error handling) were fixed, and the tests are passing on it. The deployment steps for nuxt-preview are listed in the PR description. I tried deploying today but deployment failed, probably to the nginx problem that @zackkrida mentioned previously.

Once we fix the deployment, we can start testing memory consumption.

@zackkrida
Copy link
Member

I have successfully deployed to https://nuxt-preview.openverse.org/

@obulat
Copy link
Contributor Author

obulat commented May 24, 2024

Now that the preparation work is done, we can start memory testing.

Memory consumption testing

Steps

  • wordpress/openverse-infrastructure#917 Remove caching from https://nuxt-preview.openverse.org (I created the draft rule in the Cloudflare UI)
  • Create a script for load-testing the frontend #4391 Write a k6 script similar to the API load testing script. It should be possible to request static content pages only, and search pages only, as well as requesting both. This will allow us to test whether static pages have a different memory load than the search pages. The script should be able to request locales other than English.
  • Run the load testing script testing memory consumptions for the following scenarios.
  • English-only site: static pages -> search pages -> static and search pages
  • Site with the 4 testing locales, with the same sequence.

We will write the script next week, and start load-testing after it is merged.

@sarayourfriend
Copy link
Collaborator

sarayourfriend commented May 28, 2024

(I created the draft rule in the Cloudflare UI)

@obulat please open a PR in the infrastructure repository for Cloudflare rule changes. There are now drifted configurations in the Terraform because of the manual changes to the cache settings in Cloudflare, which will overwrite those manual changes if applied.

@obulat
Copy link
Contributor Author

obulat commented Jun 5, 2024

2024-06-05 Update

We ran the load testing against staging (Nuxt 2) and nuxt-preview (Nuxt 3) today.

On all scenario options (static pages with only English, search pages with only English, static pages with the 4 testing locales, static and search pages with 4 testing locales), Nuxt 2 consumes almost twice as much memory as Nuxt 3.

Nuxt 2

Nuxt 2 CPU and memory utilization during load test

Nuxt 3

Nuxt 3 CPU and memory utilization during load test

However, during the all scenarios test, the app had to restart because the CPU usage went over 100%. This is because the CPU in staging is not enough for Openverse, the production uses CPU that is 8 times larger than the staging one, and production can use 10-66% of CPU.

With the caveat that the Nuxt 3 app does not have Sentry enabled, it seems that the memory usage is not a problem.

Next steps

  • Add Sentry to make the Nuxt 3 branch have exactly the same functionality as the current production app.
  • Build Nuxt 3 app with all available locales
  • Increase the CPU of nuxt-preview and run the load testing with all locales. We don't necessarily need to request pages from all locales, I think having the i18n routes should be enough for testing.
  • Route some portion of the production traffic to the Nuxt 3 app to test production loads.

@zackkrida
Copy link
Member

We don't necessarily need to request pages from all locales, I think having the i18n routes should be enough for testing.

Definitely @obulat. The vue-router problem seems to arise simply by having these pages in memory.

@sarayourfriend
Copy link
Collaborator

sarayourfriend commented Jun 5, 2024

Really exciting, Olga!

Route some portion of the production traffic to the Nuxt 3 app to test production loads.

As part of this, I'll want to look at reducing caching on the frontend as well, otherwise we could end up with users getting slightly different experiences. I don't know what the precise differences would be, but it's better if the traffic is consistent. I'll look and see how many frontend requests that aren't static resources are actually cached. We need closer evaluation of this as part of other work anyway...

Don't let this block you though, if you get to deploying Nuxt 3 in production with split traffic (use weighted listeners with cookie-based stickiness enabled), don't want for me if I haven't had a chance to look at this yet!

@sarayourfriend
Copy link
Collaborator

@obulat @zackkrida can either of y'all clarify what is left for the branch? I wanted to try help getting it mergeable, but I'm afraid of the large force pushes still going on would just squash any attempt I made to help.

What can I do to help get it across the line? I see that storybook seems not to be working. Is that something I could help with? Is there another bettey way to help? I'll do some code review today as well, but it's hard to know what exactly to review 🙂

@zackkrida
Copy link
Member

My own knowledge of the latest work on the branch is limited too @sarayourfriend. Last I heard was that @obulat was integrating Sentry and fixing a bug.

We'll have to wait until Olga is back from AFK next week for an update.

@sarayourfriend
Copy link
Collaborator

@obulat can you share an update here on how we can help get this across the finish line?

@obulat
Copy link
Contributor Author

obulat commented Jun 25, 2024

I've been keeping the branch updated by rebasing onto the main.

Currently, the missing parts that I think can be fixed in separate follow-up PRs are:

Items that were removed from nuxt.config. I'm not sure if we need to add them (in a separate follow-up PR, urgent in this PR) or keep them removed:

  • nuxt-template-overrides.
  • HTML header "http-equiv": "Content-Security-Policy", content: "upgrade-insecure-requests"
  • built file names mapping in nuxt.config.js
  • config.devtool = ctx.isClient ? "source-map" : "inline-source-map"
  • transpile: [({ isLegacy }) => (isLegacy ? "axios" : undefined)],
  • prometheus

I can run the Playwright tests on my Mac. I think most of the tests have been converted to run correctly (except for Storybook tests).

@sarayourfriend
Copy link
Collaborator

I've opened https://github.com/WordPress/openverse-infrastructure/pull/965 to address the only outstanding Cloudflare/proxy/cache configuration issue I was able to identify in my review of the Nuxt 3 branch today. Otherwise, I think our existing Nginx and other proxy configurations are correct for Nuxt 3 to roll out.

@openverse-bot
Copy link
Collaborator

Hi @obulat, this project has not received an update comment in 14 days. Please leave an update comment as soon as you can. See the documentation on project updates for more information.

@openverse-bot openverse-bot moved this to 🏗 In Progress in Openverse Backlog Jul 30, 2024
@github-project-automation github-project-automation bot moved this from 🚧 In Progress to 🚢 Shipped in Openverse Project Tracker Jul 30, 2024
@zackkrida zackkrida moved this from 🏗 In Progress to ✅ Done in Openverse Backlog Jul 30, 2024
@obulat
Copy link
Contributor Author

obulat commented Jul 30, 2024

The Nuxt 3 PR was merged and deployed to production 🚀

@sarayourfriend
Copy link
Collaborator

@obulat The issues you added the blocked labels to for the code freeze, can they be unblocked now?

@zackkrida zackkrida moved this from 🚢 Shipped to ✅ Success in Openverse Project Tracker Aug 1, 2024
@obulat
Copy link
Contributor Author

obulat commented Aug 2, 2024

@obulat The issues you added the blocked labels to for the code freeze, can they be unblocked now?

Done 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧭 project: thread An issue used to track a project and its progress
Projects
Archived in project
Status: ✅ Success
Development

Successfully merging a pull request may close this issue.

5 participants