Skip to content
This repository has been archived by the owner on Jul 30, 2018. It is now read-only.

Using ES6 and TypeScript classes as a base #5

Closed
kitsonk opened this issue Oct 13, 2015 · 4 comments
Closed

Using ES6 and TypeScript classes as a base #5

kitsonk opened this issue Oct 13, 2015 · 4 comments
Milestone

Comments

@kitsonk
Copy link
Member

kitsonk commented Oct 13, 2015

Currently the create and mixin API can accept ES6 and TypeScript classes. There are a few "challenges" with using these, it can especially get confusing with the type inference.

At the moment, none of the constructor functions are executed. This was mainly to avoid any unintended consequences of the constructors. In addition, you cannot directly call ES6 constructors without the new keyword, meaning that several instances would be created and thrown away during construction.

From a disadvantage point, you do not get any of the affects of the constructor. This is very important though when using non-method properties in TypeScript classes as the type inferrance will abstract those properties from the class, but since then TypeScript moves all of that property setting to the constructor on emit, you don't actually get the values defined. For example, this does not work properly with compose:

import compose from 'dojo-compose/compose';

class Foo {
  foo: string = 'bar';
}

const ComposeFoo = compose(Foo);
const foo = new ComposeFoo();
console.log(foo.bar); // outputs `undefined`

I see two paths forward:

  • Explain this behaviour and why.
  • Change the behaviour, using some sort of intelligence in create which would under ES5 pass the instance to the constructors in the class and in ES6 create new instances for each constructor and then mix in anything that wasn't in the prototype.
@maier49
Copy link
Contributor

maier49 commented Oct 30, 2015

If the goal of compose is to move away from the classical inheritance system, then in my mind at least at seems reasonable to offer limited support for interaction with ES6/Typescript classes. Basically the message would be that compose tries to be compatible with existing code that uses those constructs but it doesn't encourage using them.

@mwistrand
Copy link

I'm curious to know how the second path could be implemented cleanly. As far as I can tell, classes could only be passed to compose if their constructors did nothing but set properties, or if any methods called from the constructor were known to have the desired effect in the order they were being called by compose. As @maier49 points out, if the goal is composition over inheritance, then the main target is objects rather than classes. In other words, compatibility with classes is more of nice coincidence than a deliberate design goal (even if TypeScript is very class-centric).

@kitsonk
Copy link
Member Author

kitsonk commented Nov 19, 2015

@mwistrand that is the challenge that I have, there could likely be side-effects, which would have to be worked through, but essentially what I was thinking was something that during the mixin process, that an instance of the class would be created, the own properties would be mixed into the prototype and then own properties of the instance.prototype would be mixed in.

Some considerations are that in ES7, class properties are being proposed.

I agree the intent is the class compatibility as a by product, but in certain ways, we should try to may the byproduct as easy (and unsurprising) as possible.

One other thought, it is theoretically possible to utilise the Reflect API to detect any class properties that are being passed in and then "throw" (or possibly define them), but that would add runtime overhead to the composition process.

@kitsonk kitsonk added this to the alpha.2 milestone Mar 11, 2016
@kitsonk kitsonk modified the milestones: 2016.04, alpha.2 Apr 8, 2016
@kitsonk kitsonk modified the milestones: 2016.05, 2016.04 Apr 26, 2016
@kitsonk
Copy link
Member Author

kitsonk commented Jun 7, 2016

For now, not considering changes to compose, so it is document and explain behaviour.

@kitsonk kitsonk closed this as completed Jun 7, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants