Open wing328 opened 7 years ago
I'll have a look on this.
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?
cc @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;
@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.
@wing328 @frol Guys we have other issues on ES6 as well. Shall we discuss them right here too?
@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)
> 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.
Can we find the contributor of that code and ping him? :)
Also, related PRs may help to understand how things came into this state.
cc @dinukadesilva who contributed the ES6 templates for JS
This certain issue fixed with PR #5977
I will make another commit for the promise-es6
Description
The Petstore integration tests for JS ES6 failed
Swagger-codegen version
Latest master.
Steps to reproduce
To repeat the issue for
javascript-promise-es6
To repeat the issue for
javascript-es6
Suggest a Fix
If anyone wants to work on the issue, please reply to let us know.