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

Remove or replace Nuxt reference and logic for Vue3 migration #10491

Closed
16 tasks done
cnotv opened this issue Feb 21, 2024 · 7 comments · Fixed by #10533
Closed
16 tasks done

Remove or replace Nuxt reference and logic for Vue3 migration #10491

cnotv opened this issue Feb 21, 2024 · 7 comments · Fixed by #10533
Assignees
Labels
Milestone

Comments

@cnotv
Copy link
Contributor

cnotv commented Feb 21, 2024

Description

Some lingering code inherited form Nuxt is going to clash with the breaking changes from the Vue3 migrations.
These needs to be either corrected to match the syntax, removed if unnecessary or replaced with a new implementation.

Context

Given the deprecation of the emitters, all these cases have to be handled as they will break for sure:

  • $nuxt.$on
  • $nuxt.$off
  • $nuxt.$emit

Other breaking cases:

  • $nuxt.$nextTick (docs)
  • Vue.util.defineReactive, used in more places
  • Vue.__nuxt__fetch__mixin__
  • Component = Vue.extend(Component); (docs) Can be removed in migration branch
  • Vue[installKey] (lines)
  • vueDirective.apply(Vue, arguments); (line) Can be removed in migration branch
  • Vue.config.$nuxt.$nuxt = true; and Vue.config().$nuxt = {}; (line)
  • vueApp.config().$nuxt (line)
  • nuxt.publicRuntimeConfig, this is needed to start the app (line) Seems can be left untouched

Optional other cases which would be nice to get rid of:

  • $nuxt.$nuxt
  • $nuxt.$store
  • $nuxt.$route
  • $nuxt.$router
@cnotv cnotv added the kind/tech-debt Technical debt label Feb 21, 2024
@cnotv cnotv added this to the v2.8.next1 milestone Feb 21, 2024
@codyrancher
Copy link
Contributor

What's the issue you're seeing with Vue.util.defineReactive? By any chance did it just get moved? I found it in the vue code base https://github.com/vuejs/vue/blob/main/src/core/observer/index.ts#L128.

@codyrancher
Copy link
Contributor

@cnotv here's the current progress codyrancher#3.

I'll continue on the list.

@cnotv
Copy link
Contributor Author

cnotv commented Feb 22, 2024

What's the issue you're seeing with Vue.util.defineReactive? By any chance did it just get moved? I found it in the vue code base https://github.com/vuejs/vue/blob/main/src/core/observer/index.ts#L128.

In Vue 3 you cannot import anymore Vue from the library, there's just the function createApp.
The link you posted is from Vue2.

@cnotv
Copy link
Contributor Author

cnotv commented Feb 22, 2024

@gaktive gaktive modified the milestones: v2.8.next1, v2.9.0 Feb 23, 2024
@cnotv
Copy link
Contributor Author

cnotv commented Feb 26, 2024

I'm adding other cases while crossing them because the app breaks, just in case we miss them.

@codyrancher
Copy link
Contributor

@cnotv I've opened a PR which takes care of each of the checked items on the list. For the remaining items:

  1. Component = Vue.extend(Component); I think we could remove this in your branch once we have all components/pages defined in the same way but not something we can push in master right now
  2. vueDirective.apply(Vue, arguments); We already talked about this. I think this can be removed in your branch since the way directives are defined and installed is different in vue 3. I don't think we need to make this extra check.
  3. nuxt.publicRuntimeConfig What's the problem with this one? This is really just a string replacement from vue.config.js

@cnotv
Copy link
Contributor Author

cnotv commented Feb 29, 2024

3. nuxt.publicRuntimeConfig What's the problem with this one? This is really just a string replacement from vue.config.js

Yeah it actually seems working without touch it, so far.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants