fix(typescript-axios): use baseURL of (custom) axios instance

[nicobao]

Closes #11799

This PR is a minor change affecting typescript-axios.

I tried to pass a custom axios instance but it wouldn't take into account its baseURL:

      import axios from "axios";
      
      const customAxios = axios.create({
        baseURL: "http://localhost:3000",
      });

      DefaultApiFactory(undefined, undefined, customAxios)
        .authIsEmailAvailablePost(email)
        .then((response: boolean) => {
           console.log(response);
        })
        .catch((e) => {
           console.error(e);
        });

I checked in the generated code, the customAxios does have the baseURL and is properly loaded, but instead of http://localhost:3000/auth/isEmailAvailable, the URL called was http://localhost/auth/isEmailAvailable, because BASE_PATH is loaded, which is http://localhost, and customAxios.baseURL is never read whatsover:

https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/resources/typescript-axios/common.mustache#L139

By default, axios global object has baseURL===undefined, so the patch I did should not affect anything if no custom axios instance is passed.
Note that with this patch, typescript-axios would also work as expected when no custom axios instance is passed but the global axios baseURL is modified.

I am not sure why BASE_PATH is called like this, it should be BASE_URL. Similarly, Configuration.basePath should be Configuration.baseURL.

basePath (as opposed to baseURL), as something like /v2, is added here:
https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/resources/typescript-axios/apiInner.mustache#L277
==> request(axios, basePath)
That basePath comes from the basePath in param of the factory, but I think it should be basePath || Configuration.basePath instead. And we should add another option to Configuration which will be Configuration.baseURL - and use it in common.mustache like I did in this patch.

@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) @davidgamero (2022/03) @mkusaka (2022/04)

To me, this is a bug fix, that's not breaking (except for people relying on the bug...) - that's why I target master.

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.
• In case you are adding a new generator, run the following additional script :

  ./bin/utils/ensure-up-to-date
  

  Commit all changed files.
• File the PR against the correct branch: master (6.3.0) (minor release - breaking changes with fallbacks), 7.0.x (breaking changes without fallbacks)
• If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

[TiFu]

Code looks good;

Question on the implementation: Wouldn't it make more sense to just get rid of the entire basePath construct and just set the right basePath in the configuration in the APIFactory?

That significantly reduces the number of different parameters being passed around.

[macjohnny]

LGTM
thanks for your contribution!

[macjohnny]

Code looks good;

Question on the implementation: Wouldn't it make more sense to just get rid of the entire basePath construct and just set the right basePath in the configuration in the APIFactory?

That significantly reduces the number of different parameters being passed around.

this makes sense. I merge this PR now, so maybe this can be done in a follow-up

[nicobao]

Code looks good;

Question on the implementation: Wouldn't it make more sense to just get rid of the entire basePath construct and just set the right basePath in the configuration in the APIFactory?

That significantly reduces the number of different parameters being passed around.

I agree.
I am also not sure whether it is necessary to have two fields in Configuration: baseURL and basePath. That's because they are actually two different concepts. But we use the external "basePath" param as a real basePath (such as "/v2"), whereas we use Configuration.basePath as axios.defaults.baseURL (such as "http://api.example.com"). Kinda confusing.

I think we should drop the external basePath as you suggested and only keep Configuration.basePath, which we could rename to Configuration.baseURL.

All of these are breaking changes though.

[goxiaoy]

This causes baseURL append twice if you have changed global default by

axios.defaults.baseURL = '/basic-api';

see https://axios-http.com/docs/config_defaults

[nicobao]

This causes baseURL append twice if you have changed global default by

axios.defaults.baseURL = '/basic-api';

see https://axios-http.com/docs/config_defaults

Please could you elaborate? I don't understand the problem.
Do you mean that if defaults baseURL is set, it is picked over the custom axios instance one?
Or the default baseURL is picked instead of something else you think should have higher priority?

[goxiaoy]

This change breaks the default behavior of using globalAxios

configuration?.basePath is empty

considering the following codes:

   import axios from "axios";
    
   axios.defaults.baseURL = '/basic-api';

    DefaultApiFactory()
      .authIsEmailAvailablePost(email)
      .then((response: boolean) => {
         console.log(response);
      })
      .catch((e) => {
         console.error(e);
      });

/basic-api is appendded twice , the first in

const axiosRequestArgs = {...axiosArgs.options, url: (configuration?.basePath || axios.defaults.baseURL || basePath) + axiosArgs.url};

the second in axios itself

return axios.request<T, R>(axiosRequestArgs);

In your case, you might need to set host in your openapi spec instead of changing openapi-generator codes

{
"host": "localhost:3000",
"schemes": [
    "http"
  ],
}

Another solution is to allow basePath to empty when host is not set in the spec

export const BASE_PATH = "{{{basePath}}}".replace(/\/+$/, "");

axios.defaults.baseURL is called as default because once you specify the baseURL later (in your problem is http://localhost), this configuration will be ignored

[nicobao]

Another solution is to allow basePath to empty when host is not set in the spec

export const BASE_PATH = "{{{basePath}}}".replace(/\/+$/, "");

axios.defaults.baseURL is called as default because once you specify the baseURL later (in your problem is http://localhost), this configuration will be ignored

So that would both solve my problem without hurting the default behavior existing prior to this PR?
This solution seems like a good compromise.
You seem to have a good understanding of the problem.
If you send a PR, I am happy to review and locally test it so we can merge and solve your problem quickly (though beware that I am not a maintainer).