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

Suspended component still trigger watchers while being switched out #6811

Open
posva opened this issue Oct 3, 2022 · 7 comments
Open

Suspended component still trigger watchers while being switched out #6811

posva opened this issue Oct 3, 2022 · 7 comments
Labels
🐞 bug Something isn't working need guidance The approach/solution in the PR is unclear and requires guidance from maintainer to proceed further. scope: suspense

Comments

@posva
Copy link
Member

posva commented Oct 3, 2022

Vue version

latest

Link to minimal reproduction

https://sfc.vuejs.org/#eNq1VMGOmzAQ/ZWRL2QlAneUbLt/UHWvXFgYwKmxLduQVhH/3sF2GtioaaXVKorAM2+ex2+eubAXrbNpRFawg60N1w4sulE/lxKAD1oZBxewfSWEOn/HNgVt1MQbhBlaowZIqDhZgb9VBqV7/SXrmM/yW2jZ6R78Yu/RPhbh9HO1ktaBUaNDOK762d3InxZc7G6XeGiShhJKASz/dpS140qCU10ncPcElyUMAZZNlRgX/s3qeFwf6sum6WKVWojmUh7yoCMpSAuHgxaVQ6/n4W10jjb/Wgte/ziWLHRRMp+F2JRH5gHqSWjZmwg5vI5Wo7SB0EdqRWJKagIKbonUN18yyK8l+armkP/piKUsjGE/VDo7WSXJBF6OMiZsyYqrQCWjYSzrkvXOaVvkuW39hE42U6bL6S0zo3R8wAztsH8z6mzREHHJ0hVHTsEJzZ5ka9CgecT5DnrHGyWf6Shbkz2087lydZ8Clyes3dbGwWhKYCZUt0sCKfjBC1U1XHaJ99LWjoHp6jkP8HvsyF/H57WbUgihKOpms7qvZIehncVcgWhOr2A+DNhwmlwBzhBViLZitH0BiVbW+as1U9kjDzZ8isZYnS54LqS2Fnl3HW/C/q+k4eLhTw9ssK1GQQXRvZW/RX5Et7sYhflMff2+HxA4dGmobyPhQhZclv7xjy/Anfpe2b/JP/8GNyvngQ==

Steps to reproduce

  • click the button

What is expected?

only one watcher should trigger

What is actually happening?

both, the watcher of the leaving and entering component are benig triggered

System Info

No response

Any additional comments?

In this scenario the update of the variable being watched is what triggers the switch

Is this just a race condition? Since the old component still needs to display, I would say it's normal for it to render due to watchers. This would require vue router to have a special handling of the current route so

@posva posva added 🐞 bug Something isn't working need guidance The approach/solution in the PR is unclear and requires guidance from maintainer to proceed further. scope: suspense labels Oct 3, 2022
@moushicheng
Copy link
Contributor

moushicheng commented Oct 5, 2022

let me make a supplement,in Vue document

When a revert happens, fallback content will not be immediately displayed. Instead, will display the previous #default content while waiting for the new content and its async dependencies to be resolved.

" will display the previous #default content" ,It means that unmounted old component will delay. so when you switch ParentSync to ParentAsync ,it will trigger watch in ParentSync as it's still not unmounted. So, I'm not sure if this is a bug.

To avoid this situation,use timeout=0 prop,as follow:

<Suspense timeout=0>
  <component :is="route" />
</Suspense>

@adamjedlicka
Copy link
Contributor

@moushicheng @posva This wont do, because original issue arose from a vue-router use case. When I'm navigating between pages, I want the old page to be displayed, while new page is being loaded in the background thanks to Suspense. Unfortunately this is currently impossible in vue-router, because the old page keeps responding to effects/watchers while being switched out.

I'm quite baffled by this, because I don't know how to fetch data in pages based on route parameters, so that old pages don't trigger re-fetch while navigating to different page. Nuxt has the same problem.

@posva
Copy link
Member Author

posva commented Oct 6, 2022

You can check the data fetching documentation in vue router, there are a few examples. There is also an ongoing RFC for a more advanced data fetching api in the rfcs repo

@adamjedlicka
Copy link
Contributor

@posva Thanks, I looked into both, but unfortunately neither is sufficient for us.

First, about the data fetching in the Vue Router documentation. Data fetching in beforeRouteEnter or in the created method are kinda meh since Vue 3 with setup came. I need application context for the data fetching (is the user signed? what config is in the store? what cookies are set? what route parameters are there?...) which are only accessible via composables in the setup method. (BTW, wishful thinking, it would be great if we could use composables relying on app context (provide/inject) in route guards).

Fetching in created is now even less desirable since introduction of Suspense.

@adamjedlicka
Copy link
Contributor

Here is a unit test:

  test('post flush watchers in toggled components', async () => {
    let cnt = 0

    const CompA = {
      template: `<div>A</div>`,
      setup: async () => {
        const route = inject<any>('route')

        watch(
          () => route.value,
          () => cnt++,
          { immediate: true, flush: 'post' }
        )
      }
    }

    const CompB = {
      template: `<div>B</div>`,
      setup: async () => {
        const route = inject<any>('route')

        watch(
          () => route.value,
          () => cnt++,
          { immediate: true, flush: 'post' }
        )
      }
    }

    const route = shallowRef(CompA)

    const Parent = {
      template: `
        <Suspense>
          <Component :is="route" />
        </Suspense>
      `,
      setup: () => {
        provide('route', route)
        return { route }
      }
    }

    const root = nodeOps.createElement('div')
    render(h(Parent), root)

    // wait for flush
    await nextTick()
    // wait for child async setup resolve
    await nextTick()

    expect(serializeInner(root)).toBe(`<div>A</div>`)
    expect(cnt).toBe(1)

    route.value = CompB

    // wait for flush
    await nextTick()
    // wait for child async setup resolve
    await nextTick()

    expect(serializeInner(root)).toBe(`<div>B</div>`)
    expect(cnt).toBe(2)
  })

@adamjedlicka
Copy link
Contributor

@posva Hey. So I created a PR with a fix here: #7009 . Could you please look at it and ideally merge it?

@metkm
Copy link

metkm commented Mar 21, 2024

I see some issues linked here but hey seem to look like they are solved but I this this issue still occurs. Or it might be because I have this problem using nuxt 3.

Currently using flush: 'post' solved the issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 bug Something isn't working need guidance The approach/solution in the PR is unclear and requires guidance from maintainer to proceed further. scope: suspense
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants