mics icon indicating copy to clipboard operation
mics copied to clipboard

Define which (if any) properties should be exported on mixins

Open minecrawler opened this issue 8 years ago • 5 comments

This code exposes certain properties on the returned constructor. If a user tries to place their own static property in a class with one of those property names, it will be overwritten. In order to counter that problem, Symbols should be used. However, IE11 does not support Symbols and Babel has problems dealing with them.

minecrawler avatar Jul 01 '17 10:07 minecrawler

Yes you are right. However, these properties are clearly documented. To me, they are part of the contract of mix that it returns a mixin, which is an ES5 constructor function with these properties.

If a user tries to place their own static property in a class with one of those property names, it will be overwritten.

Yes. And there is a bit more to it:

  • If the user includes a static function named constructor, that function will be used instead of the default ES5 constructor function. So as the result of mix(). This allows the user to override the default constructor to e.g. implement caching / pools etc. This is already implemented.
  • Not implemented yet, but I am considering doing the same for a static mixin function, allowing the user to hook into the mixing process.
  • The interface object is not strictly needed as it can be derived from the prototype. However I think it is nice to have it available as a property and it is presumably faster as it no longer needs to go through the whole prototype chain.

In fact, class, mixin and interface could be private. There really is no need for them to be exposed. However it may make implementation more difficult. I don't really like using symbols for them (yet) as support will remain bad for some time to come.

Do you have any ideas on how to improve it?

Edit: one simple idea could be to move the class and interface properties and make them a sub of mixin. E.g.:

var Looker = mix(..)

typeof Looker                  // 'function'
typeof Looker.mixin            // 'function'
typeof Looker.mixin.class      // 'function'
typeof Looker.mixin.interface  // 'object'

This wouldn't completely fix the issue but it would make it smaller.

Download avatar Jul 01 '17 15:07 Download

I just opened the issue since I think the solution can be improved, but I do not see any immediate way, other than using Symbols. Even though support is bad, it might already work with Babel in older browsers.

Well, for now, I am doing refactoring, so I leave that here for later.

minecrawler avatar Jul 01 '17 15:07 minecrawler

https://github.com/Download/mics/pull/9

Download avatar Jul 01 '17 23:07 Download

I think we should try to decide on this before 1.0 as it's a part of the Public API right now.

Download avatar Jul 03 '17 10:07 Download

Turning these into something a bit more obscurely named would definitely help. I agree with stubbing, but perhaps stick them all inside a _mics property.

var Looker = mix(..)

typeof Looker                  // 'function'
typeof Looker._mics            // 'object'
typeof Looker._mics.mixin      // 'function'
typeof Looker._mics.class      // 'function'
typeof Looker._mics.interface  // 'object'

mindfullsilence avatar Nov 07 '17 07:11 mindfullsilence