[BUG]: Query does not refetch on the next mount about suspensive HOT 12 CLOSED

@manudeli nevermind. I was wrong. It doesn't work in both cases

from suspensive.

@timsofteng, I tested it in the reproduction environment you leave me.
What you mean is that you think ErrorBoundary should try to reset if there is a caught error? However, if ErrorBoundary tries to reset on every mount, it will make ErrorBoundary can't show fallback. so I don't think it's a good idea to force reset every mount.

If you have a good idea, could you please fork suspensive, and create a branch, and suggest it as a pull request with modifying @suspensive/react's code directly on websites/visualization? This way will make you can more clearly show what you want.

from suspensive.

And I want to suggest this ErrorBoundaryGroup way to meet your needs

I want to modify https://github.com/timsofteng/query-boundary-bug/blob/suspensive/src/PageTwo.tsx like below code.
@suspensive/react's ErrorBoundaryGroup will easily trigger ErrorBoundary's reset in parent.

Could you resolve your problem in this way?

import { useEffect, useState } from 'react'
import reactLogo from './assets/react.svg'
import viteLogo from '/vite.svg'
import './App.css'
import { useNavigate } from 'react-router-dom'
import { useFetchTodo } from './api'
import { ErrorBoundary } from './ErrorBoundary'
import {
  useErrorBoundaryGroup,
  withErrorBoundaryGroup,
} from '@suspensive/react'

const PageTwo = withErrorBoundaryGroup(function PageTwo() {
  const group = useErrorBoundaryGroup()

  useEffect(() => {
    group.reset()
  }, [group])

  return (
    <ErrorBoundary>
      <Wrapped />
    </ErrorBoundary>
  )
})

export default PageTwo

function Wrapped() {
  const [count, setCount] = useState(0)

  const navigate = useNavigate()
  useFetchTodo()

  return (
    <>
      <div>
        <a href="https://vitejs.dev" target="_blank">
          <img src={viteLogo} className="logo" alt="Vite logo" />
        </a>
        <a href="https://react.dev" target="_blank">
          <img src={reactLogo} className="logo react" alt="React logo" />
        </a>
      </div>
      <h1>Page 2</h1>
      <button onClick={() => navigate('/one')}>to page one</button>
      <div className="card">
        <button onClick={() => setCount((count) => count + 1)}>
          count is {count}
        </button>
        <p>
          Edit <code>src/App.tsx</code> and save to test HMR
        </p>
      </div>
      <p className="read-the-docs">
        Click on the Vite and React logos to learn more
      </p>
    </>
  )
}

from suspensive.

@manudeli
Thank you for your reply and for your solution.
To be honest I still have don't understand why it doesn't work just out of the box in my example.
Why do we need this strange useEffect and additional hook?
Expected behavior is just send another request on mount after this failed component was unmounted.

Please notice that it works exactly as I expected if we change route "/one/two" just to route "/two".

from suspensive.

Your solution means we should manually reset existed boundary for the component which was unmounted previously. But I don't understand why is this boundary existed for it.

from suspensive.

The best solution which I've tried was this inside of the fallback component

  useEffect(() => {
    return () => {
      queryClient.removeQueries({
        predicate: (q) => q.state.status === "error",
      });
    };
  }, []);

from suspensive.

@timsofteng

The best solution which I've tried was this inside of the fallback component

  useEffect(() => {
    return () => {
      queryClient.removeQueries({
        predicate: (q) => q.state.status === "error",
      });
    };
  }, []);

However, I think this will work against your intentions. Because you're intending to just try to show the children of this ErrorBoundary when the webpage returns to "/two", right?

But I think

1. this will show the fallback first when back to "/two"
2. and inside the fallback it will call queryClient.removeQueries
3. then show ErrorBoundary.children.

I don't think this order is what you want. What do you think?

from suspensive.

@manudeli May you please describe it clearly? Please notice that hook will be triggered on unmount of fallback component in my example.

from suspensive.

@manudeli May you please describe it clearly?

I update my comment right now sorry.

Please notice that hook will be triggered on unmount of fallback component in my example.

Ah ha! I misunderstood it...! well, It's better than my example I think. but I got idea from you, it's not really clear, but do you think we need to add ErrorBoundary's new optional prop like onUnmount((caught): void) or resetOnUnmount(boolean)?

I clearly understand at what point you need these implementations.

from suspensive.

@manudeli we definitely need some solution for this issue and both your solutions sounds good.
The only one thing I'm worried about is this issue exists only for routes /one /one/two and doesn't exist for routes /one /two. I have no idea why.

from suspensive.

You mean you can't meet no refetching when revisiting /two not /one/two? Actually, I could meet no refetching in my local machine right now when /one, /two too same with /one, /one/two in https://github.com/timsofteng/query-boundary-bug. Could you show me video or gif capturing your display? I couldn't reproduce it like you.

I want to recommend this tool https://chrome.google.com/webstore/detail/chrome-capture-screenshot/ggaabchcecdbomdcnbahdfddfikjmphe this is pretty good to video capture in browser. I usually use it when I need capture gif to upload in GitHub

from suspensive.

@timsofteng I'll close this issue first, but if there issue again, you can reopen this issue too. 👍

from suspensive.