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

Issues with inheritance using babel and ES6 classes #327

Closed
stalniy opened this issue May 6, 2016 · 5 comments
Closed

Issues with inheritance using babel and ES6 classes #327

stalniy opened this issue May 6, 2016 · 5 comments
Assignees

Comments

@stalniy
Copy link
Contributor

stalniy commented May 6, 2016

Description:
I'm trying to use custom recordClass inside mapper. For doing this I extend JSData.Record in order to have all the niceties of schema validation. But validation doesn't work.

Example: https://jsfiddle.net/wyumc5ao/2/

Where is the issue?:
The issue is in __super__ accessor which is defined by extend function. This property is inherited by child class (i.e., Task in the example) from Record class. Babel doesn't set __super__ property on the constructor and then utils.getSuper(...) === Record fails because utils.getSuper returns Component and not Record as it should.

Possible fix:
Add additional check for `hasOwnProperty('super'). Something like this:

    getSuper: function getSuper(instance, isCtor) {
      var ctor = isCtor ? instance: instance.constructor;

      if (!ctor.hasOwnProperty('__super__')) {
         Object.defineProperty(ctor, '__super__', { configurable: true, value: Object.getPrototypeOf(ctor) || ctor.__proto__ }); // eslint-disable-line
      }

      return ctor.__super__ // eslint-disable-line; 
    },

Conclusion:
If you are ok with the fix I'll send pull-request on Sunday

@jmdobry
Copy link
Member

jmdobry commented May 6, 2016

It's okay that __super__ isn't set with ES2015 class inheritance. getSuper will look for __super__ (which is present if you use .extend) or Object.getPrototypeOf(instance.constructor) (which is present if you use class syntax).

Validation doesn't work because according to JSON-schema, you need type: 'object' as a top-level property of the schema. It would probably be best if JSData adds that for you.

So in your JSFiddle, if you add type: 'object' to your schema, validation will work. e.g.

schema: {
  type: 'object',
  properties: {...}
}

@stalniy
Copy link
Contributor Author

stalniy commented May 7, 2016

getSuper returns wrong result for es2015 inheritance because parent class is set as prototype for child class.

That means all static properties are inherited from parent class which has __super__ property. And when you do Child.__super__ it will fallback to Parent.__super__ which eventually returns a truthy result. So, Object.getPrototyOf is never called

On 6 мая 2016, at 22:10, Jason Dobry notifications@github.com wrote:

It's okay that super isn't set with ES2015 class inheritance. getSuper will look for super (which is present if you use .extend) or Object.getPrototypeOf(instance.constructor) (which is present if you use class syntax).

Validation doesn't work because according to JSON-schema, you need type: 'object' as a top-level property of the schema. It would probably be best if JSData adds that for you.

So in your JSFiddle, if you add type: 'object' to your schema, validation will work. e.g.

schema: {
type: 'object',
properties: {...}
}

You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub

jmdobry added a commit that referenced this issue May 7, 2016
@jmdobry
Copy link
Member

jmdobry commented May 7, 2016

That means all static properties are inherited from parent class which has __super__ property

That makes sense.

@jmdobry jmdobry self-assigned this May 7, 2016
@jmdobry
Copy link
Member

jmdobry commented May 7, 2016

Fixed in 3.0.0-beta.5

@stalniy stalniy closed this as completed May 16, 2016
@stalniy
Copy link
Contributor Author

stalniy commented May 16, 2016

Thanks a lot!

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

2 participants