WordPress / openverse

Openverse is a search engine for openly-licensed media. This monorepo includes all application code.
https://openverse.org
MIT License
214 stars 175 forks source link

Nuxt 3 migration #411

Open obulat opened 1 year ago

obulat commented 1 year ago
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 2023[^1]. 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: https://github.com/WordPress/openverse-frontend/pull/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.

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

sarayourfriend commented 1 year ago

@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 commented 1 year ago

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

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

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 https://github.com/WordPress/openverse/blob/20949acc9063d8dde629cad1c9941b5ad2ca4050/frontend/src/pages/search.vue#L55-L65

sarayourfriend commented 1 year ago

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

obulat commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

:+1: Added to the agenda for this week.

obulat commented 1 year ago

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.

openverse-bot commented 1 year ago

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 commented 1 year ago

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

zackkrida commented 11 months ago

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.

obulat commented 4 months ago

2024-02-07 Update

The Migration to Nuxt 3 PR was deployed to staging. This coincided with an attempt of scraping, so we discovered that this version has a memory leak. This is why we decided to postpone this project until the delayed Nuxt EOL on June 30, 2024. We plan to check the progress in April 2024.

We have contacted the maintainers of nuxt, @nuxtjs/i18n, unhead and vue-router over GitHub and in the Nuxt Discord server. Some of the memory improvements were added in the latest versions of the i18n module.

zackkrida commented 2 months ago

2024-04-24 Update

Some Openverse maintainers (@obulat, @zackkrida, @dhruvkb, and @sarayourfriend) met today to discuss next steps for this project.

The primary current blocker to this project is a performance degradation for sites with many pages in version 4 of vue-router. That problem is tracked in an issue in the vue-router project here. Nuxt 3 depends on this version of Vue-router.

Encouragingly, this issue was moved to the "In Progress" column of the Vue Router roadmap on April 17th of this year.

While we await progress on that issue, we identified some next steps for our migration. The plan is to get our Nuxt 3 branch up to date, deployed (separately from our production site), and run some load tests to reproduce our memory leak. After reproducing the leak we plan to test the efficacy of a few different strategies to mitigate it as a stop-gap measure until the upstream performance degradation is fixed:

I think we should create a "Nuxt 3 Performance Testing" milestone for this work and initial issues to track:

After completing that initial milestone, we can determine if we need to try and of the additional "build-splitting" strategies to reduce the number of pages in memory.

What do you think @WordPress/openverse-frontend? @obulat could you create the milestone and the issue to update the branch? @sarayourfriend would you mind creating issues and sketching out a strategy for the infrastructure side?

Finally, I think it is worth noting we should hold off on beginning #3592 until this milestone is rectified.

sarayourfriend commented 2 months ago

Thanks for the notes and update, Zack. I'll get the infrastructure issues up tomorrow for the new staging nuxt 3.

conduct a load test against an English-only version of the site

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. If not, then we have a strong indication that it is a problem with the number of routes, and we'll know we can move on to the more complex final step. If there is still a performance issue, however, we'll need to address that first, before spending the time on the complexity of divvying up the application between the static routes and non-static routes. In other words, if previous steps haven't worked, it is only at this point that we'll be able to confidently say it is only an issue with vue router, and not some additional problem with either Nuxt 3 generally or perhaps with the way we've architected the frontend.

I also wanted to bring up what Dhruv said about needing to change OS-level memory settings on his PC to run the development server. I'd like to know whether this is a common situation for Nuxt 3, and if it continues to be an issue for Nuxt 3. It makes no sense for that to be a requirement to run a web app's development server. Node is a relative memory hungry runtime but... that is really an absurd issue to have. Hopefully it's just the vue router thing, but that's what gives me the most concern that I want to tread carefully and know for sure that it's just the vue router issue, and not some additional problem beyond that.

sarayourfriend commented 2 months ago

Here's the infrastructure issue: https://github.com/WordPress/openverse-infrastructure/issues/869

I think it should be done in a single PR, the main thing will be making sure the deployment process for this temporary service is documented. Like I said in our chat, I think it isn't worth spending the time to make this fully automated like our real staging, given it's a temporary service.

zackkrida commented 2 months ago

@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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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

https://github.com/WordPress/openverse/pull/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 :pray:

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 commented 1 month ago

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: https://github.com/WordPress/openverse/pull/4224.

obulat commented 1 month ago

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 commented 1 month ago

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

obulat commented 1 month ago

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

Memory consumption testing

Steps

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

sarayourfriend commented 1 month ago

(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 commented 3 weeks ago

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

zackkrida commented 3 weeks ago

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 commented 3 weeks ago

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 commented 1 week ago

@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 :slightly_smiling_face:

zackkrida commented 1 week ago

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 commented 4 days ago

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

obulat commented 4 days ago

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:

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).