[BUG][typescript-fetch] Output is now nested in a "src" folder which breaks typescript package references

[fuzzzerd]

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?

Description

Changes in this commit introduced several related bugs.

1. Output is now partially nested within a new src folder.
2. package.json.mustache was not updated to reflect this change
3. breaks anyone who was using local npm package references with typescript to avoid creating a separate /dist folder for the generated client code.

openapi-generator version

v4.0.3 => v4.1.0

Suggest a fix

• Revert the folder name change.
• Add a flag to opt in or out of the new src folder
• Update the mustache templates accordingly.

[auto-labeler]

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

[macjohnny]

this change was introduced in #3403
I am also in favor of reverting the src sub-path change. @fuzzzerd would you like to file a PR for this?
cc @JFCote @TiFu what do you think?

[JFCote]

@macjohnny I'm no expert in typescript/npm but I did these changes to fix the npm version that wasn't working at all when it was time to publish (there was no compiled code in resulting npm package).

I don't mind of the folder structure in the end. That was done to follow good practice in typescript npm package.

So, if the new PR still provide a valid output when publishing npm package, that's good for me. Simply reverting #3403 will break the npm publishing so it's not a valid solution.

Let me know when the PR is ready, I will gladly test it.

[fuzzzerd]

@macjohnny I lack the tooling setup and Java expertise to feel comfortable making sweeping changes to the underlying generator class. I'm just not familiar with the tooling or setup, even if the actual code changes aren't that large.

I could work on revising the mustache templates to produce a valid output package. As it is today, the generated package.json has the wrong paths for the /dist files anyway.

My current work around is to actually use OAG v4.0.3 with a custom template pulled from the master branch. Something like:

oaig generate -g typescript-fetch -t "c:\my-fork-of-master"

I can work on the templates to make sure they produce a valid npm package. Is it possible to coordinate two PRs that depend on each other if someone else can make the underling generator changes?

[fuzzzerd]

@JFCote Can you go into any more detail about what is/was wrong with the package? Did you expect the output of OpenAPI Generator to produce both the typescript source code for the package as well as the compiled javascript? I think you will always need to run npm install && npm run build on the output of the generator.

[JFCote]

@fuzzzerd First, what is oaig?

As for your question:

The problem was that the npm package contained .ts files. A real npm package needs to be compiled because if you don't, you will need to have all sort of nasty trick in your webpack configuration. When you run npm publish, there is a configuration that tell npm what to do. In our case, it is tsc and npm run build, as you can see here: https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/resources/typescript-fetch/package.mustache#L8

My changes simply:

• Make sure the output of the compilation is in dist
• Make sure the src contains the original typescript code.
• Make sure dist is used for the npm package.
• The root contains configuration files only, no code.

I followed this article: https://medium.com/cameron-nokes/the-30-second-guide-to-publishing-a-typescript-package-to-npm-89d93ff7bccd
The author have a real npm package that is hosted here: https://github.com/ccnokes/dom-event-utils

You can see there is an src folder and when you run a publish, it creates a dist folder and use this output to create the npm package.

[fuzzzerd]

Sorry: OAG = OpenAPI Generator (updated my post).

[fuzzzerd]

@JFCote your explanation makes sense.

I used OpenAPI Generator to produce a folder with the package inside. I called it private_node_modules\myclient and then did a file reference in my main package.json. Since there were only typescript files in there everything just worked, and my project took care of doing the compiling. I never actually published the generated npm package.

The changes in the PR effectively break that use case; but I don't have a full grasp of the different ways the npm package references can work, especially with webpack and typescript. I'll do some additional investigative work and see if there's a way to make it work for both use cases.

[JFCote]

@fuzzzerd Like I said earlier, I'm no typescript / npm expert. If I understand correctly, you generate the default fetch-client, not the npm one but you seem to fake an npm import. Maybe that's the problem? Since you don't use npm, why don't you just import the folder with a normal import statement in your ts files?

Like this:

import fetch-client from './my-generated-fetch-client/src/index'

Sorry if that doesn't make sense, I'm probably not the best person to help you here :)

[macjohnny]

Since you don't use npm, why don't you just import the folder with a normal import statement in your ts files?
Like this:
import fetch-client from './my-generated-fetch-client/src/index'

This makes sense and is how I use it in my project if the package is not published.

the more I think about it, the more I am in favor of leaving the code as it is now, as the npm package isn't intended to be used as an npm package without compilation, which means, the generated package.json is correct.

[fuzzzerd]

To be clear, I am using the generated client as an npm package, just not one that is published to public npm repository. My package.json looks like this:

"dependencies": {
   "api-client": "file:private_modules/api-client",
}

Having said that, as it stands today in master, the package.json points to ./dist/index.ts.d and ./dist.index.js respectively, but the build script generates ./dist/src/index.ts.d and ./dist/src/index.js. One of them needs updated.

https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/resources/typescript-fetch/package.mustache

https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/resources/typescript-fetch/tsconfig.mustache

I would love official guidance on how to properly consume the generated typescript-fetch clients, as its entirely possible what I'm doing is off the blessed path. The documentation on the main website is sorely lacking in that department.

[macjohnny]

when changing into the generated package and building with

cd samples/client/petstore/typescript-fetch/builds/with-npm-version
npm install
npm run build

I get

./dist/apis
./dist/index.d.ts
./dist/index.js
./dist/models
./dist/runtime.d.ts
./dist/runtime.js
./dist/apis/PetApi.d.ts
./dist/apis/PetApi.js
./dist/apis/StoreApi.d.ts
./dist/apis/StoreApi.js
./dist/apis/UserApi.d.ts
./dist/apis/UserApi.js
./dist/apis/index.d.ts
./dist/apis/index.js
./dist/models/Category.d.ts
./dist/models/Category.js
./dist/models/ModelApiResponse.d.ts
./dist/models/ModelApiResponse.js
./dist/models/Order.d.ts
./dist/models/Order.js
./dist/models/Pet.d.ts
./dist/models/Pet.js
./dist/models/Tag.d.ts
./dist/models/Tag.js
./dist/models/User.d.ts
./dist/models/User.js
./dist/models/index.d.ts
./dist/models/index.js
./src/apis
./src/index.ts
./src/models
./src/runtime.ts
./src/apis/PetApi.ts
./src/apis/StoreApi.ts
./src/apis/UserApi.ts
./src/apis/index.ts
./src/models/Category.ts
./src/models/ModelApiResponse.ts
./src/models/Order.ts
./src/models/Pet.ts
./src/models/Tag.ts
./src/models/User.ts
./src/models/index.ts

which seems correct.

concerning the docs, you are right, documentation of the usage of the generated code could be improved, but please have a look at the generated Readme

[fuzzzerd]

I may have spoke out of turn. I am not able to reproduce my issue of having /dist/src/... in the output.

I think my issue might be related to adding of the sub folder src and me not knowing that happened, so running subsequent updates still had my old generated client. Let me do more a bit more digging and see if I can't get it working with a clean slate.

I'll close the issue if its an issue on my end or update with what I did and what I got that isn't working.

[fuzzzerd]

@macjohnny I was able to get things working, there were two things I had to change.

1. I had to remove the ./ from the "main" and "typings" entries in package.json -- this is in line with the other package.json files in I found my node_modules. I'm not sure what's correct though.

With the client as-generated, when I did an npm install path-to-generated-client the reference was added to my package.json but it was NOT created in my node_modules folder. After changing the paths to fully relative paths, a symlink was created in my node_modules that points to the path in package.json.

This allowed my main project to compile with typescript, but generated a run time error.

2. The generated tsconfig.json specifies "module": "commonjs" and in my project I was using "module": "esnext". I changed the generated file to "esnext" and now everything compiles and runs.

This leaves me with two deviations from the as-generated code from v4.1.0. I think the change to package.json is something others will run into, as I can't see how that is specific to my setup. With respect to tsconfig.json I'm still researching the impact of that change, I don't think the module choice of a package should bleed upstream to consumers, but I'm still researching this. But perhaps that should be configurable as part of the --additional-properties flag?

[fuzzzerd]

FWIW: the error I get at run time, is this:

ReferenceError: exports is not defined
runtime.js:73
Object.defineProperty(exports, "__esModule", { value: true });

as mentioned above, changing the generated tsconfig.json to "module": "esnext" clears this up. I'm thinking perhaps its related to webpack dynamic importing? Per here: https://davidea.st/articles/webpack-typescript-code-split-wont-work

[fuzzzerd]

I also found this: suggesting esnext is more appropriate for client side apps: https://mariusschulz.com/blog/dynamic-import-expressions-in-typescript

[macjohnny]

@fuzzzerd

The generated tsconfig.json specifies "module": "commonjs" and in my project I was using "module": "esnext"

this is the reason why the generated api client should be either

• compiled and published, which allows it to be consumed independently of the module setting, or
• used in your code, without the generated tsconfig.json, but this increases the requirement for the generated code to adhere to your project setup/tsconfig.json

you can also specify the tsconfig.json in .openapi-generator-ignore to avoid it being overwritten, allowing custom changes.

[JFCote]

@fuzzzerd May I suggest we close this issue and continue this conversation in the chat room. Core members, template generator and users should be able to answer your questions.

https://join.slack.com/t/openapi-generator/shared_invite/enQtNzAyNDMyOTU0OTE1LTY5ZDBiNDI5NzI5ZjQ1Y2E5OWVjMjZkYzY1ZGM2MWQ4YWFjMzcyNDY5MGI4NjQxNDBiMTlmZTc5NjY2ZTQ5MGM

I think your main problem is not an OpenApi Generator but a comprehension one. There are multiple options for the fetch client, check the "samples" here: https://github.com/OpenAPITools/openapi-generator/tree/master/samples/client/petstore/typescript-fetch/builds

Unless I'm mistaken, you are generating the default one (without any special parameters). This usually means that you should simply copy it in a folder in your project and import it like any other class in your project. You don't need a "node_module" folder, you don't need to add anything to your package.json.

But just the fact that you have a package.json let me think that you are dealing with a node project so you should consider using the npm version with the appropriate generator options. But if you use it that way, you should publish it to be able to import it. We use Verdaccio where I work as a repository.

Again, don't worry, I have been there not so long ago :) Explore the different options of the fetch generator and find the one for you. Don't hesitate to ask for help in the official chat room.

[JFCote]

Closing this issue and continuing discussion in chat. If we found a real problem, let's create a new issue with the problem.

[rob-deutsch]

I have run into many issues with this breaking change to openapi-generator typescript-fetch and it seems strongly like #3403 should be reverted.

Unless I'm mistaken, you are generating the default one (without any special parameters). This usually means that you should simply copy it in a folder in your project and import it like any other class in your project.

It is not that simple because typescript-fetch creates a new tsconfig.json file. This typescript generator should not generate a tsconfig.json file.

#3403 has changed typescript-fetch to generate both:
a) Typescript code (which it always has and always should); and
b) NPM package infrastructure (which should be out of scope)

typescript-fetch should not output code which assumes its going to be used in an NPM package. Not all javascript nor typescript gets put in an NPM package.

It especially should not output code which assumes it will be the primary constituent of an NPM package.

If people want openapi-generator to also be able generate scaffolding for an NPM package then I would suggest that either:
a) A specific config class should be created for an NPM package
b) The Javascript config class should have an option to output types

It seems illogical that typescript-fetch is currently outputting typescript code which can only be used to generate Javascript code (throwing away the Typescript code after doing so) to be used as an NPM package.

[macjohnny]

@rob-deutsch I agree that this change caused some troubles, which I was already concerned about before it was merged (#3403 (review))
I propose to only generate an npm-package if the option npmName is given, similar to the typescript-angular generator:

openapi-generator/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/TypeScriptAngularClientCodegen.java

Lines 147 to 149 in 99204ec

	if (additionalProperties.containsKey(NPM_NAME)) {
	addNpmPackageGeneration(ngVersion);
	}

@rob-deutsch would you like to implement this?

[JFCote]

@macjohnny I agree, I've come to the same conclusion in another issue but I can't find it right now. It seems I forgot an "if" when I did the PR. I currently don't have the time to do it myself but if @rob-deutsch do it, I'll be happy to review it.

[brendandburns]

I wanted to revisit this, we want to switch to this generator, but we are generating as a sub-folder within a larger npm package. This means we can't use the typescript-fetch generator because it hard codes the src path when the right home for this code in our package is src/gen/...

Can we add a flag to opt out of this path somehow?

[TiFu]

Would simply adding a symlink pointing from src/gen/... to the right folder in the generated ts-fetch code solve your issue? (that will only reliably work if you know which OS your stuff will be running on - should work quite well under Linux)

[macjohnny]

@brendandburns according to this change
https://github.com/OpenAPITools/openapi-generator/pull/4472/files#diff-b6db0f9dfdf7043aa45fbeb47131dcebR101-R103

the src path should only be used / generated if an npmName is passed. so if you want to opt out, simply omit the npmName