Implementation Plan: Nuxt 3 migration

[obulat]

Fixes

Resolves #2324 by @obulat

Reviewers

• @sarayourfriend - for experience with the previous attempts at migrating to Nuxt 3
• @zackkrida - for experience with initial conversion from Vue SSR to Nuxt

Description

This is a plan of how I propose to approach Nuxt 3 migration. I am especially curious about ideas on "Nuxt 3 / Vue 3 features in Openverse: discussion" section. It must be noted that there will be changes to this plan during its implementation due to the unknowns of this process.

Rendered plan: https://docs.openverse.org/_preview/2350/projects/proposals/nuxt_3_migration/20230608-implementation_plan_nuxt_3_migration.html

This discussion is following the Openverse decision-making process. Information about this process can be found on the Openverse documentation site. Requested reviewers or participants will be following this process. If you are being asked to give input on a specific detail, you do not need to familiarise yourself with the process and follow it.

Current round

This discussion is currently in the Clarification round.

The deadline for review of this round is TBD.

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.

[github-actions]

Full-stack documentation: https://docs.openverse.org/_preview/2350

Please note that GitHub pages takes a little time to deploy newly pushed code, if the links above don't work or you see old versions, wait 5 minutes and try again.

You can check the GitHub pages deployment action list to see the current status of the deployments.

[sarayourfriend]

This looks good to me so far, and it gives a decent sense of how much we have to do for this. I've left a handful of questions for clarification, but I would also like to request a clearer/more explicit list of the required changes, ordered as though could be converted to issues and perhaps with links to individual sections for ones that need more explanation. Splitting the changes between the modules section, the Vue 3 breaking changes section, and the implementation stage sections makes it hard to get a general overview of how the implementation would go. I appreciate the two implementation stages being explicit, though. How much benefit is there to pushing as much as we can forward into the pre-nuxt3 branch phase? Seems like it would prevent a lot more merge conflicts, right? Also, should we put a freeze on frontend feature development during the Nuxt 3 conversion phase, with all frontend development focused exclusively on Nuxt 3 conversion? It would be frustrating to have the Nuxt 3 branch almost perfectly working only to run into big feature related merge conflicts (like #2118 and #2126, which haven't started implementation yet).

[sarayourfriend]

This is a personal opinion based on experience with auto imports, but I absolutely loathe them. My primary complaints are:

• They make code much harder to follow or understand. "Where the heck does "useAnalytics" come from? Anything dynamic automatically becomes a weird special case. Variable name collision and shadowing is also much harder to understand or anticipate side effects for.
• The are "magic" through and through, and magic is great until it doesn't work and then suddenly you are faced with having to understand the magic (which is invariably more complex than the non-magical way) to workaround it, maybe just to fix a bug that would otherwise be trivial.
• They only very slightly increase velocity for those experienced with the magic while significantly increasing friction for anyone else who's never seen such magic before, maybe didn't even know it existed.

The few times I've had to work on a Rails app, for example, I wanted to pull my hair out the entire time because everything was "just there" without any easily traceable threads.

I've also literally never said to myself, "I hate typing out imports" or "Wow, I just spent ages working out the imports of this module" etc. That is to say, I've never thought this was a problem, much less one that would need heaps of time and effort put into solving it and living with the lack of control or precision such a solution would invite.

Anyway, that's my personal opinion and experience with this kind of thing. I vehemently dislike it. But if the majority of maintainers think it's a better experience, then I will hold my nose and accept it. I will note, however, that it would be a huge pain to reverse this if we went with it, so if we go for it, we should be prepared to either accept it even if it turns out to be worse or to spend heaps of time adding imports back into code that had it before and worked fine with it in the first place.

[obulat]

I anticipated a similar response :) Thank you for describing the reasons in such a detail, it's really helpful.

On the first point, I looked at the Nuxt 3 draft branch locally to see if the IDE support helps with it. And even though I can click "Go to definition" on a non-imported component and open the component file, I can't do the same for a composable. Going to definition there sends you to the file that lists all of the auto-imported items, so while it is possible to find the source of the composable, it's not as straightforward as when we import it.

Variable name collision and shadowing is also much harder to understand or anticipate side effects for.
This is also a serious consideration.

The second point - I guess you can't understand the extent of how bad it can be until that happens to you :)
I agree with the third point, too.

What I wanted to avoid was going against the framework. This would cause us to have to add work-arounds around the magic, which might need a lot of work. However, seeing that the IDE support is not as great as I was expecting, and considering all of the drawbacks of using auto-imports, I'm open to not using this feature.

[obulat]

After writing this comment, I decided to check if we would have problems with the imports and tests. In the current Nuxt 2 app, the components are auto-imported, so the app can run even if we don't add all component imports. However, the tests show warnings for components that weren't imported.

In my first try, this was a big problem with explicitly adding all of the imports. The biggest problem is the imports from .nuxt and the imports that start with #, especially deep within modules (nuxtjs/svg-sprite being the one I tried to work around the most). Not saying that this is a blocker, but just noting here that getting all of the imports to work both in the prod app and the tests would require a lot of debugging.

[sarayourfriend]

And even though I can click "Go to definition" on a non-imported component and open the component file, I can't do the same for a composable. Going to definition there sends you to the file that lists all of the auto-imported items, so while it is possible to find the source of the composable, it's not as straightforward as when we import it.

This reminds me actually, that Nuxt 3's auto-imports I believe is the main reason why running TypeScript requires a build. Nuxt 3's build process pulls the auto-imported composables out and generates the tsconfig.json to include all the ambient types that you're meant to extend in your Nuxt app's tsconfig.json.

I wonder how much all those ambient types slow down TypeScript (or perhaps increase its memory footprint).

In either case, we'd need to change CI to run the types check only after a Nuxt 3 build is run, at least if that process is still the same, which sounds to be the case based on the IDE behaviour of go-to-definition for the composables.

Not saying that this is a blocker, but just noting here that getting all of the imports to work both in the prod app and the tests would require a lot of debugging

That's a shame 🙁. When I tried to convert Nuxt using bridge, the option to disable auto-imports didn't even work. Does turning off auto-imports stop component auto-importing in the app build? The Nuxt 3 only reference auto-importing for composables, both in the composables documentation and in the auto-imports documentation 🤔

[obulat]

In either case, we'd need to change CI to run the types check only after a Nuxt 3 build is run, at least if that process is still the same, which sounds to be the case based on the IDE behaviour of go-to-definition for the composables.

I will add this to the proposal, thanks! (Check if we need to change CI to run the types check after a Nuxt 3 build is run - change if necessary)

The Nuxt 3 only reference auto-importing for composables, both in the composables documentation and in the auto-imports documentation 🤔
I'm not sure if I understand this sentence correctly, but Nuxt 3 docs do say that they auto import components, composables and even utils (although the way it's worded is not very straightforward): https://nuxt.com/docs/guide/concepts/auto-imports#directory-based-auto-imports

By the way, I didn't realize that the recommended way to explicitly add imports is by using #imports, found while re-reading the docs for this comment.

[sarayourfriend]

Check if we need to change CI to run the types check after a Nuxt 3 build is run - change if necessary

This would also need to change in the pre-commit, which is probably the place it will hurt the DevEx the most.

[zackkrida]

I definitely understand the desire to do what's idiomatic for the framework and absolutely hating this automagical import stuff.

Does turning off auto-imports stop component auto-importing in the app build?

I just tested this with a fresh Nuxt 3 install. Sadly, components still appear to be auto imported even with this config:

export default defineNuxtConfig({
  imports: {
    autoImport: false
  }
})

It does work for composeables, but not components.

I'm really conflicted on how to proceed for this one.

[sarayourfriend]

Actually, if it still auto-imports components, I think that's fine. That's the current behaviour of Nuxt 2, so it would present effectively zero change to our workflow.

[zackkrida]

Oh, true!

[sarayourfriend]

this conversion is out of scope for this project

Agreed 👍

The decision wether to convert components to script setup would be at the discretion of the PR implementer.

It's probably worth emphasising anywhere relevant that this should only be done if it absolutely simplifies the PR. We should not refactor for refactoring's sake alone, after all.

[sarayourfriend]

Vitest looks great and I appreciate the fact that it would remove the need to do all the Jest juggling of transformers and such. Type tests is also interesting and may be useful in some places.

Do you know how much ESLint plugin compatibility exists between the two? The APIs are supposed to be very similar, so I'd expect that a lot of the eslint-plugin-jest rules would work out of the box. Have you explored this at all?

[obulat]

I've found eslint-plugin-vitest which says that most of its rules are essentially the ports of eslint-plugin-jest with necessary modifications.
I'll add it here.

[sarayourfriend]

That's fantastic 🚀

[zackkrida]

Vitest maintains a migration doc that might be useful to link here:

https://vitest.dev/guide/migration.html#migrating-from-jest

[sarayourfriend]

Sadly the package had some work done to support ESM that never got merged 😢

nbubna/Case#37

I wonder if we could help out by finishing out the existing PR?

The library is MIT, so as you send, we could vendor in these functions with credit. Contributing the library would be ideal but given our time constraints with repsect to Node 16 EOL, it may not be possible. Vendoring the functions would also have the happy side effect of "tree-shaking" out the rest we don't use 🤷

[obulat]

I've read through this PR when writing the plan, and felt that it's really complicated because it's trying to support ESM and UMD, and several ways of exporting, too.

Vendoring seems the best solution for the short term, with some effort contributing to the library in the long term.

[sarayourfriend]

Do we have async components currently? Which ones would need this update?

[obulat]

The banners inside the VBanner, and headers in the search-layout.

openverse/frontend/src/components/VBanner/VBanners.vue

Lines 34 to 38 in 41b0cdb

	components: {
	VMigrationNotice: () => import("~/components/VBanner/VMigrationNotice.vue"),
	VTranslationStatusBanner: () =>
	import("~/components/VBanner/VTranslationStatusBanner.vue"),
	VAnalyticsNotice: () => import("~/components/VBanner/VAnalyticsNotice.vue"),

openverse/frontend/src/layouts/search-layout.vue

Lines 73 to 75 in 41b0cdb

	VHeaderDesktop: () => import("~/components/VHeader/VHeaderDesktop.vue"),
	VHeaderMobile: () =>
	import("~/components/VHeader/VHeaderMobile/VHeaderMobile.vue"),

I'll add this to the plan

[sarayourfriend]

These changes do not replace defineEvent, as far as I can tell. The emits feature described in the link is actually required for defineEvent to work in the first place, the docstring for the function specifies that it's a workaround for TS in Composition API: https://github.com//WordPress/openverse/blob/697f62f01a32cb7fcf2f4a7627650a113cba40da/frontend/src/types/emits.ts#L9-L18

I'm surprised that Vue doesn't have a built-in solution for this though, it's very silly to need to write an empty validation function (maybe one that even gets included in your final bundle, I'm not sure if Nuxt shakes those out in build) just to get types for events.

In any case, the defineEvent utility was already developed for the feature linked on the documentation page and I don't see anything there or on the Vue 3 emits documentation for ways to define the event types without the strange empty validator function workaround.

[obulat]

I was actually thinking of defineEmits: https://vuejs.org/api/sfc-script-setup.html#defineprops-defineemits, more on typing emits: https://vuejs.org/guide/typescript/composition-api.html#typing-component-emits

However, they are only available in components that use script setup.

I think the best course here would be to keep using defineEvent with emits, and refactor the components to script setup and defineEmits later, after this project is done. It could be a separate milestone for converting all of the components that emit events to script setup.

[sarayourfriend]

I think the best course here would be to keep using defineEvent with emits, and refactor the components to script setup and defineEmits later, after this project is done. It could be a separate milestone for converting all of the components that emit events to script setup.

Sounds good to me! I'm now remembering that I wrote defineEvent in frustration that defineEmits didn't work in traditional components 😅

[sarayourfriend]

We need a step in this to remove the nuxt-templates-overrides too, right?

[obulat]

Right!

[sarayourfriend]

Can you give more details for why this is the modification required to handle this breaking change? I'm not making the connection between inheritAttrs="false" and the class and style attributes being included in $attrs.

[sarayourfriend]

Would it be possible to describe the necessary Nuxt 3 changes here the way the Vue 3 changes are described?

[zackkrida]

As I shared with Olga, I'll review this today.

[obulat]

I've updated the plan to add the suggestions based on @sarayourfriend's comments. I removed the "Breaking changes" section for Vue and Nuxt. Instead, I added the steps of what we need to update with the note if it's a "Vue 3 breaking change". I also tried to add links to documentation to each step.
Some items will need to be edited for clarity, I'll try to do it tomorrow.

[openverse-bot]

Based on the medium urgency of this PR, the following reviewers are being gently reminded to review this PR:

@zackkrida
This reminder is being automatically generated due to the urgency configuration.

Excluding weekend¹ days, this PR was ready for review 7 day(s) ago. PRs labelled with medium urgency are expected to be reviewed within 4 weekday(s)².

@obulat, if this PR is not ready for a review, please draft it to prevent reviewers from getting further unnecessary pings.

Footnotes

1. Specifically, Saturday and Sunday. ↩

2. For the purpose of these reminders we treat Monday - Friday as weekdays. Please note that the operation that generates these reminders runs at midnight UTC on Monday - Friday. This means that depending on your timezone, you may be pinged outside of the expected range. ↩

[sarayourfriend]

@zackkrida Will you be able to review this still or are you planning to wait until after the revisions?

[zackkrida]

I'll be reviewing this prior to any additional revisions. I did want until the previous revisions were made, after chatting about them with Olga.

[zackkrida]

I think we could just vendor this and recommend that, rather than listing multiple potential options here.

[zackkrida]

Could you elaborate on what this entails? It feels really vague and I'm not sure what it refers to.

[zackkrida]

Team capacity may be a legitimate concern here. At the very least, we would have to pull maintainers from other parts of the project if we plan to do this in Q3-Q4 of this year.

[zackkrida]

Some other risks I can think of:

1. Drift between the nuxt3 and main branches, or difficulty keeping them in sync
2. Encountering an unexpected dependency incompatability that halts the migration, further exacerbating pt.1

[zackkrida]

This is an excellent breakdown of the sheer mass and complexity of work required to execute this migration. I have to admit I find it terrifying. Particularly the lack of stability and support in many of the ecosystem packages: i18n, nuxt-vitest, storybook (there's a $500 bounty in this issue, sentry, and I imagine many more. It seems the only thing that might bring some stability to these dependencies is more time, as many of them are in active development. I worry that the list of changes here represents our known issues, that the list of known issues is quite sizable, and that the list of unknown issues may be even larger.

Given that this is no longer time sensitive, as #2430 adds Node 18.x support which will be maintained far into 2025, I wonder if an ideal course of action may be to focus on the prerequisite changes, but also any work to audit and clean up the frontend as much as possible beyond what is necessary for Nuxt 3. And to explicitly revisit the Nuxt 3 part of the project several months from now when some of these libraries might have better support.

It's a fine line between delaying the inevitable and waiting for dependencies to make improvements.

It's also worth considering that two frontend contributors, in fact the reviewers of this proposal, will either be AFK entierely or have reduced capacity at the time of year we might consider implementing this work.

I could also see the Nuxt 3 steps and branch dragging on for a really long time. I think the workflow to fix these problems could benefit from some pair programming sessions. So often this work involves untangling cryptic dependency errors.

I think I'd like to wait and give this a final approval once any remaining revisions are in place, but the overall approach and breakdown of the tasks makes a lot of sense to me. I find myself really distracted by how gargantuan of an undertaking this will be.

[sarayourfriend]

Given that this is no longer time sensitive, as #2430 adds Node 18.x support which will be maintained far into 2025, I wonder if an ideal course of action may be to focus on the prerequisite changes, but also any work to audit and clean up the frontend as much as possible beyond what is necessary for Nuxt 3. And to explicitly revisit the Nuxt 3 part of the project several months from now when some of these libraries might have better support.

I strongly support this option now that Node 16 EOL is no longer a factor, especially when Elasticsearch 17 EOL is also approaching at the same time we were originally planning to implement this, as @obulat (IIRC, might have been Krystle) pointed out elsewhere.

[krysal]

@obulat Should this be marked as blocked until #2490 is merged? The project thread says it depends on some TypeScript updates. Could you leave a comment on what is needed to complete this plan?

[obulat]

Closing this PR until the Additional search views project is done. Then, I'll re-open it with the updated status and plan.