Skip to content
This repository has been archived by the owner on Nov 11, 2023. It is now read-only.

Promise callback of Mutate function not being called #121

Closed
ajhenry opened this issue May 15, 2019 · 6 comments
Closed

Promise callback of Mutate function not being called #121

ajhenry opened this issue May 15, 2019 · 6 comments

Comments

@ajhenry
Copy link
Contributor

ajhenry commented May 15, 2019

Describe the bug
When using a Mutate function with the PUT verb, the callback of the mutation promise is not called

To Reproduce
Minimal Code example to reproduce

<Mutate
    verb="PUT"
    base={`https://jsonplaceholder.typicode.com/put/posts/1`}
>
    {(update, { loading, error }) => {
    console.log(loading);
    console.log(error);
    return (
        <div
        style={{ width: "100px", height: "100px" }}
        onClick={() => {
            console.log("Clicked");
            update({
            id: 1,
            title: "foo",
            body: "bar",
            userId: 1
            }).then(() => console.log("Then called"));
        }}
        >
        Click
        </div>
    );
    }}
</Mutate>

There are no throw errors and loading turns from false -> true -> false indicating an operation took place.

Not sure if the operation is being aborted early for some reason, or is throwing an error that is not being caught by the Mutate function.

NOTE: You can use the .finally method on the promise but it is unclear if this is intended behavior in the docs

Expected behavior
According to the docs, "Each mutation returns a promise...", which to me indicates that I should be allowed to use the .then function on the returned promise.

Screenshots
N/A

Desktop (please complete the following information):

  • OS: Windows 10 1803
  • Browser: Firefox
  • Version: 66.0.5 (64-bit)

Additional Note
Even the docs are using the .then function on the mutate function so I believe this is definitely a bug
See here

onClick={() => delete(movie.id).then(() => dispatch('DELETED'))}

Also a side note, delete is a reserved word in JS and that line will result in a compile error

@TejasQ
Copy link
Contributor

TejasQ commented May 15, 2019

Hey @ajhenry! Thanks for reporting this! It’s almost midnight in Germany. We will for sure look at this in the morning.

Appreciate your using our tools!

@fabien0102
Copy link
Contributor

Indeed the error is not correctly propagated in Mutate (https://github.com/contiamo/restful-react/blob/master/src/Mutate.tsx#L181), the trick is to add a .catch statement. In your snippet you just have a 404 error for example. I've spotted this bug yesterday during my useMutate implementation but you was quicker than me for reporting 😉

https://codesandbox.io/embed/competent-lalande-z3jsk?fontsize=14

Good catch for the example in the documentation 😅 using delete is quite stupid ^^

fabien0102 added a commit that referenced this issue May 16, 2019
fabien0102 added a commit that referenced this issue May 16, 2019
@TejasQ
Copy link
Contributor

TejasQ commented May 16, 2019

Published v7.6.2 under next.

npm install restful-react@next should fix this for you now, @ajhenry. Please let us know if it works and we'll close the issue.

@TejasQ TejasQ reopened this May 16, 2019
@ajhenry
Copy link
Contributor Author

ajhenry commented May 16, 2019

@TejasQ Yes I can confirm that this PR fixed the issue with the error callback.

One other thing I did notice is that when issuing a PUT verb, it is common to get back a 204 response code (No Content), but it seems the fetch response handler still tries to call .json() on it anyway, resulting in the data section of the returned result with this: JSON.parse: unexpected end of data at line 1 column 1 of the JSON data.

This is more of a nitpick issue though, but thank you for the fix!

@ajhenry ajhenry closed this as completed May 16, 2019
@TejasQ
Copy link
Contributor

TejasQ commented May 16, 2019

Woah. Thanks for that! We'll fix that too!

@fabien0102
Copy link
Contributor

Time to add a new unit test! Thanks for the feedback 💯

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants