[typescript-axios] Add option to add NodeJS imports

[vesse]

While NodeJS has had global URL and URLSearchParams for quite some time they still need to be imported when using TypeScript and not adding DOM to libs (see DefinitelyTyped/DefinitelyTyped#34960). Since server side code should not really need DOM added an option to generate the imports needed so there is no need to postprocess the generated code.

Also add import for form-data when using both withNodeImports and multipartFormData, and add the form data headers to localVarRequestOptions.headers if FormData object has function getHeaders.

PR checklist

• Read the contribution guidelines.
• Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
• Run the following to build the project and update samples:

  ./mvnw clean package 
  ./bin/generate-samples.sh
  ./bin/utils/export_docs_generators.sh
  

  Commit all changed files.
  This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
  These must match the expectations made by your contribution.
  You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
  For Windows users, please run the script in Git BASH.
• File the PR against the correct branch: master (5.3.0), 6.0.x
• If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

cc @TiFu (2017/07) @taxpon (2017/07) @sebastianhaas (2017/07) @kenisteward (2017/07) @Vrolijkx (2017/09) @macjohnny (2018/01) @topce (2018/10) @akehir (2019/07) @petejohansonxo (2019/11) @amakhrov (2020/02)

[arminrosu]

Thanks, came here for the same issue and you already have a PR. Thanks!

I would suggest though to rename this option to withNodeImports and add an import FormData from 'form-data' to api.mustache as well.

This is required for multipart/formData file uploads:

openapi-generator/modules/openapi-generator/src/main/resources/typescript-axios/apiInner.mustache

Line 65 in f27799c

const localVarFormParams = new {{^multipartFormData}}URLSearchParams(){{/multipartFormData}}{{#multipartFormData}}((configuration && configuration.formDataCtor) || FormData)(){{/multipartFormData}};{{/hasFormParams}}{{/vendorExtensions}}

[vesse]

Sure, makes sense. Didn't notice this need because I haven't used form-data myself. Just a moment.

[arminrosu]

Sorry to bother again, I just checked my code, and there is 1 modification more I had to make re formData:

openapi-generator/modules/openapi-generator/src/main/resources/typescript-axios/apiInner.mustache

Line 192 in f27799c

localVarRequestOptions.headers = {...localVarHeaderParameter, ...headersFromBaseOptions, ...options.headers};

- localVarRequestOptions.headers = {...localVarHeaderParameter, ...headersFromBaseOptions, ...options.headers};
+ localVarRequestOptions.headers = {...localVarHeaderParameter, ...headersFromBaseOptions, ...localVarFormParams.getHeaders(), ...options.headers};

It seems otherwise it doesn't add the necessary length headers. And because the function it generates can take a stream as input, it might be difficult / inconvenient to set the length as a custom options.headers

[vesse]

@arminrosu now added also form-data import and the getHeaders call if the FormData object at hand has the method (browser FormData does not have the function so this change should not break browser usage)

[arminrosu]

Thanks again Vesa!

@ maintainers - if this PR gets merged, I suggest you remove the typescript-node generator, as it uses request internally, which has been deprecated for years.

[vesse]

Not sure if this should have // @ts-ignore – it is of course possible to set the multipartFormData parameter for a spec that does not have any form data endpoints and then this import would be unused (and probably the form-data dependency is missing too if not needed), but then again is it a user error if they configure to use something that is not needed?

[arminrosu]

is it a user error if they configure to use something that is not needed?

In my view, it totally is. Or otherwise put - if you declared your need for this code but it's not there (e.g. you want to manually extend the file), would that not be a generation error?

---

In my humble opinion, neither of eslint-disable, tslint:disable or ts-ignore should be present in this file. All of the unused imports can and should be fixed using eslint --fix, prettier --write or whatever code formatter someone uses. Since this is a programatic change, not a manual one, you can consistently apply it after each generation.

That's how I do it. I remove all these comments using sed after generating the source code and before formatting. The only warnings I get are related to the use of any typings.

Besides, tslint has also been deprecated since 2019.

Btw, typescript-node has quite a few linting errors, including circular dependencies - yet another reason to deprecate that generator template.

[amakhrov]

eslint --fix doesn't fix unused imports
and TS itself doesn't have autofix capability (so noUnusedLocals compiler setting will simply fail to compile the generated code)

[arminrosu]

@amakhrov I'm using eslint-plugin-unused-imports to remove unused imports.

IMO, the generated code should either be valid, or not disable linting. The latter is especially tricky, since you must support all linting tooling (e.g. there should also be a .prettierignore), which to me seems unfeasable.

What other code generators do, is lint and/or format the code themselves after generation. That could be an option for JS as well.

But we are diverging from the topic of the PR.

[vesse]

@TiFu (2017/07) @taxpon (2017/07) @sebastianhaas (2017/07) @kenisteward (2017/07) @Vrolijkx (2017/09) @macjohnny (2018/01) @topce (2018/10) @akehir (2019/07) @petejohansonxo (2019/11) @amakhrov (2020/02)

Anything I could add / concerns?

[vesse]

OK so while waiting we decided to jump to Node 16 and with newer @types/node this would not be needed anymore, so kinda mixed feelings if this needs to be supported or not.

[arminrosu]

we decided to jump to Node 16 and with newer @types/node this would not be needed anymore, so kinda mixed feelings if this needs to be supported or not.

@vesse not sure that works for everyone. We're already on @types/[email protected] but we still expect explicit imports in our codebase (for various readability and tooling reasons).

While you don't need this anymore, we most certainly do and appreciate the PR all the much more 🤗 .

[AaronElijah]

+1 on this -> my company use NextJS for almost all of our web apps, which requires node >12.22.0 so the PR is very much appreciated! Thank you! 👍

[vesse]

@macjohnny thanks! I don't know for what you use the milestones but this was not included in 5.3.1.

[macjohnny]

Thanks, updated the milestone to 5.4.0