Skip to content
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

[JavaScript] index.js and ApiClient.js generated to incorrect location #1150

Closed
demonfiddler opened this issue Oct 1, 2018 · 6 comments · Fixed by #2511
Closed

[JavaScript] index.js and ApiClient.js generated to incorrect location #1150

demonfiddler opened this issue Oct 1, 2018 · 6 comments · Fixed by #2511

Comments

@demonfiddler
Copy link
Contributor

demonfiddler commented Oct 1, 2018

Description

Prior to the swagger-codegen -> openapi-generator fork, JavaScriptClientCodegen had already been broken, and remains so. The breakage is that two of the supporting files, ApiClient.js and index.js, are now generated to the hard-coded ${output}/src directory, ignoring the sourceFolder configuration parameter. This can break their module references to the api and model modules if either a sourceFolder other than src is specified or if an invokerPackage is specified. The two files need to be moved back into ${sourceFolder}/${invokerPackage}, otherwise nothing will work! The sample in swagger-codegen/samples/client/petstore/javascript does not override sourceFolder nor does it specify an invokerPackage, so it looks like this, which does work:

src/
    api/
        PetApi.js
        ...
    model/
        Animal.js
        ....
    ApiClient.js
    index.js (requires api/*, model/*)

As things currently stand it is no longer possible to generate a project with this structure if you override sourceFolder and/or invokerPackage. Instead, you'll end up with:

${sourceFolder}/
    ${invokerPackage}/
        api/
            PetApi.js
            ...
        model/
            Animal.js
            ....
src/
    ApiClient.js
    index.js (requires unresolvable api/*, model/*)

I am quite familiar with this code, as a couple of years ago I submitted several PRs, including one to get proper JSDoc comments that can be parsed by both JSDoc and Tern.js.

In addition, the test specification files are generated to ${outputFolder}/test/api/*ApiTest.spec.js and ${outputFolder}/test/model/*.spec.js and contain define() / require() calls that expect the index file to be at (hard-coded) ../../src/index. Given that the proposed fix entails moving index.js and ApiClient.js back to ${sourceFolder}/${invokerPackage} where they belong, the test spec files will have to account for the possbility that the generator configuration might specify an invokerPackage, in which case the index file would be located at ../../${sourceFolder}/${invokerPackage}/index. Furthermore, the output location of the test specification files does not itself currently include ${invokerPackage} and as a consequence when performing multiple YAML generations to the same output folder there is the possibilility of name collisions between identically named APIs or models from different YAMLs. To guard against this possibility the spec files should instead be generated to test/${invokerPackage}/api/*ApiTest.spec.js and test/${invokerPackage}/model/*.spec.js, and the relative location of the index files adjusted to suit.

openapi-generator version

3.2.1-SNAPSHOT

OpenAPI declaration file content or url

I am not permitted to post the proprietary YAMLs for which this effect is observed but it should be observable with any YAML (e.g., PetStore).

Command line used for generation

Generated using openapi-generator-maven-plugin:

<configuration>
    <generatorName>${language}</generatorName>
    <inputSpec>${openapi.yaml.directory}/ae.yaml</inputSpec>
    <output>${openapi.output.directory}/ae</output>
    <groupId>${project.groupId}</groupId>
    <artifactId>${project.artifactId}</artifactId>
    <artifactVersion>${project.version}</artifactVersion>
    <apiPackage>${api.package.ae}</apiPackage>
    <modelPackage>${model.package.ae}</modelPackage>
    <invokerPackage>ae</invokerPackage>
    <configOptions>
        <sourceFolder></sourceFolder>
        <artifactName>XXX AE Microservice</artifactName>
    </configOptions>
</configuration>
Steps to reproduce

Generate the JavaScript then attempt to load the generated index.js or ApiClient.js using AMD or CommonJS - this will fail because src/index.js and ApiClient.js refer to modules under ${invokerPackage}.

Related issues/PRs

Possibly #1046

Suggest a fix/enhancement

Revert the swagger-generator change that moved the output location of these two files:

    final String[][] JAVASCRIPT_SUPPORTING_FILES = new String[][]{
            ...
            new String[]{"index.mustache", "src/index.js"},
            new String[]{"ApiClient.mustache", "src/ApiClient.js"},
            ...
    };

which used to be configured like this in the preprocessSwagger(Swagger) method:

        supportingFiles.add(new SupportingFile("index.mustache", createPath(sourceFolder, invokerPackage), "index.js"));
        supportingFiles.add(new SupportingFile("ApiClient.mustache", createPath(sourceFolder, invokerPackage), "ApiClient.js"));

I forgot to mention: the current state of affairs prevents you from generating multiple YAMLs to different invokerPackages then aggregating the results, since the index.js and ApiClient.js files are now shared between all executions and thus can only relate to the first of these (subsequent generations skip these files).

@macjohnny
Copy link
Member

@demonfiddler
Copy link
Contributor Author

@macjohnny I'll see if I have the time available but I might be reassigned to another task soon. My primary focus is finishing a PR for some enhancements to the JAXRS-CXF generator. Will let you know.

@InfoSec812
Copy link
Contributor

InfoSec812 commented Oct 23, 2018

As a workaround, you can try adding the following webpack config:

resolve: {
  alias: {
    'my': 'my/src'
  }
}

@demonfiddler demonfiddler changed the title [JavaScript] [JavaScript] index.js and ApiClient.js generated to incorrect location Oct 24, 2018
@demonfiddler
Copy link
Contributor Author

@macjohnny I've just been looking at the samples. I can see that there are various profiles to include the samples in the build (and thus run their tests) but I couldn't find any automated means of regenerating their content - which presumably is a prerequisite for a generator regression test. How does this work?

@macjohnny
Copy link
Member

@demonfiddler you can add the sample generation script to the „ensure-samples-uptodate“ script which is automatically run in CI and ensures that the samples are up to date by re-generating the samples and checking the git status.

See e.g. #444

@bicatu
Copy link

bicatu commented Feb 5, 2019

Hi, any updates on this issue? Maybe remove the js as an available language until this gets solved?

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