Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Typescript Fetch] Multiple enhancements #7914

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

Open
JFCote wants to merge 2 commits into swagger-api:master
base: master
Choose a base branch
Loading
from StingrayDigital:typescript-fetch-improvement
Open

[Typescript Fetch] Multiple enhancements #7914

JFCote wants to merge 2 commits into swagger-api:master from StingrayDigital:typescript-fetch-improvement

Conversation

@JFCote
Copy link
Contributor

@JFCote JFCote commented Mar 26, 2018

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
  • Filed the PR against the correct branch: 3.0.0 branch for changes related to OpenAPI spec 3.0. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

@TiFu (2017/07) @taxpon (2017/07) @sebastianhaas (2017/07) @kenisteward (2017/07) @Vrolijkx (2017/09) @macjohnny (2018/01)

Description of the PR

Add support for string enums
Support new esnext syntax for modules
Fix issue to support typescript 2.4 +

Not sure if it triggers breaking changes. If so, just let me know on which branch I should create this PR. Thanks!

c0va23, fungus1487, theodm, and Bobgy reacted with thumbs up emoji
Support new esnext syntax for modules
Fix issue to support typescript 2.7
return value;
} else {
return "\'" + escapeText(value) + "\'";
return "\"" + escapeText(value) + "\"";
Copy link
Contributor

@macjohnny macjohnny Mar 27, 2018

Choose a reason for hiding this comment

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

I wouldn't change it to double quotes. instead, #7369 should be fixed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok I will rollback this part. I was following the advice of my pro typescript teammate here but I guess it doesn't matter :)
Will provide a new version sometime this morning.
Thanks!

Copy link
Contributor

wing328 commented Apr 12, 2018

If no further question/feedback on this PR, I'll merge it tomorrow (Friday)

Copy link

Linking issue #8180

The changes supported here will resolve issues I am having compiling for strict Typescript 2.4

JFCote added a commit to OpenAPITools/openapi-generator that referenced this pull request May 24, 2018
Copy link

Are there any updates on this PR? Or is it dead?

Copy link
Contributor Author

JFCote commented Jul 16, 2018
edited
Loading

@msiemens I have cloned this PR in a fork of this project, here: OpenAPITools/openapi-generator#145
It is still not accepted since it created errors with the unit test but if you want to help, it could be merged pretty quickly.
I'm not contributing for swagger-codegen anymore. All my work will now be in openapi-generator.

Check it out if you want :)

Copy link

Hi @wing328 - I found this PR through #8180 from @fungus1487. I get the same TypeScript compilation errors (TypeScript v3.0.3) because properties are not initialized, and they would be fixed by this PR. Can I make the same changes and open a new PR, to hopefully get past the failing checks?

Copy link
Contributor

@jackkoppa in the meantime, OpenAPITools/openapi-generator#145 has been merged, so you should be able to generate the fixed code with the latest OpenAPI Generator

jackkoppa reacted with thumbs up emoji

Copy link
Contributor

Hey @jackkoppa, if you're willing to create the PR, feel free to do it and ping me there. i can help you to get your changes merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@macjohnny macjohnny macjohnny left review comments

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Projects

None yet

Milestone

v2.4.0

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /