feat(createContainer): allow function composition

[levithomason]

Problem

Currently, createContainer cannot be composed. Composing HOCs is a very common need in the React community.

compose(
  createContainer(() => { ... })),
  withJoyride(),
  ResponsiveComponent(),
)(MyComponent)

This cannot be accomplished because createContainer requires all arguments up front. We have to wrap it in order to compose it:

compose(
  MyComponent => createContainer(() => { ... }, MyComponent),
  withJoyride(),
  ResponsiveComponent(),
)(MyComponent)

Solution

This is solved by currying. This PR adds cheap currying to createContainer.

When all arguments all passed, the function is called:

const ContainerComponent = createContainer(() => { ... }, MyComponent)

When partial arguments are passed, a function is returned that accepts the remaining arguments:

const awaitingComponent = createContainer(() => { ... })

const ContainerComponent = awaitingComponent(MyComponent)

[apollo-cla]

@levithomason: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/

[klaussner]

There already is a function for that, withTracker, which was recently added to the react-meteor-data package. :slightly_smiling_face:

[levithomason]

It seems very strange to create a brand new function with a new name that is just a curried version of another.

Looking at withTracker, it actually isn't curried either 😕 It always returns a new function. This means users have to import two different functions, with seemingly unrelated names, depending on how they want to use it:

import { createContainer, withTracker } from 'meteor/react-meteor-data'

const DoesntWork = withTracker(..., MyComponent)
const AlsoDoesntWork = createContainer(...)(MyComponent)

const works = createContainer(..., MyComponent)
const alsoWorks = withTracker(...)(MyComponent)

Why not this?:

import { createContainer } from 'meteor/react-meteor-data'

const works = createContainer(..., MyComponent)
const alsoWorks = createContainer(...)(MyComponent)

It would seem much more clear and efficient to simply curry createContainer so there is one function that works in all cases. I get that withTracker has already been released and there is change cost. However, it seems like the current direction is one that should be corrected rather than embraced.

Worst case, at least curry both createContainer and withTracker so they are more usable, but I don't see a reason to keep two functions that do exactly the same thing.

[dburles]

Hey @levithomason we had some discussion on that (see here https://github.com/meteor/react-packages/pull/215)

[dburles]

More info here: https://github.com/meteor/react-packages/tree/devel/packages/react-meteor-data#note-on-withtracker-and-createcontainer

Simple answer is that there's no reason for anyone to use createContainer anymore.

[levithomason]

Closing as there seems to be little interest in currying these methods.

Thanks @dburles for the extra context, it was very helpful. I still think withTracker should have been curried so it can be called withTracker(a, b) or withTracker(a)(b), however, it is at least composable.

Sidenote, perhaps it is worth adding a warning to createContainer since it is on its way out. This way, users can be notified to switch to withTracker.

[dburles]

No problem! Btw I thought your name was familiar (from the semantic-ui-react repo!).

As for currying the withTracker function I can't really think of much benefit in doing so. Though I'd be interested if you do know a valid reason to.

[levithomason]

Yep, that's me 👋 😄

Though I'd be interested if you do know a valid reason to.

Only preference. I like to call functions once when I only need to call them once.

I've come to love functional programming implementations like lodash/fp or Ramda where all functions are auto curried. It just makes using them really elegant to use in every situation.

export default withTracker(a)(b)

// vs

export default withTracker(a, b)

If meteor were a functional lib and this design decision was going to affect a broader area, I may have had more stake in debating. Definitely not worth bikeshedding over which 2 characters are most optimal to appear between the a and b though: )( vs , 😜

[dburles]

Yeah I do like the auto currying you get with those libraries. I think in this case best to keep it simple; as in most cases you'll want to 'compose' other HoC's alongside it (like recompose) for example.

Btw on an unrelated note. I would really love to see the official semantic integration with Meteor updated! not sure if you're familiar with the situation there.

[levithomason]

Oh, yep. It looks abandoned. I tried to look at updating it but it is pretty hairy and fragile. I have admin rights at Semantic-Org, but I couldn't make sense of the hairball in that package 😬

Meteor and Semantic UI both have their own odd conventions that clash quite a bit. Believe it or not, you cannot install semantic-ui-less source and compile with Meteor without transforming the entire framework. Almost every file needs modifications. The path to upgrading that package seems quite complicated and cumbersome as well. The author actually published a complete build system bridge package just to pre-build Semantic UI so Meteor can digest it. It is a lot more work than it should be.

At work, I've resorted to a bash script that just installs semantic-ui-less and makes it meteor friendly. I'm not a Meteor expert by any means, but I'm wondering what the advantages are of having all the extra steps in order to wrap it in a meteor package. It seems very straightforward to install the npm dep instead.

Do you have any insight there? BTW, we can move this convo over to that repo if you prefer, too.

[dburles]

Yeah I totally get where you're coming from! Let's move the convo here: https://github.com/Semantic-Org/Semantic-UI-Meteor/issues/92