New generator: typescript-redux-query #3824

petejohansonxo · 2019-09-03T18:24:40Z

PR checklist

Read the contribution guidelines.
Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh, ./bin/openapi3/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\. If contributing template-only or documentation-only changes which will change sample output, be sure to build the project first.
Filed the PR against the correct branch: master, 4.1.x, 5.0.x. Default: master.
Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

This PR creates a new TypeScript generator that generates functions for consuming APIs using the redux-query library. In particular, redux-query operates on the concept of a QueryConfig that is an object structure that describes a query/mutation, and then the redux-query library then handles actually making the API requests, managing state of requests in Redux, and processing responses via the transform and update callbacks.

Given that, the functions exported from this generator don't actually call the operation, but instead generate a QueryConfig that describes an API request, and that can be passed to redux-query library from there.

Authentiation

The general recommendation for auth of redux-query requests is not to add the auth details "inline" in the QueryConfig objects, but instead use a Redux middleware to update the passed in Actions and update the QueryConfig w/ the necessary auth headers, etc.

To support this, instead of "inlining" the authentication bits, we instead generate code to include in the meta property of the QueryConfig a description of the requested authenticate type for the particular config. Custom middleware can be written by consumers of the generated code to inspect this meta-info at runtime and add the correct auth headers, to the QueryConfig objects before they get passed into the redux-query library where the requests are actually sent to the API endpoints.

My first generator and contribution to this project, so if I missed anything, please let me know.

Thanks!

CC: @TiFu (2017/07) @taxpon (2017/07) @sebastianhaas (2017/07) @kenisteward (2017/07) @Vrolijkx (2017/09) @macjohnny (2018/01) @nicokoenig (2018/09) @topce (2018/10) @akehir (2019/07)

wing328 · 2019-09-04T07:39:22Z

...ator/src/main/java/org/openapitools/codegen/languages/TypeScriptReduxQueryClientCodegen.java

@@ -0,0 +1,292 @@
+/*
+ * Copyright 2018 OpenAPI-Generator Contributors (https://openapi-generator.tech)
+ * Copyright 2018 SmartBear Software


Please remove line 3 as per https://opensource.stackexchange.com/questions/7300/copyright-notice-in-the-file-header-apache-v2-license/7301#7301

@wing328 Since this was copied from the typescript-fetch version as a starting point, does this still hold true?

I don't know. I'm not an expert in software licensing.

macjohnny · 2019-09-09T06:19:32Z

bin/typescript-redux-query-petstore-with-npm-version.sh

@@ -0,0 +1,32 @@
+#!/bin/sh
+
+SCRIPT="$0"


please add a windows version of this script in bin/windows

Done and pushed. Haven't tested them yet, I don't have easy access to a windows machine/VM currently.

macjohnny · 2019-09-09T06:22:00Z

...ator/src/main/java/org/openapitools/codegen/languages/TypeScriptReduxQueryClientCodegen.java

+            return "{ [key: string]: " + this.getTypeDeclaration(inner) + "; }";
+        } else if (ModelUtils.isFileSchema(p)) {
+            return "Blob";
+        } else if (ModelUtils.isBinarySchema(p)) {


overriding with a custom --type-mappings date=string,date-time=string would not work here I guess, see also #3842

Exploring this. I can definitely see the allure of avoiding the JS Date and leveraging something like date-fns or moment.js instead.

Adding that parameter definitely doesn't work, and still produce Date typed properties on the various models.

I would ask if this is in scope of this initial PR, or if you'd be ok w/ merging without this pieces working, and then a follow up PR to add this ability.

Its ok, I just wanted to point out a potential problem.

Yeah, it definitely is, and one I would like to see addressed in the generator at some point.

macjohnny · 2019-09-09T06:24:38Z

modules/openapi-generator/src/main/resources/typescript-redux-query/apis.mustache

+    {{^isListContainer}}
+    if (requestParameters.{{paramName}} !== undefined) {
+        {{#isDateTime}}
+        queryParameters['{{baseName}}'] = (requestParameters.{{paramName}} as any).toISOString();


add null-check

I'm not sure I'm following this request...

queryParameters gets initialized on line 52 if we're inside this {{#queryParameters}} block.

The call to toISOString() only occurs inside the {{#isDateTime}} check.

Is this a risk for "non-required DateTime parameters" that at the TS level get typed as foo?: Date and about case # 2 here?

requestParameters.{{paramName}} could be null or undefined, so it would be better to have

if (requestParameters.{{paramName}} !== undefined && requestParameters.{{paramName}} !== null) {

macjohnny · 2019-09-09T06:24:45Z

modules/openapi-generator/src/main/resources/typescript-redux-query/apis.mustache

+        {{/isDateTime}}
+        {{^isDateTime}}
+        {{#isDate}}
+        queryParameters['{{baseName}}'] = (requestParameters.{{paramName}} as any).toISOString().substr(0,10);


add null-check

macjohnny · 2019-09-09T06:26:08Z

modules/openapi-generator/src/main/resources/typescript-redux-query/apis.mustache

+
+    {{/consumes}}
+    {{#consumes.0}}
+    headerParameters['Content-Type'] = '{{{mediaType}}}';


support for multiple media types could be added, see #3849

Do you mean around providing a specific Accept header here? It seems like this code producing the Content-Type for the request body generated is working as expected, and matches the code in #3849.

ah, yes, I was talking about the Accept header.

macjohnny · 2019-09-09T06:31:43Z

If you want to add a simple integration test to avoid regressions, see e.g.

openapi-generator/pom.xml

Line 1074 in f2fe4fc

<module>samples/client/petstore/typescript-angular-v7-provided-in-root</module>

https://github.com/OpenAPITools/openapi-generator/blob/master/samples/client/petstore/typescript-angular-v7-provided-in-root/pom.xml

https://github.com/OpenAPITools/openapi-generator/tree/master/samples/client/petstore/typescript-angular-v7-provided-in-root/tests/default

macjohnny · 2019-09-09T10:42:44Z

@petejohansonxo do you want to add yourself to the typescript technical committee (https://github.com/OpenAPITools/openapi-generator#62---openapi-generator-technical-committee) to keep an eye on future changes of the typescript-redux-query generator?

macjohnny · 2019-10-09T08:43:20Z

@petejohansonxo any update here?

petejohansonxo · 2019-10-09T09:59:23Z

@macjohnny Sorry for not getting back to this! I appreciate the review, and will work on addressing the comments this week.

micahcraig · 2019-10-16T21:37:22Z

Hi, apologies for barging in, but I came across this PR just as I was about to give up looking for a good way to integrate Redux with a Swagger specified API, so I'm very excited about it!

Unfortunately, I am running in to some errors while trying to build the generated sample:

mcraig43@C02TJ329HF1R:[~/projects/openapi-generator/samples/client/petstore/typescript-redux-query/builds/default] (generators/typescript-redux-query *)$ npm install

> @openapitools/typescript-redux-query-petstore@1.0.0 prepare /Users/mcraig43/projects/openapi-generator/samples/client/petstore/typescript-redux-query/builds/default
> npm run build


> @openapitools/typescript-redux-query-petstore@1.0.0 build /Users/mcraig43/projects/openapi-generator/samples/client/petstore/typescript-redux-query/builds/default
> tsc

node_modules/redux-query/index.d.ts:2:65 - error TS2307: Cannot find module 'redux'.

2   import { Action, AnyAction, Middleware, Reducer, Store } from 'redux';
                                                                  ~~~~~~~

node_modules/redux-query/index.d.ts:257:78 - error TS2744: Type parameter defaults can only reference previously declared type parameters.

257   export interface ReduxQueryDispatch<A extends AnyAction = ReduxQueryAction<TEntities>, TEntities = Entities> {
                                                                                 ~~~~~~~~~

The first error is easily resolved by adding redux to the dependencies in package.json.

I'm coming fresh to Typescript, so I'm less clear what can be done about the second error. I suspect there's some configuration I could be missing or another dependency I should install, but I'm not sure what or who.

If there's anything else I should try, or any way I could be of help in testing, please let me know! Thanks

macjohnny · 2019-10-17T05:36:48Z

concerning

node_modules/redux-query/index.d.ts:257:78 - error TS2744: Type parameter defaults can only reference previously declared type parameters.

257   export interface ReduxQueryDispatch<A extends AnyAction = ReduxQueryAction<TEntities>, TEntities = Entities> {

maybe this could be avoided by switching the two generic parameters, i.e.

 export interface ReduxQueryDispatch<TEntities = Entities, A extends AnyAction = ReduxQueryAction<TEntities>> {

micahcraig · 2019-10-24T16:28:55Z

@macjohnny your suggested change on node_modules/redux-query/index.d.ts does fix the compilation error, but that file, obviously, is part of an upstream dependency. The dependency, redux-query, does compile successfully without modification in isolation.

@petejohansonxo Any other suggestions on getting through the tsc step with redux-query?

Thanks again!

petejohansonxo · 2019-10-30T20:38:34Z

Yeah, that typescript error is an issue with the original TS generics improvements I'd sent over to the upstream of redux-query. I'm working on a fix for that upstream now.

petejohansonxo · 2019-10-30T20:45:18Z

Upstream PR that should address this: amplitude/redux-query#155

micahcraig · 2019-11-06T20:01:39Z

With the compilation issues squared away, I've been able to begin integrating the generated client in my project, which is awesome and has allowed me very quickly start sending requests to my API. Thank you!

I have a couple of usage questions that might be more obvious to me if I'd spent more time with redux-query and/or TypeScript, so please forgive me if these are naive.

We have a few different instances of our API for different environments which we enumerate in the 'servers' section of the spec (Open API 3.0). I see that the first server is reflected in the generated runtime.ts as BASE_PATH, but I don't see BASE_PATH referenced anywhere. Is there a good way to prepend the base path onto QueryConfig.url at runtime, or should this be done in the templates and generate time?
It seems as though I need to provide transform and update functions in my requestConfig in order for my request results to end up in the Redux store. I appreciate this level of flexibility, but my expectation was that I would be able to invoke the functions exported from .../apis/ directly and that request results would automatically update the store. Am I missing a DRYer path for leveraging these functions, or are my expectations out of line with generator's intended usage?

Thanks again for all your hard work on this. Really appreciate the effort!

petejohansonxo · 2019-11-06T20:25:34Z

@micahcraig Thanks for the testing!

I haven't had a need for the server selection at all in our uses. Some options might be to add it as extra information in the meta of the passed in requestConfig, and then use a middleware to update the QueryConfig<T> url property to prepend the server URL before the request is handled by the redux-query middleware.
Based on the way redux-query works, the most flexible approach to me seemed to be generating "QueryConfig generator functions" that would generate a QueryConfig<T> for each published operation that would:
1. Strongly type the input types to generate the config,
2. Allow strongly typed processing of the responses, which in redux-query land means an update and transform that account for the schema of the operation response type.

It didn't seem possible to have the generator presume where in the entities to place the responses, nor what update approach should be taken, so it requires the consumer of the generated generator functions to provide that.

If someone has some ideas for how you might automate those decisions in the generator, as maybe an optional mode, I'd be open to it, but it wasn't something we needed.

Our usage so far has been to invoke the generator, then do something like (pseudo-code):

compose(
  connectRequest(props => operationGeneratorFunction({ input_arg: props.arg }, {
    transform: (response: GeneratedResponseTypeSchema) => {
      return { entityProp: response.values };
    }
    update: {
      entityProp: secondArgFunctionSoWeUseNextValue
    }
  })
)(SomeComponent);

So we can benefit from the strong typing for input + output (transform), and we can also use the generator functions to instead pass to the useRequest/useMutation hook or pass the results to dispatch(mutateAsync(someGeneratorFunction())).then(foo => console.log(foo)).

macjohnny · 2019-11-14T11:31:46Z

@petejohansonxo do you consider this ready to be merged so that an initial version can be released?

macjohnny · 2019-11-14T11:59:39Z

@petejohansonxo can you please merge the current master?

petejohansonxo · 2019-11-14T17:45:39Z

@macjohnny Merged up with master. I would consider the basics here ready for testing. We're using this successfully w/ my latest version at work.

macjohnny · 2019-11-14T17:53:17Z

If the CI completes successfully I will merge it

micahcraig · 2019-11-15T22:15:41Z

Got a bit further in my testing, and noticing an issue with generated mutations with multiple optional parameters. In that case, the generated Raw function gets:

    let queryParameters = null;

    queryParameters = {};

    if (requestParameters.startDate !== undefined) {
        queryParameters['start_date'] = (requestParameters.startDate as any).toISOString();
    }

    queryParameters = {};

    if (requestParameters.endDate !== undefined) {
        queryParameters['end_date'] = (requestParameters.endDate as any).toISOString();
    }

    queryParameters = {};

    if (requestParameters.types) {
        queryParameters['types'] = requestParameters.types;
    }

The upshot of which is that only the last included optional parameter is actually included on the request.

petejohansonxo · 2019-11-16T04:22:38Z

@micahcraig Thanks for the testing. I just pushed a fix for this in a37b9a7

wing328 · 2019-12-02T09:39:50Z

@petejohansonxo thanks for the PR, which has been included in the v4.2.2 release: https://twitter.com/oas_generator/status/1201432648544972800

wing328 reviewed Sep 4, 2019

View reviewed changes

macjohnny approved these changes Sep 9, 2019

View reviewed changes

wing328 added Client: TypeScript Enhancement: New generator labels Sep 16, 2019

macjohnny mentioned this pull request Oct 31, 2019

Fix generic defaulting w/ middleware types amplitude/redux-query#155

Merged

petejohansonxo force-pushed the generators/typescript-redux-query branch from 7dc0534 to 5288926 Compare October 31, 2019 19:42

petejohansonxo added 7 commits November 14, 2019 12:41

Initial work on a redux-query client generator.

0d86ac3

Update to match the required redux-query version w/ fixed TS defs.

4b69f07

Fixes for query parameters, add samples.

c519b58

Add windows versions of redux-query scripts.

75b3ba6

Fix for form data handling.

648e802

Initial work on multiple inheritance support.

62b3556

Tweaks for discriminator usage + from/to JSON.

337a0c2

petejohansonxo force-pushed the generators/typescript-redux-query branch from 5288926 to 337a0c2 Compare November 14, 2019 17:42

Fix initialization of queryParameters if needed.

a37b9a7

petejohansonxo added 2 commits November 18, 2019 22:29

Copy over more QueryConfig values into the final generated QueryConfig.

0424586

Fix for imports that are combined types.

95f592b

macjohnny added this to the 4.2.2 milestone Nov 22, 2019

macjohnny merged commit 70954ca into OpenAPITools:master Nov 22, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New generator: typescript-redux-query #3824

New generator: typescript-redux-query #3824

petejohansonxo commented Sep 3, 2019 •

edited

Loading

wing328 Sep 4, 2019

petejohansonxo Sep 4, 2019

wing328 Sep 5, 2019

macjohnny Sep 9, 2019

petejohansonxo Oct 9, 2019

macjohnny Sep 9, 2019

petejohansonxo Oct 9, 2019

macjohnny Oct 9, 2019

petejohansonxo Oct 9, 2019

macjohnny Sep 9, 2019

petejohansonxo Oct 9, 2019 •

edited by macjohnny

Loading

macjohnny Oct 9, 2019

macjohnny Sep 9, 2019

macjohnny Sep 9, 2019

petejohansonxo Oct 9, 2019

macjohnny Oct 9, 2019

macjohnny commented Sep 9, 2019

macjohnny commented Sep 9, 2019

macjohnny commented Oct 9, 2019

petejohansonxo commented Oct 9, 2019

micahcraig commented Oct 16, 2019

macjohnny commented Oct 17, 2019

micahcraig commented Oct 24, 2019

petejohansonxo commented Oct 30, 2019

petejohansonxo commented Oct 30, 2019

micahcraig commented Nov 6, 2019

petejohansonxo commented Nov 6, 2019

macjohnny commented Nov 14, 2019

macjohnny commented Nov 14, 2019

petejohansonxo commented Nov 14, 2019

macjohnny commented Nov 14, 2019

micahcraig commented Nov 15, 2019

petejohansonxo commented Nov 16, 2019

wing328 commented Dec 2, 2019

		@@ -0,0 +1,32 @@
		#!/bin/sh

		SCRIPT="$0"

New generator: typescript-redux-query #3824

New generator: typescript-redux-query #3824

Conversation

petejohansonxo commented Sep 3, 2019 • edited Loading

PR checklist

Description of the PR

Authentiation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

petejohansonxo Oct 9, 2019 • edited by macjohnny Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

macjohnny commented Sep 9, 2019

macjohnny commented Sep 9, 2019

macjohnny commented Oct 9, 2019

petejohansonxo commented Oct 9, 2019

micahcraig commented Oct 16, 2019

macjohnny commented Oct 17, 2019

micahcraig commented Oct 24, 2019

petejohansonxo commented Oct 30, 2019

petejohansonxo commented Oct 30, 2019

micahcraig commented Nov 6, 2019

petejohansonxo commented Nov 6, 2019

macjohnny commented Nov 14, 2019

macjohnny commented Nov 14, 2019

petejohansonxo commented Nov 14, 2019

macjohnny commented Nov 14, 2019

micahcraig commented Nov 15, 2019

petejohansonxo commented Nov 16, 2019

wing328 commented Dec 2, 2019

petejohansonxo commented Sep 3, 2019 •

edited

Loading

petejohansonxo Oct 9, 2019 •

edited by macjohnny

Loading