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

API #1

Open
3 tasks done
troutowicz opened this issue May 24, 2015 · 12 comments
Open
3 tasks done

API #1

troutowicz opened this issue May 24, 2015 · 12 comments
Labels

Comments

@troutowicz
Copy link
Member

Let's discuss what the final API should look like. Do we want to include any of stampit's utility methods?

Public Stamp Interface
- [ ] stamp.methods
- [ ] stamp.state
- [ ] stamp.enclose

  • stamp.compose

Utility Methods

  • stampit.compose
  • stampit.isStamp
@ericelliott
Copy link
Contributor

Definitely .compose()

@troutowicz
Copy link
Member Author

I'm also thinking of a utility method that makes the core stampit library available, for times when a mixin may not need to be a React component. Would eliminate the need for importing react-stampit and stampit.

import stampit from 'react-stampit'

const mixin = stampit.stampit();

@ericelliott
Copy link
Contributor

I don't understand what you mean. Could you provide a more complete use case?

@troutowicz
Copy link
Member Author

import stampit from 'react-stampit';

const nonReactStampMixin = stamit.stampit({
  foo() {}
});

const reactStampMixin = stampit(Render, {
  propTypes: {}
});

const reactStamp = stampit(Render, {
  render() {}
}).compose(nonReactStampMixin, reactStampMixin);

@ericelliott
Copy link
Contributor

Browserify auto-dedupes and npm dedupes in v3+.

Don't you think it makes more sense for people who want direct stampit access to just import stampit from 'stampit';?

@troutowicz
Copy link
Member Author

What you are saying does make total sense, what I was proposing was nothing more than a convenience method. I was considering the use case where a module may have a reason to have a non-react mixin stamp AND and a react stamp in the same file... but that may not be a use case to worry much about. I'll leave the method out.

@ericelliott
Copy link
Contributor

👍

@troutowicz
Copy link
Member Author

Added the compose utility method.

@troutowicz
Copy link
Member Author

@ericelliott
Thoughts on deep merging React static props at composition? We would exclude defaultProps due to perf reasons.

So this feature would apply to:
* contextTypes
* childContextTypes
* propTypes

Created #2 with more details on my proposed composition implementation.

@troutowicz
Copy link
Member Author

@ericelliott Merged PR #2, composition should be pretty much fully functional for React. Let me know if you see anything missing or have other ideas!

@ericelliott
Copy link
Contributor

This API is looking great so far. I'm going to start using it in my production apps and see how it works for me.

BTW, thanks for helping to get those issues resolved in React. =)

👍

@troutowicz
Copy link
Member Author

Sounds great! You're welcome, I'm glad they were accepted.

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

No branches or pull requests

2 participants