rtk-incubator / rtk-query-codegen

93 stars 13 forks source link

Usage with OpenAPI-Generator javascript SDK #58

Open ybelenko opened 3 years ago

ybelenko commented 3 years ago

Maybe it's just me as a member of OpenAPI-Generator project, but I understood this package completely wrong from the first sight. I thought it's just a wrapper of some TypeScript generator from mentioned repo, but seems like you generate everything yourself?

If so, could you add small notice in main README that you don't use any generation libraries as a core. I think it might be important for developers who would like to use already generated API clients. Beside client SDKs from OpenAPI-Generator might be more stable and enhanced at current point.

phryneas commented 3 years ago

I'm a bit at a loss - why would we generate a full external API client when this is a code generator for an api client library? Especially the "more advanced" part - I'm really not sure about this - have you taken a look at the docs?

This is primarily just to create an endpoint declaration and types that users can then enhance with hand-written invalidation rules, lifecycle events and data transformation logic, so the result would look like https://github.com/rtk-incubator/rtk-query-codegen/blob/next/test/fixtures/generated.ts#L2-L79

markerikson commented 3 years ago

I'm kinda confused. @ybelenko , can you clarify what your specific concerns are, and what you're asking for here?

ybelenko commented 3 years ago

I already have generated front client SDK with models and helper classes which looks like:

var OpenApiPetstore = require('open_api_petstore');

var api = new OpenApiPetstore.AnotherFakeApi()
var client = new OpenApiPetstore.Client(); // {Client} client model
api.call123testSpecialTags(client).then(function(data) {
  console.log('API called successfully. Returned data: ' + data);
}, function(error) {
  console.error(error);
});

Ref to full OpenAPI Petstore source samples.

So, it feels natural to customize baseQuery and use that SDK while RTK doc recommends to use your package since API based on OpenAPI schema.

My questions is, when I use both OpenAPI schema and OpenAPI generator to produce front client SDK should I also use your package?

phryneas commented 3 years ago

If you bring your own API client, you would usually use fakeBaseQuery and queryFn instead of a baseQuery & query combination. We can't generate code for external api clients as they bring their own assumptions, usually mutably hold internal state and do a lot more stuff we cannot really take into account.

The API client you linked to seems to be not very suited for storage in Redux as it generates model classes for everything and we recommend not to store non-serializable values as classes in a redux store.

Essentially everything returned from there would have to be converted to an object before storing it in Redux, which kinda makes it pretty pointless I think

ybelenko commented 3 years ago

The API client you linked to seems to be not very suited for storage in Redux as it generates model classes for everything and we recommend not to store non-serializable values as classes in a redux store. Essentially everything returned from there would have to be converted to an object before storing it in Redux, which kinda makes it pretty pointless I think

Yes, you right, but generated library returns model and response object in each endpoint call. Usually I put response body into Redux state instead of model. While models seems useful when it request param not a response.

We can't generate code for external api clients as they bring their own assumptions, usually mutably hold internal state and do a lot more stuff we cannot really take into account.

Of course, I get it. So for me it's choice between your codegen and OpenAPI-Generator codegen. Should I wrap client SDK with fakeBaseQuery or generate your endpoint declarations. RTK-Query is kinda new tool to me, while you know it perfectly I guess.

phryneas commented 3 years ago

fakeBaseQuery would just prevent you from accidentally call it. You would essentially write your endpoints by hand and do something like

x: builder.addQuery({
  async queryFn(arg){
    try {
      const result = externalApi.callX(arg)
      return { data: result }
    } catch (e) {
      return { error: error.message }
    }  
  }
})

for each of your endpoints.

Of course, you can write a little helper for yourself that will cut down on code quite a bit, but you would essentially be duplicating your generated api client. This is more an escape hatch for something like firebase where a full-fledged client exists that is not easy to replicate just with http calls. For normal http calls, I personally would probably use this code generator, although I admit the code generation might need some work for edge cases. Too many things to do at the same time at the moment :/

In the end, I'd try to generate the code once and see if it works for your api :)

ybelenko commented 3 years ago

In the end, I'd try to generate the code once and see if it works for your api :)

Is it typescript only? It's hard to read for me and I think typescript is overkill for a small projects. I use vanilla React JSX where possible.

And what http client this package uses by default? Is it fetchBaseQuery? OpenAPI-Generator uses superagent http client for a long time already and maybe there are edge cases when it's complicated to use basic fetch wrapper.

phryneas commented 3 years ago

You can throw the generated TypeScript into something like the TypeScript playground to just get the compiled JavaScript (which is pretty much stripping away the type annotations).

Personally I would use TypeScript before I even add a second file to a project.

Especially when just writing

const {data} = useSomeQuery()

and then getting full autocomplete over what data the API returns when you write data.

would be reason enough for me personally to go TypeScript.

superagent, axios and co are all from a time when browsers handled API requests differently with XHRRequests and whatnot, but they mostly have overlived themselves since fetch is part of browser APIs. (axios is 4.6kB and redaxios, which is essentially equivalent but built on fetch is less than 1kb)

superagent is over 10kb big, which could probably be implemented in 1-2kb if it would not have to be backwards compatible. fetchBaseQuery is 0.8kb and I think it should handle everything 90% of users need and also can easily be wrapped for more functionality.

ybelenko commented 3 years ago

I almost forgot that OpenAPI-Generator contains long list of TypeScript generators:

Full list of client samples here

I'm not here to advertise generator project, but want to describe my doubts as developer and user. The biggest advantage of OpenAPI Schema(formerly Swagger API) to me was ability to generate client libraries for any language. Today I need to accept that client generating is useless regarding Redux and maybe it's easier to proceed with your small package.

I've found one advantage of client SDK recently, we use mustaches for templates, so it's really easy to overwrite model template and generate new build.

phryneas commented 3 years ago

Actually, the use of mustache made me not go for the code generator - I honestly evaluated it (and dozens others) before building this.

For a guarantee of building correct TypeScript code, I did not want to rely on some string concatenation, but build a TypeScript AST and write that to a TypeScript file using the compiler itself.

Also, the goal of this in combination of RTKQ is that you should never need to touch your templates, or the generated files. Enhancing the generated files should be done according to https://redux-toolkit.js.org/rtk-query/usage/code-generation - in an external file, without any change to the generated code.

evandegr commented 3 years ago

After reading this thread with interest wanted to ask a somewhat related question.

I have an openapitools.json that I'm using to do my node express and typescript generation right now:

{
  "$schema": "node_modules/@openapitools/openapi-generator-cli/config.schema.json",
  "spaces": 2,
  "generator-cli": {
    "version": "5.1.1",
    "generators": {
      "frontend_v1.0": {
        "generatorName": "typescript-fetch",
        "output": "#{cwd}/client/gen/v1.0/#{name}",
        "glob": "api/*.yaml"
      },
      "node_v1.0": {
        "generatorName": "nodejs-express-server",
        "output": "#{cwd}/server/gen/v1.0/#{name}",
        "glob": "api/*.yaml"
      }
    }
  }
}

With your system, I'd probably just need to remove the typescript-fetch here, have a script that also would run the rtk generation alongside the openapi node-express-server? Sorry if this is a dumb question, I'm new to a lot of the tools here. Do you guys also generate server code and use in conjunction with RTK generator?

phryneas commented 3 years ago

You are right, if you want to generate server code, you have to use both tools for that.

evandegr commented 3 years ago

For anyone interested in using plain JS: I created a script in my package.json:

"generate-client-api": "rm -rf client/src/gen; mkdir client/src/gen; npx @rtk-incubator/rtk-query-codegen-openapi --hooks api/inventory.yaml > client/src/gen/rtk-openapi.ts; npx -p typescript tsc client/src/gen/rtk-openapi.ts"
bkempe commented 3 years ago

@phryneas One advantage of having the generated API calls generated separately from the RTKQuery concerns might be that the generated client can be used from other part of the application, e.g. from some custom Redux middleware code. Are there any other approaches at the moment to do this?

phryneas commented 3 years ago

This is written in Redux. It is literally dispatching actions and storing the results in the store from where you can retrieve them. (This version you can also await mutations in middleware, from the next version on you'll also be able to await queries since it will remove a possible race condition.) You do not need to use the hooks.

We have a svelte example in the docs, there are people using this with angular and vue. You don't need an extra client for this, especially when using Redux. https://redux-toolkit.js.org/rtk-query/usage/usage-without-react-hooks

bkempe commented 3 years ago

Great to hear about the upcoming await query feature! So if I use this from middleware to make one request, I still have to call unsubscribe though, right?

phryneas commented 3 years ago

Yes, as soon as you are not interested in the cache result any more and it can be collected.

bkempe commented 3 years ago

So once the await queries lands, the pattern to make requests in custom middleware would be something like this?

const { result, data } = await dispatch(api.endpoints.getPosts.initiate())
result.unsubscribe()
// use data
phryneas commented 3 years ago

Yup. That should also already work similarly in the current version of RTKQ, by selecting the result from the store after awaiting like this:

await dispatch(api.endpoints.getPosts.initiate())
const { result, data } = api.endpoints.getPosts.select()(getState())
result.unsubscribe()

Atm, there is just one race condition which will be resolved in the next version of RTQK:

If you initiate the same endpoint with the same queries twice in very short succession

bkempe commented 3 years ago

@phryneas Maybe this was just an example, but calling select()would recreate the selector every time and not provide the benefit of caching, correct?

Similarly, how do I reuse a selector specified in selectFromResult somewhere else in the application? Is the usage in this example correct? It seems like it would create 3 selectors.

const firstPostFinder = (data: Posts[] | undefined) => data?.[0]

1) ComponentA:

  const { firstPost } = api.useGetPostsQuery(undefined, {
    selectFromResult: ({ data }) => ({
      firstPost: firstPostFinder(data),
    }),
  })

2) ComponentB uses the same code ^ 3) Another selector in the application, e.g.

const getPostsSelector = api.endpoints.getPosts.select()
const firstPostSelector = createSelector(getPostsSelector, firstPost)
phryneas commented 3 years ago

Going very far off topic, but yes, api.endpoints.getPosts.select() is like a call to createSelector and should be handled similarly - save the result somewhere outside of component lifecycle.

About selectFromResult: either you create a memoized selector to pass in or you just don't create a new object in the selector that is deeper than one level - the result of selectFromResult is compared shallowly. So if firstPostFinder is a cheap operation (which in your case it is) that does not create a new object on every call (which is also the case here), doing what you do is perfectly fine.