fix(vue-app): this.$nuxt.refresh doesn't refresh every fetch hooks

[ms-fadaei]

With this fix, layout fetch, layout child components fetch (including the page), and page child component fetch will be called during the Nuxt refresh helper.

Types of changes

• [x] 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

In the current version of the Nuxt when we call $nuxt.refresh(), the page fetch will be called if the current page has a fetch, otherwise, the child components fetch method will be called.

with this fix:

• if the layout is enabled:
  • if the layout itself has the fetch, it will be pushed to the call list
  • if the layout child components (including the page) have the fetch, they will be pushed to the call list
• if the layout is disabled:
  • if the page itself has the fetch, it will be pushed to the call list
  • if the page child components have the fetch, they will be pushed to the call list

Caution: What about nested routes? layout fetch will be inserted twice? can use set instead of array to prevent duplication? fix: #9439

Checklist:

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

[codecov-commenter]

Codecov Report

Merging #9530 (0a117cb) into dev (123206c) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##              dev    #9530   +/-   ##
=======================================
  Coverage   65.19%   65.19%           
=======================================
  Files          94       94           
  Lines        4155     4155           
  Branches     1172     1172           
=======================================
  Hits         2709     2709           
  Misses       1167     1167           
  Partials      279      279           

Flag	Coverage Δ
unittests	65.19% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

---

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 123206c...0a117cb. Read the comment docs.

[pi0]

What about nested routes? layout fetch will be inserted twice? can use set instead of array to prevent duplication?

Can you please ensure about this? If is a case, we might make use of WeakSet to dedup. (Note: IE support 🤷)

Also can you please check behavior when page does not having $fetch but layout does?

[ms-fadaei]

Can you please ensure about this? If is a case, we might make use of WeakSet to dedup. (Note: IE support 🤷)

Also can you please check behavior when page does not having $fetch but layout does?

I checked that right now. it not working properly with nested routes (then I should refactor the code) I checked that too and it seems everything is right!

[ms-fadaei]

@pi0 what you thinking about getChildrenComponentInstancesUsingFetch https://github.com/nuxt/nuxt.js/blob/HEAD/packages/vue-app/template/utils.js#L56-L68

If a child has the fetch, children of it will not be considered!

[ms-fadaei]

And I realized the sequence call of fetch (first parent then child) is not guaranteed! (in the current version this is just about nested routes because of limited fetch calls but in my version, this occur in every parent-child relations)

[ms-fadaei]

@pi0 @clarkdo I'm done with this PR.

We still have two edge cases that I mentioned above with refresh handler and they aren't new! they exist from the last versions

[lihbr]

Hey friends, what's up on this one? What remains to be changed/discussed to lead to a merge? or is it just awaiting a review?

Cheers!

[lihbr]

I'll try to have a look at the last bit @danielroe mentioned this week 💪 Keeping you up-to-date!

[lihbr]

OK, I'm having a bit of trouble understanding the "duplicate fetch" maybe issue mentioned above.

Looking at the changes, 3 things have been achieved:

1. getChildrenComponentInstancesUsingFetch() has been modified to handle an exclude array, stopping the crawling if a node from exclude is encountered during the crawling process;
2. On the pages.map(), the callback now loop only over the children component of the current page, theoretically deduplicating redundant calls (? not 100% confident on that one, but that's what I understand);
3. After the pages.map(), the refresh() method now looks for a layout, if one it adds its own fetch and crawls its children component, excluding pages and their offspring.

Taking all of those changes individually they seem pretty straightforward to me. Basically, the refresh() method now achieve that kind of resolution for fetch hooks (I had budget for an animation today):

I don't really see how we can end up with duplicate calls here, or at least how we can end up with duplicate calls we didn't have already before(?)

Now, maybe where I'm wrong is on my understanding of the getMatchedComponentsInstances() function. To me, that function gets each page component actually displayed on the current route, so from my gif above ~/pages/blog.vue & ~/pages/blog/index.vue? I'm not sure about that function behavior as it seems to access properties of Vue Router current route which I'm not aware of/are Nuxt specific(?): https://router.vuejs.org/api/#route-object-properties

If I'm wrong here and there's definitely something leading to duplicated behaviors, I'm not sure however a set would solve the issue? Or maybe it would? I'd be afraid only some of the deduplicated hooks get called and that the others remain stale (a.K.a. no new data?)

Let me know!

[pi0]

@ms-fadaei @lihbr Can you please check the latest nuxt-edge? I believe #10489 should address the issues by refetching on all children. If not happy to reopen and improve 👍🏼

[lihbr]

Hey there,

Unfortunately, this doesn't appear to fix the issue as the component-crawling function still starts from the page component instead of the layout.

Maybe I'm doing something wrong, here's my repro on edge: nuxt_nuxt.js_9530_1.zip

[pi0]

Thanks for checking @lihbr. @ms-fadaei Would you please rebase your branch to dev?

[ms-fadaei]

Thanks for checking @lihbr. @ms-fadaei Would you please rebase your branch to dev?

Hi Pooya, Done!