Skip to content
This repository has been archived by the owner on Sep 10, 2022. It is now read-only.

Add create component and create pure component functions #136

Closed
wants to merge 5 commits into from

Conversation

calebmer
Copy link

I've had these helper functions in a couple of projects I've worked on and I figured it's time to see if I could get them added to recompose. This is mostly a stylistic preference I have, I find it preferable to write:

export default createComponent(
  setPropTypes(),
  ({ foo }) => 
)

over:

export default compose(
  setPropTypes()
)(({ foo }) => )

The indentation is consistent with the first example and there aren't extra annoying parentheses (and especially no )( which really drives me crazy).

In my projects I also have createComponent attach the pure decorator, but as @istarkov pointed out in #135 that may not be preferable for most scenarios. By adding a createPureComponent function it helps a common case by reducing a module's dependency count and removing a line of code.

One of the barriers for adopting recompose for me in the past is there wasn't really a composition pattern that I subjectively liked, these helpers changed that story for me. The biggest win of these functions for this library is it increases the attractiveness of this superior pattern.

The names are debatable, I also considered enhanceComponent.

@istarkov
Copy link
Contributor

Thank you for your contribution. Having a source I can say.

In most of my project I export composed HOCs with Component, like

export const componentHOC = compose(
blabla,
blublu
);

export default componentHOC(component);

real example

This allows me to reuse-extend HOCs in other similar components reuse example

I think that having non compose syntax it will be not clear that HOCs can be reused.

Also is this so big difference to extend api surface
createComponent(a,b,c) - vs - compose(a, b)(c) ?

The smaller API the easier to work with.

@wuct
Copy link
Contributor

wuct commented Apr 23, 2016

I discussed "recompose pattern"s with my colleagues, and we coincidentally chose the same pattern @istarkov mentioned above. Here is our discussion(we chose the 4th pattern in the end).

There are many advantages of this pattern. One of them is letting us test our HOCs and components separately. It makes testing become simpler.

@calebmer
Copy link
Author

I don't see a problem with growing the API surface area especially if it's to support a pattern that is visually preferable to some.

@istarkov that's a fair point about reusability, but if you wanted reusability of HOCs there is nothing stopping you from still doing that with this pattern. Using this library in the first places requires some literacy about functional programming so I don't think education on reusability is an issue.

As for API surface area, this doesn't add a lot to bundle size and with ES module optimizations perhaps none at all. For ease to work with, I believe that if a function is useful to a small proportion of people and it fits in the library it should be added (think about left pad which has made it into the JavaScript standard library!). If recompose's goal is to be the lodash of React, fear of growing API surface area should not be a blocker.

@wuct I see you didn't evaluate this pattern though, so in this PR I encourage us to debate it's merit against the other approaches. createComponent is most similar to 3 and your CON there was introduction of display name. The nice thing about having a specialized helper method is we can detect if a string is the first argument and if so set the display name (I feel this is a common enough pattern).

So your option 5 would be:

export default createComponent(
  'Component',
  // HOC
  ({
    // …destructured props
  }) => (
    // react element
  )
)

As for testing, again specialized helper functions are awesome because we can add a baseComponent static property to the higher order component to allow you to drop down a level in testing.


If there is this much debate over how to create higher order components, I think the patterns should be seriously reconsidered and one chosen as the idiomatic way. This is so there is no confusion on how this library fits in one's application. Of course I believe a helper function specifically designed for creating components is the right way, but I'm open to persuasion 😊

btw: I added some of the features I mentioned above.

@calebmer
Copy link
Author

I also removed createPureComponent, it's unnecessary. Also, is CI supposed to pass on failing tests?

@calebmer calebmer force-pushed the feat/create-component branch from f590ee2 to 6d30e11 Compare April 23, 2016 12:08
@istarkov
Copy link
Contributor

istarkov commented Apr 23, 2016

I'll try to explain my vision why current compose solution is better in other words.
As a programmer I like when method names is self describable.
I like if the code I look into gives me additional information without documentation.

Looking on code with compose(blabla, blublu)(component) I can without documentation suppose that with high probability blabla and blublu has the same interface, and this interface is something like Component -> Component (as almost everybody knows compose function).
I can suppose that this call also will return Component (without descriptive name as compose is well known). I can suppose the execution order and having that used compose not flow even get the idea of how internally all of this is composed. That I can compose any number of functions. Not a small piece of information.

Having just createComponent I can't say much about - the only thing that it returns Component and take as input some arguments.

Also as written above compose implicitly causes you to use good patterns.

@calebmer
Copy link
Author

@istarkov that's true, but I don't think it applies for everyone. I think there are some people working in the same codebase for long periods of time with 100+ components who would much prefer a convention over repeated code.

Adding this createComponent convention does not hurt your workflow and the workflows of everyone else who prefers explicitly defining their components, but it does help people (like myself) who prefer a strong convention in areas that see a lot of repetition. In this case that repetition is component boilerplate.

@acdlite
Copy link
Owner

acdlite commented Apr 23, 2016

The problem is that by adding createComponent as an official API, we're implicitly blessing it over other patterns.

Personally, like @istarkov and @wuct, I prefer to separate the higher-order component part from the base component (I like to call it enhance, but same thing).

I also don't want to give the impression that Recompose is inherently at odds with the usual ways of creating components, like extending from React.Component or using. React.createClass. It's meant to be complementary, not a total replacement.

Perhaps instead of adding a new API, we should add a doc that describes the different options available for using higher-order components in your component modules?

@calebmer
Copy link
Author

@acdlite yeah, I think that would be best considering this PR is a result of lack of education in the first place. I'll wait to close this PR until there's a PR open with some docs.

Thanks guys 👍

@acdlite
Copy link
Owner

acdlite commented May 9, 2016

Closed in favor of #153

@acdlite acdlite closed this May 9, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants