Skip to content

[typescript-axios] / #12828: Add ESM support to typescript-axios client#13586

Merged
macjohnny merged 4 commits into
OpenAPITools:masterfrom
miniengineer:fix_issue12828
Oct 7, 2022
Merged

[typescript-axios] / #12828: Add ESM support to typescript-axios client#13586
macjohnny merged 4 commits into
OpenAPITools:masterfrom
miniengineer:fix_issue12828

Conversation

@miniengineer

@miniengineer miniengineer commented Oct 4, 2022

Copy link
Copy Markdown
Contributor

fix #12828

An additional tsconfig.esm.json is added to every generated typescript-axios code having supportsES6 and npmName.
That will create an additional ECMAScript Module (ESM) to the npm build.

I used similar PR for typescript-fetch client as reference #11720

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 (6.1.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 @taxpon @sebastianhaas @kenisteward @Vrolijkx @macjohnny @topce @akehir @petejohansonxo @amakhrov @davidgamero @mkusaka

@miniengineer miniengineer marked this pull request as ready for review October 4, 2022 08:59
@miniengineer

miniengineer commented Oct 4, 2022

Copy link
Copy Markdown
Contributor Author

This is my first time contributing to open source 🙇‍♀️

About the following step ⬇️

Run the following to build the project and update samples:
./mvnw clean package
./bin/generate-samples.sh
./bin/utils/export_docs_generators.sh

After running the first command there are some errors, but they are related to the Python client 🤔
I run it while on latest master and I get the same errors, so I assume those are not related to changes in this PR.

Screen Shot 2022-10-04 at 17 20 29

When generating samples I also got an error T_T

./bin/generate-samples.sh ./bin/configs/typescript*
Error: Unable to access jarfile /blabla/openapi-generator/modules/openapi-generator-cli/target/openapi-generator-cli.jar

I am sorry, Java is not my first language.
Am I doing something wrong?

@jfeltesse-mdsol jfeltesse-mdsol left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mind blown cat

@miniengineer

Copy link
Copy Markdown
Contributor Author

😍 😍 😍 😍

@miniengineer

Copy link
Copy Markdown
Contributor Author

UPDATE

I was able to create the JAR file using the following command that skips tests
mvn clean install -Dmaven.javadoc.skip=true -Dmaven.test.skip=true

and successfully generated samples 👍

@macjohnny macjohnny left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for your contribution!

@miniengineer

Copy link
Copy Markdown
Contributor Author

@macjohnny
Thank you for the review 🙏

About removing outDir, as Julien mentioned (and I should've probably added that in the PR description) the reason why I deleted it is because we already supply output directory in the ts config file, so it's redundant in the package.json.

I have successfully build and tested out TS clients on my local 👍

@macjohnny macjohnny merged commit 2f8b7a0 into OpenAPITools:master Oct 7, 2022
@wing328 wing328 added this to the 6.2.1 milestone Nov 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[REQ] Typescript-axios ESM support

4 participants