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

[JavaScript] Usage with create-react-app + Promises + ES6 unsupported? #8024

Open
john-goldsmith opened this issue Apr 16, 2018 · 9 comments

Comments

@john-goldsmith
Copy link

john-goldsmith commented Apr 16, 2018

Description

The generated JavaScript output from passing the usePromises and/or useES6 options don't seem to play nicely with a fresh non-ejected React app created using create-react-app. I realize this sounds more like a create-react-app problem than a Swagger problem, but some initial findings indicate that Swagger's ES6 static property syntax isn't (yet) supported. It's possible I'm just overlooking something simple, but figured I'd float it to the community.

Swagger-codegen version

2.3.1

Swagger declaration file content or url

http://petstore.swagger.io/v2/swagger.json

Command line used for generation

JavaScript, with Promises:

swagger-codegen generate -i http://petstore.swagger.io/v2/swagger.json -l javascript -o . --additional-properties usePromises=true

Yields:

Failed to compile.

../my-app/src/index.js
Module not found: Can't resolve 'ApiClient' in '/Users/jgoldsmith/Sites/my-app/src'

(which, to me, just looks like some missing relative path qualifiers, e.g., ./ApiClient)

JavaScript, with Promises and ES6:

swagger-codegen generate -i http://petstore.swagger.io/v2/swagger.json -l javascript -o . --additional-properties usePromises=true,useES6=true

Yields:

../my-app/src/ApiClient.js
Module parse failed: Unexpected token (231:32)
You may need an appropriate loader to handle this file type.
|     * @readonly
|     */
|     static CollectionFormatEnum = {
|         /**
|          * Comma-separated values. Value: <code>csv</code>

(ref)

I manually changed all instances of static someProperty in the generated SDK to static get someProperty() and things seemed to work fine (but obviously that's not a real solution).

Steps to reproduce
  1. Using swagger-codegen, generate JavaScript SDKs, passing the usePromises and/or useES6 options
  2. Use create-react-app to create a new app
  3. Install/link the generated SDK into the app
  4. Run npm start in the React app
  5. Note the compilation errors
Related issues/PRs
Suggest a fix/enhancement

For non-ES6 output: use the correct relative paths
For ES6 output: change static properties to static get myProp() { return value; } or MyClass.myProp (my personal preference, ref)

@john-goldsmith
Copy link
Author

A little more research revealed some additional information.

The "Supported Language Features and Polyfills" section of the create-react-app README states that "Class Fields and Static Properties" are supported via the "Class Public Fields" TC39 proposal. That proposal, however, has been merged with the "Class Fields" TC39 proposal (currently Stage 2) which states:

Placement: Static vs instance -- static postponed to follow-on proposal

So, again, it seems like the Swagger ES6 codegen is a little too bleeding edge.

@joyfulelement
Copy link

Just out of curiosity, does the generated SDK with usePromises and useES6 option work with regular react app not generated by create-react-app?

@john-goldsmith
Copy link
Author

@joyfulelement I haven't actually tested your scenario, but I imagine it would depend on how you have webpack/babel/browserify setup for your non-CRA project. The code generated when using both the usePromises and useES6 options is valid assuming that you've setup the transpile step to correctly interpret that syntax. If, however, you don't have a transpile step, then obviously the generated code is not valid JS.

@jimklo
Copy link

jimklo commented May 24, 2018

@john-goldsmith have you found a solution for this? I'm having the same issue. I'm not really wanting to eject my CRA, but cannot seem to find a way to get our library to import into our react app.

I have the same setup CRA + swagger-codegen JS client, and getting this error:

./node_modules/sunflower-client/src/model/DbFloraDocument.js
Module parse failed: Unexpected token (128:10)
You may need an appropriate loader to handle this file type.
|     * @member {String} docId
|     */
|     docId = undefined;
|     /**
|     * @member {Array.<module:model/DbImport>} imports

delenius added a commit to delenius/openapi-generator that referenced this issue May 30, 2018
1. Don't use property initializers

The ES6 Javascript generator used property initializers, as
described [here](https://tc39.github.io/proposal-class-public-fields/).

This is not standard ES6. In particular, create-react-app chokes on it
(see [this issue](swagger-api/swagger-codegen#8024),
which is fixed by this commit).

This commit instead reverts back to using prototypes, as in the non-es6
JS generator.

2. Add missing pom.xml files

This allows us to run `mvn integration-test` in these folders.

3. Add missing double-quotes around enum field names.

This avoids errors due to fields that contain illegal characters.

4. Fix support for multiple inheritance

The previous implementation was broken. I added static "initialize"
methods, which supports multiple inheritance.

5. Comment out some broken tests.

There were tests for OuterBoolean etc, which are not generated
as part of index.js in the generated petstore clients. Hence, these
tests threw an error. The actual tests were already commented out
anyway. I just commented out one more line to avoid an error.

6. Remove some empty lines in generates JS files.
@john-goldsmith
Copy link
Author

john-goldsmith commented May 31, 2018

@jimklo Sorta -- I'm using the --template-dir CLI option to monkey patch some fixes.

Pros:

  • "Fixes" the problem! 😄
  • Doesn't impact my CRA; no need to eject
  • Overridden files remain isolated to my SDK project/repo
  • Removing the overridden files in the future will be trivial
  • Retains auto-generation features of swagger-codegen without requiring manual intervention

Cons:

  • Overridden files can become out of sync with the ones from this repo
  • Generally good to avoid monkey patching

I created several directories in my SDK repo that mirror the directories in this repo for sanity's sake, but it's not required:

my-sdk
  └ modules
    └ swagger-codegen
      └ src
        └ main
          └ resources
            └ Javascript
              └ es6
                ├ api.mustache # https://github.com/swagger-api/swagger-codegen/blob/master/modules/swagger-codegen/src/main/resources/Javascript/es6/api.mustache
                ├ ApiClient.mustache # https://github.com/swagger-api/swagger-codegen/blob/master/modules/swagger-codegen/src/main/resources/Javascript/es6/ApiClient.mustache
                ├ partial_model_generic.mustache # https://github.com/swagger-api/swagger-codegen/blob/master/modules/swagger-codegen/src/main/resources/Javascript/es6/partial_model_generic.mustache
                ├ partial_model_inner_enum.mustache # https://github.com/swagger-api/swagger-codegen/blob/master/modules/swagger-codegen/src/main/resources/Javascript/es6/partial_model_inner_enum.mustache
                └ README.md # Just for good measure

And I create the SDK with following command:

swagger-codegen generate \
  --input-spec http://petstore.swagger.io/v2/swagger.json \
  --lang javascript \
  --output . \
  --template-dir ./modules/swagger-codegen/src/main/resources/Javascript/es6 \
  --additional-properties usePromises=true,useES6=true

I haven't yet tested @delenius' changes noted above, but on the surface it sounds like it'll be the official/preferred fix.

delenius added a commit to delenius/openapi-generator that referenced this issue Jun 14, 2018
1. Don't use property initializers

The ES6 Javascript generator used property initializers, as
described [here](https://tc39.github.io/proposal-class-public-fields/).

This is not standard ES6. In particular, create-react-app chokes on it
(see [this issue](swagger-api/swagger-codegen#8024),
which is fixed by this commit).

This commit instead reverts back to using prototypes, as in the non-es6
JS generator.

2. Add missing pom.xml files

This allows us to run `mvn integration-test` in these folders.

3. Add missing double-quotes around enum field names.

This avoids errors due to fields that contain illegal characters.

4. Fix support for multiple inheritance

The previous implementation was broken. I added static "initialize"
methods, which supports multiple inheritance.

5. Comment out some broken tests.

There were tests for OuterBoolean etc, which are not generated
as part of index.js in the generated petstore clients. Hence, these
tests threw an error. The actual tests were already commented out
anyway. I just commented out one more line to avoid an error.

6. Remove some empty lines in generates JS files.
@mvamax
Copy link

mvamax commented Jun 19, 2018

Hi, anyone to summarize the "state or art" on how to integrate a codegen swagger javascript or ES6 in a create-react-app without doing an eject?

@delenius
Copy link
Contributor

delenius commented Jun 19, 2018

@mvamax , this works in the latest master of openapi-generator. It actually does not work with the es6 client, simply because CRA does not transpile the dependencies. You can run it in dev mode, but the build fails. However, it does work with the regular JS client (I only tested with Promises, but it probably works with or without). Try it out.

@akash-pal
Copy link

@jimklo Sorta -- I'm using the --template-dir CLI option to monkey patch some fixes.

Pros:

  • "Fixes" the problem! 😄
  • Doesn't impact my CRA; no need to eject
  • Overridden files remain isolated to my SDK project/repo
  • Removing the overridden files in the future will be trivial
  • Retains auto-generation features of swagger-codegen without requiring manual intervention

Cons:

  • Overridden files can become out of sync with the ones from this repo
  • Generally good to avoid monkey patching

I created several directories in my SDK repo that mirror the directories in this repo for sanity's sake, but it's not required:

my-sdk
  └ modules
    └ swagger-codegen
      └ src
        └ main
          └ resources
            └ Javascript
              └ es6
                ├ api.mustache # https://github.com/swagger-api/swagger-codegen/blob/master/modules/swagger-codegen/src/main/resources/Javascript/es6/api.mustache
                ├ ApiClient.mustache # https://github.com/swagger-api/swagger-codegen/blob/master/modules/swagger-codegen/src/main/resources/Javascript/es6/ApiClient.mustache
                ├ partial_model_generic.mustache # https://github.com/swagger-api/swagger-codegen/blob/master/modules/swagger-codegen/src/main/resources/Javascript/es6/partial_model_generic.mustache
                ├ partial_model_inner_enum.mustache # https://github.com/swagger-api/swagger-codegen/blob/master/modules/swagger-codegen/src/main/resources/Javascript/es6/partial_model_inner_enum.mustache
                └ README.md # Just for good measure

And I create the SDK with following command:

swagger-codegen generate \
  --input-spec http://petstore.swagger.io/v2/swagger.json \
  --lang javascript \
  --output . \
  --template-dir ./modules/swagger-codegen/src/main/resources/Javascript/es6 \
  --additional-properties usePromises=true,useES6=true

I haven't yet tested @delenius' changes noted above, but on the surface it sounds like it'll be the official/preferred fix.

Hi @john-goldsmith is it possible to generate reactjs component using swagger yaml?

@arthurpankiewicz
Copy link

@john-goldsmith using the template-dir option seems to still generate incompatible code. Any chance you've found a better workaround in 2019?

Thanks

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

No branches or pull requests

7 participants