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

Class-based React components and composition vs. mixins #117

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

BinaryMuse
Copy link
Owner

Changes since initial draft:

  • Ctx -> Context
  • Add wrapStatic which delegates to wrap

React v0.13 brought us native ES6-based React classes, which do not (currently, anyway) support mixins. This is both a blessing and a curse: a curse because Fluxxor's primary workflow depends on it, and a blessing because, overall, mixins are overused and many problems can better be solved by composition over inheritance (and multiple inheritance at that).

I've started to experiment with a composition-based approach to writing Fluxxor apps, and wanted to get an issue open early to get any feedback.

The basic idea is this:

  1. Instead of mixing FluxMixin and StoreWatchMixin into your React components and transferring Fluxxor store state to component state via getStateFromFlux, you would wrap your component in some kind of FluxxorController component.
  2. The FluxxorController is responsible for transferring state from the flux stores into props of your wrapped component.

This has the additional benefit of decoupling components that might otherwise be sloppily coupled to Fluxxor (e.g. now they take props and are more reusable/testable). In order to make FluxxorController as simple to use as possible, I'd also like to provide a terser approach for the cases when you will follow a common pattern (which I believe will be most of the time). I have this approach working with the React Router example in the Fluxxor repo.

FluxxorController takes three properties:

  • fluxxorStores - an array of stores to watch and re-render on, similar to StoreWatchMixin
  • fluxxorProps - A function that takes a Flux instance and returns an object; the keys and values of this object will be passed as props to your component
  • fluxxorExtraContext - An object (or function that returns an object) that will be passed as a second parameter to fluxxorProps; useful for getting access to outside references, e.g. a router instance

Here's an example in use:

<FluxxorController
  fluxxorStores={["recipe"]}
  fluxxorProps={(flux) => ({ recipes: flux.store("recipe").getRecipes() })}
>
  <RecipeList />
</FluxxorController>

In this example, on initial render (and whenever the recipe store changes), FluxxorController will re-render RecipeList, passing a prop called recipes that has a value of flux.store("recipe").getRecipes().

Since this is such a common pattern, FluxxorController.wrap can take the component, stores, props function, and extra context function and create a wrapping component for you. The above code could be rewritten:

FluxxorController.wrap(
  RecipeList,
  ["recipe"],
  (flux) => ({ recipes: flux.store("recipe").getRecipes() })
);

Another pattern that may prove interesting/useful is to declare this data on the React component itself.

class RecipeList extends React.Component {
  render() { ... }
}

RecipeList.fluxxorStores = ["recipe"];
RecipeList.fluxxorProps = (flux) => ({ recipes: flux.store("recipe").getRecipes() })

// later...

var RecipeListWrapped = FluxxorController.wrapStatic(RecipeList);
// RecipeListWrapped can now be passed easily to e.g. React Router

@RickWong
Copy link

RickWong commented Apr 6, 2015

I'll elaborate my tweet-replies here.

  • Perhaps drop the "fluxxor" prefix on the props because it's implicit. And if you're afraid of naming clashes (while I believe a library may reserve relevant prop-names, like stores) instead of props you could use a less common prop-name for example queries, loaders or even a verb like initialize.
  • Try to avoid using abbreviations like in fluxxorExtraCtx. Personally I would prefer extraContext or something totally different that's more specific.
  • A reason to use the Higher-order component (HOC) syntax instead of declaring controller properties as static is to support for multiple instances of that controller. I discovered that while writing react-transmit. In the above static example RecipeList controller would only be useful once, because its stores and props are defined statically. It would not be possible to change how the recipes prop is initialized. If you try to use the controller twice, they would by definition both get the same data. Higher-order components on the other hand have their own state, so they can be programmed at run-time to listen to other stores or have a specific prop initializer like the above <FluxxorController fluxxorStores=... /> example.

@sebastienbarre
Copy link

@BinaryMuse You are absolutely on the right track about deprecating mixins in favor of wrapper components (I call them "adapters", i.e. FluxxorAdapter, to avoid "controller" and other MVC parlance). You might find some inspiration below, if you haven't read these posts already:

Looking forward to see what you will come up with. I would embrace ES6 for the Fluxxor store as well, i.e. have a Fluxxor.Store class instead of a Fluxxor.createStore() function, but that's just me.

@joaovpmamede
Copy link

@BinaryMuse is this something that you're going to add to the lib?
Or should we just take the concept and apply on a new project with React 0.13.x and Fluxxor?

Also, you say "I have this approach working with the React Router example in the Fluxxor repo" is it on another branch?

@BinaryMuse BinaryMuse force-pushed the features/bkt-flux-controller-composition branch from 87bffe0 to f331505 Compare May 1, 2015 21:19
@BinaryMuse
Copy link
Owner Author

@joaovpmamede This will land in Fluxxor in one form or another. I've converted this issue into a PR and attached the code I have working thus far. All the flux-based concerns are now in router.jsx where the wrapped components look like

// This example is the most basic; `RecipeList` is wrapped by a
// component that re-renders it anytime the "recipe" store is changed,
// passing in `flux.store("recipe").getRecipes()` as its `recipes` prop.
RecipeListWrapped = wrap(RecipeList, ["recipe"], (flux) => {
  return {
    recipes: flux.store("recipe").getRecipes()
  };
});

// This example is similar, except no stores are watched. The purpose
// of wrapping the component is to provide a flux action creator
// as one of its properties.
RecipeAdderWrapped = wrap(RecipeAdder, [], (flux) => {
  return {
    onAddRecipe: (name, desc, ingredients, directions) => {
      flux.actions.recipes.add(name, desc, ingredients, directions);
    }
  };
});

And the components themselves have no knowledge of the flux system whatsoever (everything comes through props):

// in RecipeAdder

  onSubmit(e) {
    e.preventDefault();

    var newRecipe = this.refs.form.getValue();
    if (newRecipe) {
      this.props.onAddRecipe(
        newRecipe.name,
        newRecipe.description,
        newRecipe.ingredients,
        newRecipe.directions
      );
    }
  }

Please feel free to give it a look and let me know what you think.

@BinaryMuse BinaryMuse force-pushed the features/bkt-flux-controller-composition branch from 7e2be7f to a2e18c7 Compare May 19, 2015 21:22
@frozenfung
Copy link

Base on this concept I made a simple version to fit our project. PR and issue are welcome.
https://github.com/frozenfung/fluxxor-wrapper

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

Successfully merging this pull request may close these issues.

5 participants