Openapi-generator: [REQ] add /* eslint-disable */ to JS/TypeScript files

Created on 1 Sep 2019  ·  17Comments  ·  Source: OpenAPITools/openapi-generator

Is your feature request related to a problem? Please describe.

I'm using open-api-generator with create-react-app / typescript.

I generated dead simple API by below command:
openapi-generator generate -i openapi.yml -g typescript-axios -o ./src/api-client

Then, executed yarn start command, It shows warning:

Compiled with warnings.

./src/api-client/base.ts
  Line 17:  'AxiosPromise' is defined but never used  @typescript-eslint/no-unused-vars

./src/api-client/api.ts
  Line 19:  'COLLECTION_FORMATS' is defined but never used  @typescript-eslint/no-unused-vars

Search for the keywords to learn more about each warning.
To ignore, add // eslint-disable-next-line to the line before.

I feel these generated codes shouldn't be lint-ed.

Describe the solution you'd like

Just add /* eslint-disable */ to top of the generated .js/.ts files.

Additional context

Currently, Create React App(CRA) doesn't follow .eslintignore.
https://github.com/facebook/create-react-app/issues/2339

CRA & eslint is one of the most popular tools to develop JS/TS frontend.
I feel this feature is reasonable.

TypeScript Feature

Most helpful comment

I would really like this to be added for the very same reason mentioned by @AshSuzuki .

The files in question already have // tslint:disable at the top. But tslint is no longer the preferred tool for linting TypeScript, eslint is.

I will be more than happy to submit a PR.

All 17 comments

👍 Thanks for opening this issue!
🏷 I have applied any labels matching special text in your issue.

The team will review the labels and make any necessary changes.

I don’t think this is necessary or helpful. You are responsible for the linting you are doing, @AshSuzuki. If you feel that the linting done on the generated code is to much, just add your own .eslintrc.js in the src/api-client directory and disable the respective rules.

I think it's helpful, but unnecessary.

@bodograumann, As I mentioned in Additional context, CRA doesn't follow .eslintrc.js.

@AshSuzuki You talked about .eslintignore, which is something different.

Another solution would be to put the generated api client into a separate npm package. Then it would be included in node_modules of your main application and you can setup your own linting for the api. This will also make the api client code more easily reusable.

I would really like this to be added for the very same reason mentioned by @AshSuzuki .

The files in question already have // tslint:disable at the top. But tslint is no longer the preferred tool for linting TypeScript, eslint is.

I will be more than happy to submit a PR.

@bodograumann

Yeah, I know there's several options to prevent from linting.
But I feel linting should be disabled by default.

(Sorry about late reply)

@mihai-dinculescu

Thanks. I didn't realized these files already has // tslint:disable.
As current implementation produces it, this request would be reasonable.

It looks like #4110 made this change, so now we're just waiting for the next release to include it.

Fixed in #4110

For me, // eslint-disable is not enough to hide @typescript-eslint/no-unused-vars warning. I still have dozens of warnings like:

 Line 93:5:  'LoginDtoFromJSON' is defined but never used        @typescript-eslint/no-unused-vars

I would need to write /* eslint-disable */ or more specifically /* eslint @typescript-eslint/no-unused-vars: 0 */ at the beginning of generated files apis, models and runtime.

You are correct, @tocosastalo: https://eslint.org/docs/user-guide/configuring#disabling-rules-with-inline-comments

Do you want to file a PR to change this?

Can I help with this? I'm new to the project but I'm happy to make a PR if someone points me in the right direction. I'm guessing I just change the same .mustache files that #4110 did and then run some command to generate all of the rest?

@mikeewheaton yes, you need to change the mustache files and run „mvn clean package“, then run bin/typescript-axios-petstore-all.sh and
commit the mustache files plus the files changed in /samples.

@bodograumann's comment on 802 makes a good point about how adding /* eslint-disable */ would prevent linting for everyone, even those who want it.

To me, this is really a problem with Create React App's configuration making it difficult to ignore folders. I'm going to try the solution in #2339 again. 🤞

@macjohnny it is only fixed for typescript-fetch and not for typescript-axios. Why only for fetch?

it is only fixed for typescript-fetch and not for typescript-axios. Why only for fetch?

@rubeonline feel free to file a PR, contributions are welcome.

@macjohnny What do I need to change to add this for typescript-axios as well?

Was this page helpful?
0 / 5 - 0 ratings