Skip to content

[BUG] typescript-fetch should not use namespaces (SyntaxError: Namespaces are not supported.) #1947

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

Closed
5 of 6 tasks
ceefour opened this issue Jan 20, 2019 · 8 comments
Closed
5 of 6 tasks

Comments

@ceefour
Copy link

ceefour commented Jan 20, 2019

Bug Report Checklist

  • Have you provided a full/minimal spec to reproduce the issue?
  • Have you validated the input using an OpenAPI validator (example)?
  • What's the version of OpenAPI Generator used?
  • Have you search for related issues/PRs?
  • What's the actual output vs expected output?
  • [Optional] Bounty to sponsor the fix (example)
Description

Generated files result in error: (e.g. in create-react-app with TypeScript support)

./src/api/profile/models/Culture.ts
SyntaxError: C:\Users\ceefour\git\heartenly-pwa\src\api\profile\models\Culture.ts: Namespaces are not supported.
  110 |  * @namespace Culture
  111 |  */
> 112 | export namespace Culture {
      |        ^
  113 |     /**
  114 |      * @export
  115 |      * @enum {string}

Adding configuration supportsES6: true doesn't help.

Via @DanielRosenwasser - facebook/create-react-app#4837 (comment) :

TypeScript PM here. Broadly speaking, I feel that

namespaces are not a huge loss; the React community is firmly on-board with ES modules anyway, and that's the direction the TypeScript world is taking.
const enums are often misused. Sure, they can be great for the internals of a library to get some speed improvements, but they generally shouldn't be used outside of that scope.
Merging behavior is marginally useful now that TypeScript 3.1 has the ability to tack properties onto functions. The fact that enums merge is also something I've never seen used out in the wild.

via @Timer - facebook/create-react-app#5681 (comment) :

I'm sorry and understand that this is likely frustrating, but namespaces are a proprietary, legacy feature. They have been replaced by specification behavior: ES Modules.
Namespaces are deprecated. The TypeScript documentation is going to remove all references to them in examples and stop usage. In other words, you should not be using namespaces anymore.
Const Enums are rarely needed and often misused.

openapi-generator version

0.0.7-4.0.0-beta

OpenAPI declaration file content or url

anything that uses enum

Command line used for generation
openapi-generator generate -i E:\tmp\lovia-heartenly-profile-4.0-swagger.yaml -g typescript-fetch -o src/api/profile
Steps to reproduce
  1. generate using typescript-fetch
  2. @babel/preset-typescript won't compile, and the generated files use deprecated namespace feature
Related issues/PRs
Suggest a fix

Remove the namespace and use the enum directly in the module.

@auto-labeler
Copy link

auto-labeler bot commented Jan 20, 2019

👍 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.

@ceefour ceefour changed the title [BUG] typescript-fetch should use ES6 modules instead of namespaces (SyntaxError: Namespaces are not supported.) [BUG] typescript-fetch should not use namespaces (SyntaxError: Namespaces are not supported.) Jan 20, 2019
@macjohnny
Copy link
Member

@ceefour sounds good. do you want to file a PR for that?

@denyo
Copy link
Contributor

denyo commented Jan 22, 2019

@ceefour your suggestion

Remove the namespace and use the enum directly in the module.

works as long as their isn't another enum with the same name.

For instance with the pet store examples that's the case and you end up with the following error within the models barrel:

[ts] Module './Order' has already exported a member named 'StatusEnum'. Consider explicitly re-exporting to resolve the ambiguity. [2308]

So to prevent this I'd suggest to prefix the enum with the namespace.

@ceefour
Copy link
Author

ceefour commented Jan 22, 2019

I agree with @denyo, that would work for all cases. 👍🏾

Optionally there could be a configuration option to disable namespace prefixing if the API author is sure their enum names never conflict. But generally I agree with @denyo's solution.

@cassinaooo
Copy link
Contributor

@macjohnny
Having the same problem, could file a PR if someone guide me through it, I'm not familiar with the code base. Can you point me in the right direction? Module, files to check etc... ?

@macjohnny
Copy link
Member

@willianscfa the starting point for the code generation template is here:

/**
* @export
* @namespace {{classname}}
*/
export namespace {{classname}} {
{{#vars}}
{{#isEnum}}
/**
* @export
* @enum {string}
*/
export enum {{enumName}} {
{{#allowableValues}}
{{#enumVars}}
{{{name}}} = {{{value}}}{{^-last}},{{/-last}}
{{/enumVars}}
{{/allowableValues}}
}
{{/isEnum}}
{{/vars}}

the controller is here https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/TypeScriptFetchClientCodegen.java

after running

mvn clean
mvn package

you can test the result with

java -jar modules/openapi-generator-cli/target/openapi-generator-cli.jar generate -g typescript-fetch -i samples/my-namespace-test.yml -o samples/typescript-fetch-namespace-test/

@Rassilion
Copy link
Contributor

typescript-axios client has same problem.Is there any update plan for that client?

@macjohnny
Copy link
Member

@Rassilion you could add this to your plans and support us fixing it ;-)

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

No branches or pull requests

5 participants