Open api import #28

fabien0102 · 2018-08-14T12:55:08Z

Why

The idea is to be able to give any open-api specs file (in json or yaml) and generate fully typed restful-react components 🎉

Usage

$ npm i -g restful-react
$ restful-react import open-api.yaml --output myAPI.d.ts

Output (current)

/* Generated by restful-react */

import qs from \\"qs\\";
import React from \\"react\\";
import { Get, GetProps, Mutate, MutateProps } from \\"restful-react\\";

export type Omit<T, K extends keyof T> = Pick<T, Exclude<keyof T, K>>;

export type Pet = NewPet & {id: number};

export interface NewPet {name: string; tag?: string}

export interface Error {code: number; message: string}

export type FindPetsProps = Omit<GetProps<Pet[], Error>, \\"path\\"> & {tags?: string[]; limit?: number};


export const FindPets = ({tags, limit, ...props}: FindPetsProps) => (
  <Get<Pet[], Error>
    path={\`/pets?\${qs.stringify({tags, limit})}\`}
    base=\\"http://localhost\\"
    {...props}
  />
);


export type AddPetProps = Omit<MutateProps<Error, Pet, NewPet>, \\"path\\">;


export const AddPet = (props: AddPetProps) => (
  <Mutate<Error, Pet, NewPet>
    path={\`/pets\`}
    base=\\"http://localhost\\"
    {...props}
  />
);


export type FindPetByIdProps = Omit<GetProps<Pet, Error>, \\"path\\"> & {id: number};


export const FindPetById = ({id, ...props}: FindPetByIdProps) => (
  <Get<Pet, Error>
    path={\`/pets/\${id}\`}
    base=\\"http://localhost\\"
    {...props}
  />
);


export type DeletePetProps = Omit<MutateProps<Error, void, void>, \\"path\\"> & {id: number};


export const DeletePet = ({id, ...props}: DeletePetProps) => (
  <Mutate<Error, void, void>
    path={\`/pets/\${id}\`}
    base=\\"http://localhost\\"
    {...props}
  />
);

Usage inside the client project (current)

import { FindPetById } from "./myAPI"

const MyComponent = () =>
  <FindPetById id={42}>
   {(data) => /* I have type definition here!!! */}
  </FindPetById>

Todo

As the above example show, everything open-api definition pattern are not following yet. I will update this example during the API evolution 😉

contiamo-ci · 2018-08-14T12:56:42Z

	Warnings
⚠️	❗ Big PR

Generated by 🚫 dangerJS

fabien0102 · 2018-08-28T07:51:07Z

OpenAPI import / generator

TejasQ

This looks so good! It will be a huge value add for people using OpenAPI!

TejasQ · 2018-08-28T08:43:09Z

bin/restful-react-import.js

+
+const program = require("commander");
+const { join } = require("path");
+const importOpenApi = require("../dist/import-open-api");


This file is in bin and a part of the restful-react build, so he take the generated .js file from dist ;)

This is quite confusing. Can we think of a clearer way to express this in code?

TejasQ · 2018-08-28T08:45:14Z

scripts/import-open-api.ts

+  const componentName = pascal(operation.operationId!);
+
+  const isOk = ([statusCode]: [string, ResponseObject | ReferenceObject]) => statusCode.toString().startsWith("2");
+  const isError = ([statusCode]: [string, ResponseObject | ReferenceObject]) => !statusCode.toString().startsWith("2");


Careful, .startsWith("3") is not an error.

TejasQ · 2018-08-28T08:45:43Z

scripts/import-open-api.ts

+  const schema = importSpecs(path);
+
+  if (!schema.openapi.startsWith("3.0")) {
+    throw new Error("This tools can only parse open-api 3.0.x specifications");


We should support older versions too.

Since a lot of tools can make the swagger -> open-api migration, it's not really a problem 😉

mshustov

Hey, sorry for chiming in. Talked to @TejasQ about contract validation and found that I'm also interested in the outcome of this work. Ping if I can help somehow

mshustov · 2018-09-19T16:36:18Z

scripts/import-open-api.ts

+}
+`;
+        case "array":
+          return `export type ${pascal(name)} = ${getArray(schema)}`;


shouldn't be covered by getScalar?

I really need to add a bunch of integration tests to be sure about this, but it's probably true 👍

mshustov · 2018-09-19T16:37:26Z

scripts/import-open-api.ts

+      } else {
+        if (res.content && res.content["application/json"]) {
+          const schema = res.content["application/json"].schema!;
+          return isReference(schema) ? getRef(schema.$ref) : getScalar(schema);


seems that's a common pattern and could be extracted

function resolveValue(schema){ return isReference(schema) ? getRef(schema.$ref) : getScalar(schema); }

mshustov · 2018-09-19T16:38:39Z

scripts/import-open-api.ts

+${Object.entries(schema.properties || {})
+            .map(
+              ([key, properties]) =>
+                isReference(properties) ? `  ${key}: ${getRef(properties.$ref)}` : `${key}: ${getScalar(properties)}`,


couldn't be transformed into ${key}: ${resolveValue(property)}?

Probably ^^ I introduce getScalar lately and totally forget everything about this implementation

mshustov · 2018-09-19T17:00:50Z

scripts/import-open-api.ts

+const importSpecs = (path: string): OpenAPIObject => {
+  const data = readFileSync(path, "utf-8");
+  const { ext } = parse(path);
+  return ext === ".yaml" ? YAML.parse(data) : JSON.parse(data);


yml seems to be also a valid extension https://en.wikipedia.org/wiki/YAML

mshustov · 2018-09-19T17:02:52Z

scripts/import-open-api.ts

+ * @param responses reponses object from open-api specs
+ * @param componentName name of the current component
+ */
+const getResponseTypes = (responses: Array<[string, ResponseObject | ReferenceObject]>, componentName: string) =>


componentName is not used

My old me probably have a plan about this, but now… 😄 Good catch!

mshustov · 2018-09-21T12:04:39Z

scripts/import-open-api.ts

+ *
+ * @param operation
+ */
+const generateGetComponent = (operation: OperationObject, verb: string, route: string) => {


file is called import-open-api and generateGetComponent consumes it. so consumer shouldn't be declared here. am I wrong?

The entry point is the main function on bottom, and yes I will split this file in smaller, and also add a lot of unit tests! It was just more conveniant like this to iterate 😉 (it's a wip branch)

fabien0102 · 2018-09-21T12:20:50Z

@restrry Thanks for your feedbacks 💯 This PR is totally work in progress, and sadly I don't have any time to work on it… 😞

Ideally I will also split this huge file in smaller one, and add some unit tests for every small parts (when it make sense).

One of the big part missing where you can maybe help, is about the type of the body. Let me add a bit more context:

<Mutation verb="POST">{
  (post) => <Button onClick={() => post({/* the body can't be typesafe for now */})}/>
}</Mutation>

To deal with this I would like to add more generic into Mutation to be able to specify the type of the body. The goal of course is to have this type generated from open-api specs (https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.1.md#request-body-object)

TejasQ · 2018-09-21T18:29:06Z

@restrry to echo what @fabien0102 said, your contributions here would be super helpful – unfortunately we're quite busy developing the product so we need all the help we can get on this project. 😉

mshustov · 2018-09-24T06:21:27Z

np.
is there something that could be helpful to understand the context of the matter better #28 (comment) ?

TejasQ · 2018-09-24T08:42:09Z

@fabien0102 has all the info. He'll respond today.

fabien0102 · 2018-09-24T09:16:01Z

@restrry Logically everything in the PR description (#28 (comment)), the idea is to have the best as possible type safe component generated from open-api specs. My current implementation only have the Get component generated and also every types needed for this one.

I really trying to keep the PR todo list and examples up to date (I also add this query params on the todolist).

To be very transparent, the trello card attached:

Really, all relevant information are in the description, the trello card is just for a planning purpose 😉

micha-f · 2018-10-11T12:10:30Z

@fabien0102 This is incredibly cool.

How about adding optional frontend validation of the responses in the frontend (maybe using https://www.npmjs.com/package/yup) as a second step? We could set it up so that you can enable/disable this validation and choose whether validation errors should throw or just console warn. These could be options to the generator script so that you only add this overhead to your code if you really want this.

@peterszerzo You have experience with yup - what are your thoughts?

peterszerzo · 2018-10-11T12:17:14Z

I've had a very good time working with yup so far, it supports pretty much everything with a reasonably friendly API. Not quite https://package.elm-lang.org/packages/elm/json/latest/Json-Decode, but it'll serve us really well :). Making it easy to generate validations from Open API would be a huge safety win for applications, and it is really not that difficult.

(loosely related, I've proposed some restul-react changes that allow the library to work better with yup: #57)

fabien0102 · 2018-10-11T14:10:22Z

@micha-f Very good idea for adding yup! It can be a very nice second step for this generator 💯 We already have talked about this validation layer with @peterszerzo 😉

But first of all, I need to fix the build and test everything in a real project to be sure everything is well following.

mshustov · 2018-10-11T19:13:26Z

I tried to approach the problem in a bit different way. in the chain raw spec --> transformed data --> component declaration the most error-prone layer is a transformation from swagger spec to js objects, so I started looking at off-the-shelf solutions.
https://github.com/swagger-api/swagger-codegen/ - probably the best transformer, but requires Java, could be problematic to maintain consistency between the peer's environment, CI etc.
https://github.com/wcandillon/swagger-js-codegen - ready solution in JavaScript, exists for 4 years, so most likely covers a majority of use cases, not actively supported. trying to figure out the current status. meantime started using the fork https://github.com/mtennoe/swagger-typescript-codegen and implemented Proof of Concept with it (for my work). there is something to improve, so already started contributing in the lib. hope we can use another template engine, but now it supports the only mustache. anyway there is an example just for demo purposes for your use case master...restrry:swagger-gen

TejasQ · 2018-10-12T13:09:52Z

Hey, sorry for chiming in.

@restrry thank you for your contribution! This is definitely not something you need to say sorry for. Ever.

We actually looked into off the shelf solutions as well and, unfortunately could not find much that we could comfortable work with. I think it might be easier to process JSON swagger docs (both, v2 and OpenAPI) using Babel, which we have some experience with for the transformation layer (this is what Babel is for). However, this doesn't quite apply for yaml files and so is possibly not an ideal solution unless we yaml -> json -> babel -> components, which could be either cool or convoluted depending on perspective. Let's see how things develop.

anyway there is an example just for demo purposes for your use case https://github.com/contiamo/restful-react/compare/master...restrry:swagger-gen?expand=1

This link doesn't quite work as expected. Do you have an alternative? 😅

About yup

I think this extra layer of safety is a welcome addition to the feature! Great idea, @micha-f!

mshustov · 2018-10-12T14:30:42Z

updated the link.
regarding babel-plugin - suspect it will be problematic, because babel supposed to be used for code transformation, not data transformation/mapping

TejasQ · 2018-10-12T14:37:56Z

We seem to be mixing up concepts a bit, my apologies. 😅

I was talking more about the static domain of transforming openapi.json to TypeScript interfaces (which is what the tools you've mentioned here do, no?), and not operating on actual data returned from a real-world request. I think you're right – babel would not be an ideal tool for the latter case.

Also, I'm not sure how moustache (or any templating at all) fits into the equation. Could you help me out a bit there? I think I'm missing something...

micha-f · 2018-10-14T18:10:54Z

@fabien0102 Might make sense to check out the swagger-typescript-codegen approach. It seems like a nice way to avoid having to deal with all sorts of open api edge cases.

mshustov · 2018-10-15T07:10:56Z

@TejasQ
data transformation pipeline
raw swagger spec ---with data parser into---> transformed data --- with template engine into---> component declaration
right now swagger-typescript-codegen doesn't allow to separate concerns and provide only one method that transforms spec in component declaration, so you have to use built-in template engine (mustache). shouldn't be hard to fix tho

TejasQ · 2018-10-15T08:27:08Z

shouldn't be hard to fix tho

Oh man I'd love to see a PR from you into this PR 👼

fabien0102 · 2018-10-15T11:51:54Z

Every solutions can work in this case, I must admit that I don't really see the advantage to use mustache or handlebar since we have built-in template string in JavaScript, but this also works.

Just to compare:

{{#tsType}}
{{! must use different delimiters to avoid ambiguities when delimiters directly follow a literal brace {. }}
{{=<% %>=}}
<%#isRef%><%target%><%/isRef%><%!
%><%#isAtomic%><%&tsType%><%/isAtomic%><%!
%><%#isObject%>{<%#properties%>
'<%name%>'<%^isRequired%>?<%/isRequired%>: <%>type%><%/properties%>
}<%/isObject%><%!
%><%#isArray%>Array<<%#elementType%><%>type%><%/elementType%>>|<%#elementType%><%>type%><%/elementType%><%/isArray%><%!
%><%#isDictionary%>{[key: string]: <%#elementType%><%>type%><%/elementType%>}<%/isDictionary%>
<%={{ }}=%>
{{/tsType}}

https://github.com/mtennoe/swagger-typescript-codegen/blob/bfcd0cae2f8ced0dcfafc2153c40f1130cba2fd2/templates/type.mustache#L1-L12

restful-react/src/scripts/import-open-api.ts

Lines 281 to 292 in 019a147

    
           export const generateSchemaDefinition = (schemas: ComponentsObject["schemas"] = {}) => { 
        
             return ( 
        
               Object.entries(schemas) 
        
                 .map( 
        
                   ([name, schema]) => 
        
                     (!schema.type || schema.type === "object") && !schema.allOf && !isReference(schema) 
        
                       ? `export interface ${pascal(name)} ${getScalar(schema)}` 
        
                       : `export type ${pascal(name)} = ${resolveValue(schema)};`, 
        
                 ) 
        
                 .join("\n\n") + "\n" 
        
             ); 
        
           };

The main difference between theses libraries and my implementation is that nobody use yet OpenAPI 3.x and the generation code is not it typescript. I personally really prefer use typescript for this kind of task (autocompletion on OpenAPI specs thanks to openapi3-ts), so very nice to play with 😃

Regarding the edge cases, I'm quite confident, I had the OpenAPI specs opened everytime when I develop this and a lot of unit tests. But I will discover the remaining one during my integration tests with our products 😃

But we can borrow some ideas from these generators:

ignore deprecated endpoint (to be discuss)
use swaggerType.$ref.substring(swaggerType.$ref.lastIndexOf('/') + 1); to resolve $ref (for now we force to have every refs inside #/components/schemas/ as is recommended is the specs https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.1.md#reference-object) (we don't deal with external files for now, as everybody ^^)

Bref, this implementation is working well for now. We have types on every layer and it's quite cool 😎 The only missing piece is the build step, for now you need to update the tsconfig.json with "module": "commonjs", to be able to have a working version.

@restrry If you can try locally this script with your own projects to have a feedback, this can be very helpful.

Procedure to test:

clone the branch
yarn
Update the module key in tsconfig.json -> "module": "commonjs",
yarn build
./lib/bin/restful-react.js import {openapi.yaml} -o {declaration.d.ts}

For now it's OpenAPI 3 only, so you will probably need to convert your specs (I will try to put the convertor directly inside the script if it's possible)

TejasQ · 2018-10-15T12:27:24Z

I don't really see the advantage to use mustache or handlebar since we have built-in template string in JavaScript

Exactly this. I have to say I completely agree and was wondering what it would look like in code, hence my suggesting a PR. I much prefer the latter approach here.

mshustov · 2018-10-15T15:35:52Z

@fabien0102 unfortunately, cannot help with testing, as I don't use restful-react :) just sharing my thoughts about implementation, as I'm working on the same problem for my project (on my free time), so most likely I continue with 3rd part library (swagger-typescript-codegen, for example) so other people could reuse it for their needs.
also, I'd prefer to use swagger@v2.0 because adding another step for transformation into open-api@v3 is just another place for bugs ;)
anyway, I continue to track the progress here, probably can borrow some ideas or help with something. cheers 🍺

TejasQ

Looks amazing so far. I will update the docs a bit when everything's ready to merge. 🚀

TejasQ · 2018-11-29T09:16:32Z

README.md

@@ -1,5 +1,6 @@
 # RESTful React
- [![Greenkeeper badge](https://badges.greenkeeper.io/contiamo/restful-react.svg)](https://greenkeeper.io/)
+
+[![Greenkeeper badge](https://badges.greenkeeper.io/contiamo/restful-react.svg)](https://greenkeeper.io/)


Suggested change

[![Greenkeeper badge](https://badges.greenkeeper.io/contiamo/restful-react.svg)](https://greenkeeper.io/)

README.md

rollup.config.js

TejasQ · 2018-11-29T09:21:14Z

src/Mutate.tsx

@@ -15,7 +15,7 @@ export interface States<TData, TError> {
  error?: GetState<TData, TError>["error"];
 }

-export type MutateMethod<TData> = (data?: string | {}) => Promise<TData>;
+export type MutateMethod<TData, TBody> = (data?: string | TBody) => Promise<TData>;


TData represents a response body so I find TBody a little confusing in this case. Can we name it TRequestBody instead?

TejasQ · 2018-11-29T09:24:04Z

src/scripts/import-open-api.ts

+  SchemaObject,
+} from "openapi3-ts";
+
+// @ts-ignore - no type definition here


Can we polyfill with a .d.ts file and declare module "swagger2openapi"?

I tried… It was just not working, but I will give another try

TejasQ · 2018-11-29T09:27:50Z

src/scripts/import-open-api.ts

+};
+
+/**
+ * Generate a restful-react compoment from openapi operation specs


Suggested change

* Generate a restful-react compoment from openapi operation specs

* Generate a restful-react component from openapi operation specs

micha-f · 2018-11-29T10:58:42Z

src/scripts/import-open-api.ts

@@ -267,7 +272,7 @@ export const generateRestfulComponent = (
  const needAResponseComponent = responseTypes.includes("{");

  // We ignore last param of DELETE action, the last params should be the `id` and it's given after in restful-react


What does this mean the last params should be the id and it's given after in restful-react?

In restful-react, for the delete action, we can do:

<Mutate verb="DELETE" path="myresource"> {deleteMyRes => <button onClick={() => deleteMyRes("myid")}>delete this</button>} </Mutate>

And this will call {base}/myresources/myid on click => the id is injected after, not directly in the path

There's a lot of English changes to come to this PR. My plan is to add a commit with updated docs/comments/etc.

IMO we can resolve this for now and then add docs at the end as a final touch.

@TejasQ As long as we don't forget. This is not docs, this is a code comment. In my opinion, comments should be fixed with the code they comment on.

My plan is to add a commit with updated docs/comments/etc.

We won't forget. 😄

fabien0102 · 2018-11-30T10:54:07Z

go2dts -> openAPI

fabien0102 · 2018-11-30T10:54:11Z

go2dts -> openAPI

TejasQ

Looks good, but I have some thoughts.

package.json

src/Get.tsx

src/Mutate.test.tsx

src/bin/restful-react-import.ts

TejasQ · 2018-12-03T15:21:16Z

Let's merge this after we confirm the prerelease works with IdP.

fabien0102 added enhancement New feature or request discussion labels Aug 14, 2018

fabien0102 self-assigned this Aug 14, 2018

fabien0102 requested a review from TejasQ August 14, 2018 12:55

TejasQ reviewed Aug 28, 2018

View reviewed changes

mshustov reviewed Sep 21, 2018

View reviewed changes

TejasQ added help wanted Extra attention is needed and removed discussion labels Sep 21, 2018

fabien0102 mentioned this pull request Sep 26, 2018

Add TBody to Mutate to be able to type the body #59

Closed

fabien0102 force-pushed the open-api-import branch from 7223a19 to 019a147 Compare October 10, 2018 19:56

fabien0102 added 6 commits November 27, 2018 11:00

Fix npm build

a1095b2

Fix npm build

4ed5034

Fix delete edge case

b62ddc6

Deal with long polling

bf76759

Fix polling types

f65cd44

Deal with no response case

d76a54e

TejasQ reviewed Nov 29, 2018

View reviewed changes

fabien0102 added 3 commits November 29, 2018 10:43

Introduce query params

d3a51af

Fix typos

c6756dd

Rename TBody to TRequestBody

9a710a7

micha-f reviewed Nov 29, 2018

View reviewed changes

fabien0102 added 2 commits November 29, 2018 12:30

Refactor overkill discrimination

07ce83f

Generate query params

6e5027e

fabien0102 changed the title ~~[wip] Open api import~~ Open api import Nov 29, 2018

Fix swagger2openapi definition

ef52906

TejasQ reviewed Dec 3, 2018

View reviewed changes

package.json Show resolved Hide resolved

package.json Show resolved Hide resolved

src/Get.tsx Show resolved Hide resolved

src/Mutate.test.tsx Show resolved Hide resolved

src/bin/restful-react-import.ts Show resolved Hide resolved

src/bin/restful-react-import.ts Show resolved Hide resolved

Improve code style

c9d3a86

fabien0102 assigned TejasQ Dec 3, 2018

fabien0102 and others added 6 commits December 6, 2018 09:20

Add a transformer param for advanced cases

e4dc683

Add a tslint disable on empty object

c16fdd2

Update documentation about codegen

bb0564e

Deal with unused imports

3a8aba1

Document QueryParams and transformer

782349a

Fix README snippet

a9dc566

TejasQ approved these changes Dec 7, 2018

View reviewed changes

TejasQ merged commit 57c9b14 into master Dec 7, 2018

TejasQ deleted the open-api-import branch December 7, 2018 11:47

	* Generate a restful-react compoment from openapi operation specs
	* Generate a restful-react component from openapi operation specs

		@@ -267,7 +272,7 @@ export const generateRestfulComponent = (
		const needAResponseComponent = responseTypes.includes("{");

		// We ignore last param of DELETE action, the last params should be the `id` and it's given after in restful-react

Open api import #28

Open api import #28

Conversation

fabien0102 commented Aug 14, 2018 • edited Loading

Why

Usage

Output (current)

Usage inside the client project (current)

Todo

contiamo-ci commented Aug 14, 2018

fabien0102 commented Aug 28, 2018

TejasQ left a 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

mshustov left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabien0102 commented Sep 21, 2018

TejasQ commented Sep 21, 2018

mshustov commented Sep 24, 2018 • edited Loading

TejasQ commented Sep 24, 2018

fabien0102 commented Sep 24, 2018

micha-f commented Oct 11, 2018 • edited Loading

peterszerzo commented Oct 11, 2018

fabien0102 commented Oct 11, 2018

mshustov commented Oct 11, 2018 • edited Loading

TejasQ commented Oct 12, 2018 • edited Loading

mshustov commented Oct 12, 2018

TejasQ commented Oct 12, 2018

micha-f commented Oct 14, 2018 • edited Loading

mshustov commented Oct 15, 2018

TejasQ commented Oct 15, 2018

fabien0102 commented Oct 15, 2018

TejasQ commented Oct 15, 2018

mshustov commented Oct 15, 2018 • edited Loading

TejasQ left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabien0102 commented Nov 30, 2018

fabien0102 commented Nov 30, 2018

TejasQ left a comment

Choose a reason for hiding this comment

TejasQ commented Dec 3, 2018

fabien0102 commented Aug 14, 2018 •

edited

Loading

mshustov commented Sep 24, 2018 •

edited

Loading

micha-f commented Oct 11, 2018 •

edited

Loading

mshustov commented Oct 11, 2018 •

edited

Loading

TejasQ commented Oct 12, 2018 •

edited

Loading

micha-f commented Oct 14, 2018 •

edited

Loading

mshustov commented Oct 15, 2018 •

edited

Loading