[JavaScript][ES6] The Petstore integration tests failed

[wing328]

Description

The Petstore integration tests for JS ES6 failed

/private/tmp/swagger-codegen/samples/client/petstore/javascript-promise-es6/node_modules/babel-core/lib/transformation/file/index.js:600
      throw err;
      ^

SyntaxError: /private/tmp/swagger-codegen/samples/client/petstore/javascript-promise-es6/src/model/AnimalFarm.js: Invalid left-hand side in assignment expression (37:8)
  35 |     constructor() {
  36 |         
> 37 |         this = new Array();
     |         ^
  38 |         Object.setPrototypeOf(this, AnimalFarm);
  39 | 
  40 |         
    at Parser.pp$5.raise (/private/tmp/swagger-codegen/samples/client/petstore/javascript-promise-es6/node_modules/babylon/lib/index.js:4454:13)
    at Parser.pp$2.toAssignable (/private/tmp/swagger-codegen/samples/client/petstore/javascript-promise-es6/node_modules/babylon/lib/index.js:3008:16)

Swagger-codegen version

Latest master.

Steps to reproduce

To repeat the issue for javascript-promise-es6

cd samples/client/petstore/javascript-promise-es6
npm install
npm test

To repeat the issue for javascript-es6

cd samples/client/petstore/javascript-es6
npm install
npm test

Suggest a Fix

If anyone wants to work on the issue, please reply to let us know.

[CodeNinjai]

I'll have a look on this.

[CodeNinjai]

Concerns
https://github.com/swagger-api/swagger-codegen/blob/master/modules/swagger-codegen/src/main/resources/Javascript/es6/partial_model_generic.mustache
Line 22

Actually for this certain issue I suggest to change

this = new Array();

into

Array.apply(this);

I think babeljs will complain reinitializing this in the constructor due to immutability.

constructor({{#vendorExtensions.x-all-required}}{{name}}{{^-last}}, {{/-last}}{{/vendorExtensions.x-all-required}}) {
        {{#parent}}{{^parentModel}}{{#vendorExtensions.x-isArray}}
        this = new Array();
        Object.setPrototypeOf(this, {{classname}});{{/vendorExtensions.x-isArray}}{{/parentModel}}{{/parent}}

        {{#useInheritance}}
        {{#parentModel}}{{classname}}.call(this{{#vendorExtensions.x-all-required}}, {{name}}{{/vendorExtensions.x-all-required}});{{/parentModel}}
        {{#interfaceModels}}{{classname}}.call(this{{#vendorExtensions.x-all-required}}, {{name}}{{/vendorExtensions.x-all-required}});{{/interfaceModels}}
        {{/useInheritance}}

        {{#vars}}{{#required}}this['{{baseName}}'] = {{name}};{{/required}}{{/vars}}

        {{#parent}}{{^parentModel}}return this;{{/parentModel}}{{/parent}}
    }

What do you think?

[wing328]

cc @frol

[frol]

This looks like a bad design in the first place... replacing this doesn't sound like a wise idea. I am not sure what this code does, but it seems to be a hacky implementation of a "factory" pattern. At the very least, it seems that instead of doing reassignment of this (this = new Array()), it can use a local variable and return it insead of this:

const model = new Array();
...
return model;

[CodeNinjai]

@wing328 @frol
I completly agree with that, but not knowing the complete context of this let me be a bit careful as polymorphic effects dont leap out rightoff.

[CodeNinjai]

@wing328 @frol
Guys we have other issues on ES6 as well. Shall we discuss them right here too?

[frol]

@CodeNinjai If you can describe them in separate issues, that would be great since that will make things easier to work on. (Please, CC me if necessary since I don't watch all the issues on the project)

[CodeNinjai]

> export default class AnimalFarm {
>     /**
>     * Constructs a new <code>AnimalFarm</code>.
>     * @alias module:model/AnimalFarm
>     * @class
>     * @extends Array
>     */
> 
>     constructor() {
>         this = new Array();
>         Object.setPrototypeOf(this, AnimalFarm);
>         return this;
>     }
> 
>     /**
>     * Constructs a <code>AnimalFarm</code> from a plain JavaScript object, optionally creating a new instance.
>     * Copies all relevant properties from <code>data</code> to <code>obj</code> if supplied or a new instance if not.
>     * @param {Object} data The plain JavaScript object bearing properties of interest.
>     * @param {module:model/AnimalFarm} obj Optional instance to populate.
>     * @return {module:model/AnimalFarm} The populated <code>AnimalFarm</code> instance.
>     */
>     static constructFromObject(data, obj) {
>         if (data) {
>             obj = obj || new AnimalFarm();
> 
>             ApiClient.constructFromObject(data, obj, 'Animal');
>         }
>         return obj;
>     }
> }

This is the complete snippt of AnimalFarm.js

I do not fully understand the reason why this must be an array type.

[frol]

Can we find the contributor of that code and ping him? :)

Also, related PRs may help to understand how things came into this state.

[wing328]

cc @dinukadesilva who contributed the ES6 templates for JS

[CodeNinjai]

This certain issue fixed with PR #5977