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

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

Open
minecrawler opened this issue Jul 1, 2017 · 5 comments
Open
Milestone

Comments

@minecrawler
Copy link
Collaborator

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.

@Download
Copy link
Owner

Download commented Jul 1, 2017

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.

@minecrawler
Copy link
Collaborator Author

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.

@Download
Copy link
Owner

Download commented Jul 1, 2017

#9

@Download Download added this to the 1.0 milestone Jul 3, 2017
@Download
Copy link
Owner

Download commented Jul 3, 2017

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

@Download Download changed the title Export of internal properties Define which (if any) properties should be exported on mixins Jul 3, 2017
@Download Download mentioned this issue Jul 3, 2017
@mindfullsilence
Copy link

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'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants