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 fetch() usage as leftover from Nuxt #9711

Closed
cnotv opened this issue Sep 14, 2023 · 9 comments
Closed

Remove fetch() usage as leftover from Nuxt #9711

cnotv opened this issue Sep 14, 2023 · 9 comments
Assignees
Labels
area/vue3 Bugs and technical debts as outcome to the Vue3 migration kind/tech-debt Technical debt
Milestone

Comments

@cnotv
Copy link
Member

cnotv commented Sep 14, 2023

Description

As the Nuxt world is departed, we should replace data fetching in the Vue3 way: https://vuejs.org/guide/built-ins/suspense.html#async-setup

The feature is not supported in Vue 2.7 https://v2.vuejs.org/v2/guide/migration-vue-2-7

The pattern is so specifically intended to have it as essential rule in the linter: https://eslint.vuejs.org/rules/no-lifecycle-after-await.html

@cnotv
Copy link
Member Author

cnotv commented Apr 22, 2024

@eva-vashkevich suggested moving the logic of fetch() within the beforeMount() lifecycle hook by wrapping the evaluation within a promise as a proven solution in some cases.

@cnotv cnotv added this to the v2.9.0 milestone Apr 22, 2024
@cnotv cnotv changed the title Replace fetch() usage as leftover from Nuxt with async setup Remove fetch() usage as leftover from Nuxt Apr 25, 2024
@cnotv
Copy link
Member Author

cnotv commented Apr 25, 2024

The whole fetch() deprecation would include these considerations:

  • We would need to define async requests in another location
  • The use of async setup() is not possible due these restrictions:
    • access global vars
    • define custom lifecycle hooks
    • access to other properties from async setup(), e.g. from data()
  • The global variable $fetch() is adopted in more circumstances and will need to be replaced as well
  • fetch() is added from a mixin altogether with other elements, which should also be removed:
    • Data state which includes errors, pending, and timestamp
    • Computed $fetchState which returns the state
    • beforeCreate() is used to assign $fetch()
    • Some delay is added for fetching
  • We may consider using the vue-router data fetching hook, although it does not cover components and the current logic may not fit

@cnotv
Copy link
Member Author

cnotv commented Apr 26, 2024

After analyzing the hackish implementation of Nuxt to create the fetch() "hook", I have been trying to create a PoC on Vue3 and it seems feasible the possibility to have an async fetch() also there.

https://codesandbox.io/s/vue3-vuex-custom-hook-yjghfv?file=/src/views/Home.vue

NOTE: This is available exclusively using Options API and not inside setup @rak-phillip .

@cnotv
Copy link
Member Author

cnotv commented Apr 26, 2024

I am postponing then this refactoring after migration to Vue3 and keeping it open to define guidelines for the alternatives of fetch().

@cnotv cnotv modified the milestones: v2.9.0, v2.10.0 Apr 26, 2024
@cnotv
Copy link
Member Author

cnotv commented Apr 26, 2024

Did we not say we were going to remove this? Because of the way it's written, I am not so sure is going to work with Vue3.
What's the purpose of this code? I have seen it outside of the fetch.js file, so I considered was a secondary purpose.

Also thanks for pointing it out.

@cnotv
Copy link
Member Author

cnotv commented Apr 29, 2024

@cnotv do we understand the purpose of the code related to invoking fetch on HMR1 and matching routes2?

Just for the record, could we write down an update of what is the current issue after isolating the fetch() logic?
It would be nice to keep consistent clear information about the current state.
Even a link to something else where it has been already written is fine.

@cnotv
Copy link
Member Author

cnotv commented Apr 29, 2024

Ok so to summarize, we have this code coming from Nuxt which we are not able to identify the whole purpose. Therefore we'll rather try to recreate some further PoC based on our code. This should ensure the right execution of the lifecycle hooks and related loading components.
The outcome will be to get rid of everything else.

@nwmac nwmac modified the milestones: v2.10.0, v2.11.0 Jul 4, 2024
@gaktive gaktive added the area/vue3 Bugs and technical debts as outcome to the Vue3 migration label Oct 2, 2024
@cnotv
Copy link
Member Author

cnotv commented Oct 2, 2024

We can close this as migration is completed. Postponing this in a future architecture talk.

@cnotv cnotv closed this as completed Oct 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/vue3 Bugs and technical debts as outcome to the Vue3 migration kind/tech-debt Technical debt
Projects
None yet
Development

No branches or pull requests

4 participants