Only retain successful responses in cache about react-fetch-component HOT 8 CLOSED

@davin-english I haven't ran into this so far to make it priority. In sounds like you are using something similar to the TODO within your code. If so, and you think it's a good approach, I would be willing to merge a PR with the change. If you could throw in a test as well I would be most obliged.

from react-fetch-component.

Yeah it's pretty close to what I'm doing (I'm actually using your react-odata lib), but my code more or less looks like this:

<OData baseUrl={props.connect.url} options={props.connect.options} cache={props.connect.cache} as={'json'}>
    {({ loading, error, data }) => {
        let someProp;

        if (data) {
            someProp = data.value;
        } else if (error) {
            someProp = 'An error occurred.';

            // here
            props.connect.cache.remove(props.connect.url);
        } else {
            someProp = 'Loading...';
        }

        return <SomeComponent propName={someProp}></SomeComponent>;
    }}
</OData>

from react-fetch-component.

I'll look into adding your TODO to Fetch.js and adding a test for it if I have time.

In the meantime, I also noticed that on lines 132-134 in Fetch.js, you throw an error and return newState. Any chance you know of an alternative to not swallowing the error? The reason I ask is because I'm trying to test the else if (error) clause in my previous comment via Jest/jest-fetch-mock, but the test fails when the "throw" is encountered on line 132. If I remove it, and instead let it return newState, my test succeeds (and loads the correct error content which I can see in my snapshot).

Sidenote: Isn't that return newState line technically unreachable because of the throw before it?

from react-fetch-component.

@davin-english It's been a while since I've written this, but my recollection is:

• If I don't re-throw and an exception occurs within your React component, it gets swallowed (making it really hard to debug. For example

<Fetch>
  {({ data }) => data.foo.bar }
</Fetch>

If data or data.foo is undefined, it will swallow the Uncaught ReferenceError

• Returning newState I believe can then by handled further down the Promise chain if needed promise.catch(...).

Could you add an Error Boundary (componentDidCatch) to help?

from react-fetch-component.

Btw, I use fetch-mock. I had looked into using jest-fetch-mock, but ran into an issue when I tried using it a while back.

I did just remember having an issue when I upgrade to fetch-mock 6.3 and have an test skipped. If there is a better solution to error handling, I'm interested. The things to be mindful of are:

• Not swallowing errors when the component's rendering/etc
• Handling a network error (CORS, etc)

from react-fetch-component.

6.0.1 appears to be working great with the merge. I think this issue can be closed. I'll look into using an Error Boundary for the nextState issue. And depending on what I find, I'll let you know if I need to put up another PR for it.

from react-fetch-component.

Thanks for the update and sounds good.

from react-fetch-component.

@techniq FYI switching to fetch-mock indeed fixed my error testing woes. No need for Error Boundaries or anything special.

from react-fetch-component.