Skip to content

feat(vue-app): new fetch syntax #6880

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

Merged
merged 64 commits into from
Feb 4, 2020
Merged

feat(vue-app): new fetch syntax #6880

merged 64 commits into from
Feb 4, 2020

Conversation

atinux
Copy link
Member

@atinux atinux commented Jan 16, 2020

Types of changes

  • Bug fix (a non-breaking change which fixes an issue)
  • New feature (a non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Description

ℹ️ See RFC: nuxt/rfcs#27
ℹ️ See original pull request first: #5118
ℹ️ Documentation about its usage is available on https://github.com/nuxt/nuxt.js/tree/feat/new-fetch/examples/new-fetch

This PR is not a breaking change since it activates this new fetch only when fetch is used without expecting any arguments.

Old fetch

export default {
  fetch(context) {
    // old fetch logic
  }
}

With this pr, this syntax is now deprecated in favour of middleware(context)

New fetch

export default {
  fetch() {
    // New fetch, access to `this`
  }
}

Resolves #3776, #32 and #127

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly. (PR: en: new fetch documentation docs#1823)
  • I have added tests to cover my changes (if not applicable, please state why)
  • All new and existing tests are passing.

@atinux atinux mentioned this pull request Jan 16, 2020
6 tasks
@MLDMoritz
Copy link

If you need any help please let me know.

galvez
galvez previously requested changes Jan 16, 2020
@codecov-io
Copy link

codecov-io commented Jan 19, 2020

Codecov Report

Merging #6880 into dev will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##              dev    #6880   +/-   ##
=======================================
  Coverage   62.66%   62.66%           
=======================================
  Files          82       82           
  Lines        3300     3300           
  Branches      898      898           
=======================================
  Hits         2068     2068           
  Misses        989      989           
  Partials      243      243
Flag Coverage Δ
#unittests 62.66% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0dbf645...f16ebdd. Read the comment docs.

@pi0 pi0 merged commit 6db325c into dev Feb 4, 2020
@pi0 pi0 deleted the feat/new-fetch branch February 4, 2020 18:36
@husayt
Copy link
Contributor

husayt commented Feb 11, 2020

@pi0 @atinux
I have been playing with new fetch for last few days and came across some practical hurdles, the most painful of them is error handling. The way it currently implemented it allows only hiding the component through $fetchState.error variable. So if $fetchState.error===true you can only hide your component. But often I want to be taken to error page if data is not found, as I would normally do with asyncdata via error method.

 <span v-if="$fetchState.pending" >loading</span>
 <span v-else-if="$fetchState.error" >error</span>
 <Quote :item="quote" />
       
.....

  async fetch() {
     try {
      this.quote = await this.$api.getQuote(this.$route.params.id)
    } catch (e) {
      this.$nuxt.error({
        statusCode: 404,
        message: "Quote not found " + this.$route.params.id
      })
      this.$fetchState.error = true
      // throw new Error("Aforizm tapılmadı: " + this.$route.params.id)
    }
  },

So I try the above code and it works fine on client-side, but if error happens during SSR I get The client-side rendered virtual DOM tree is not matching server-rendered content. warning.

So how this most common error handling pattern should implemented via new Fetch hook ? Thanks

@atinux
Copy link
Member Author

atinux commented Feb 12, 2020

Hi @husayt

Actually, since this happens in serverPrefetch hook, we cannot change the the root element on SSR anymore. So we are blocked only at the component level.

I will recommend to use asyncData when it's about the page data actually @husayt

I believe that with Vue 3 we may find an alternative to it 🤞

@husayt
Copy link
Contributor

husayt commented Feb 12, 2020

Thanks @atinux ,
since this hook was introduced I was trying to clarify for myself role of new fetch along other hooks. Initially I thought that asyncdata might not be required anymore, but as this example shows that asyncdata has still got a valid usecase.

For now the main benefit for me it seems that new fetch allows to asynchronously provide data at component level for both server and client side, which is itself is huge and allows new patterns not available to us before.

Another gotcha with fetch is that it happens at the very end. Would be very interesting to explore firing data request in page middleware/asyncdata and awaiting in fetch

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 this pull request may close these issues.

Allow layouts or the error layout to use fetch/asyncData