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

useMutation first parameter type error #1175

Closed
doroz0 opened this issue Apr 27, 2023 · 4 comments · Fixed by #1191
Closed

useMutation first parameter type error #1175

doroz0 opened this issue Apr 27, 2023 · 4 comments · Fixed by #1191

Comments

@doroz0
Copy link

doroz0 commented Apr 27, 2023

Used libraries

core, jsonapi, swr

Library version(s)

@datx/core 2.5.1, @datx/jsonapi 2.5.1, @datx/swr 2.5.1

Sample API response (if relevant)

No response

Environments with the issue

All

Environments without the issue

No response

Current behavior

export const createPost = (client: JsonapiSwrClient, body: string) => {
  const model = new Post({ body });
  const url = getModelEndpointUrl(model);
  const data = modelToJsonApi(model);

  return client.request<Post>(url, "POST", { data });
};

const [create, { status: createStatus }] = useMutation(createPost, { // <---------------
  onSuccess: async () => {
    const input = inputRef.current;
    if (input) input.value = "";
    mutate();
  },
});

image

Expected behavior

No errors

Reproduction steps

You can try it out at https://github.com/doroz0/soc-net-clone-nextjs, just remove the `as any` (https://github.com/doroz0/soc-net-clone-nextjs/blob/main/src/pages/index.tsx#L24)
@kristian240
Copy link

The issue with this error is that createPost method is using client.request. The return type of client.request is a Response which has either to one or to many relationship. With @datx/swr we have created 2 new responses - SingleResponse and CollectionResponse. To use them, change the client.request to client.requestSingle or client.requestCollection and the type will be correct.

I tried to override the request method to enhance the DX with this PR but there are other issues that occurred and that are not that easy to solve quickly since a lot of internals are using client.request and we are doing castings on another level.

Next idea is that to create a eslint rule to solve throw an error when developer is using client.request in a project that has @datx/swr installed, what do you think about that?

One other solution is to check if a client.request has been called with fetchOptions: { Response: CollectionResponse/SingleResponse } which will indicate that this is an okay call to make and throw an error if called without - but that does not seem right and this is also causing us to ship more code just to fix the typing issues.

@isBatak @DarkoKukovec @doroz0 what you say on all of this?

@DarkoKukovec
Copy link
Member

I didn't notice many places where we're using the request method internally. I changed this behavior and updated a test that failed, and everything seems to be working fine?
94978ed

@kristian240
Copy link

Thanks a lot, @DarkoKukovec :D Would this be considered a breaking change if we are throwing errors?

@kristian240 kristian240 mentioned this issue Jul 19, 2023
5 tasks
@DarkoKukovec
Copy link
Member

Yeah, I guess so... 🤔
#1179 is also still pending because it is a breaking change

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

Successfully merging a pull request may close this issue.

4 participants