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

Fractal Project Structure #684

Merged
merged 20 commits into from
Apr 21, 2016
Merged

Fractal Project Structure #684

merged 20 commits into from
Apr 21, 2016

Conversation

justingreenberg
Copy link
Contributor

@justingreenberg justingreenberg commented Apr 1, 2016

RFC: Fractal Project Structure

Please provide feedback! Structure excerpt from README (Updated 4/11/16):

.
├── bin                      # Build/Start scripts
├── blueprints               # Blueprint files for redux-cli
├── build                    # All build-related configuration
│   └── webpack              # Environment-specific configuration files for webpack
├── config                   # Project configuration settings
├── interfaces               # Type declarations for Flow
├── server                   # Koa application (uses webpack middleware)
│   └── main.js              # Server application entry point
├── src                      # Application source code
│   ├── main.js              # Application bootstrap and rendering
│   ├── components           # Reusable Presentational Components
│   ├── containers           # Reusable Container Components
│   ├── layouts              # Components that dictate major page structure
│   ├── static               # Static assets (not imported anywhere in source code)
│   ├── styles               # Application-wide styles (generally settings)
│   ├── store                # Redux-specific pieces
│   │   ├── createStore.js   # Create and instrument redux store
│   │   └── reducers.js      # Reducer registry and injection
│   └── routes               # Main route definitions and async split points
│       ├── index.js         # Bootstrap main application routes with store
│       ├── Root.js          # Wrapper component for context-aware providers
│       ├── Home             # Fractal route
│       │   ├── index.js     # Route definitions and async split points
│       │   ├── assets       # Assets required to render components
│       │   ├── components   # Presentational React Components
│       │   ├── container    # Connect components to actions and store
│       │   ├── modules      # Collections of reducers/constants/actions
│       │   └── routes **    # Fractal sub-routes (** optional)
│       └── NotFound         # Capture unknown routes in component
└── tests                    # Unit tests

Fractal App Structure

Also known as: Self-Contained Apps, Recursive Route Hierarchy, Providers, etc

Small applications can be built using a flat directory structure, with folders for components, containers, etc. However, this structure does not scale and can seriously affect development velocity as your project grows. Starting with a fractal structure allows your application to organically drive it's own architecture from day one.

We use react-router route definitions (<route>/index.js) to define units of logic within our application. Additional child routes can be nested in a fractal hierarchy.

This provides many benefits that may not be immediately obvious:

  • Routes can be be bundled into "chunks" using webpack's code splitting and merging algorithm. This means that the entire dependency tree for each route can be omitted from the initial bundle and then loaded on demand.
  • Since logic is self-contained, routes can easily be broken into separate repositories and referenced with webpack's DLL plugin for flexible, high-performance development and scalability.

Large, mature apps tend to naturally organize themselves in this way—analogous to large, mature trees (as in actual trees 🌲). The trunk is the router, branches are route bundles, and leaves are views composed of common/shared components/containers. Global application and UI state should be placed on or close to the trunk (or perhaps at the base of a huge branch, eg. /app route).

Layouts
  • Stateless components that dictate major page structure
  • Useful for composing react-router named components into views
Components
  • Prefer stateless function components
    • eg: const HelloMessage = ({ name }) => <div>Hello {name}</div>
  • Top-level components and containers directories contain reusable components
Containers
  • Containers only connect presentational components to actions/state

    • Rule of thumb: no JSX in containers!
  • One or many container components can be composed in a stateless function component

  • Tip: props injected by react-router can be accessed using connect:

      // CounterMusicQueryContainer.js
      import { connect } from 'react-redux'
      import Counter from 'components/Counter'
    
      export default connect((state, ownProps) => ({
        counter: state.counter,
        music: ownProps.location.query.music // why not?
      }))(Counter)
    
      // Location -> 'localhost:3000/counter?music=reggae'
      // Counter.props = { counter: 0, music: 'reggae' }
Routes
  • A route directory...
    • Must contain an index.js that returns route definition
    • Optional: assets, components, containers, redux modules, nested child routes
    • Additional child routes can be nested within routes directory in a fractal hierarchy.

Note: This structure is designed to provide a flexible foundation for module bundling and dynamic loading. Using a fractal structure is optional—smaller apps might benefit from a flat routes directory, which is totally cool! Webpack creates split points based on static analysis of require during compilation; the recursive hierarchy folder structure is simply for organizational purposes.

Summary:

  • Migrate to a Fractal Project Structure
    • Code-splitting and async reducers, demo:
      • Watch network tab and redux extension
      • Navigate to /counter route
      • Async chunks load over network and reducers are injected into store
    • Add some tests for new components
      • Update current tests to use enzyme APIs correctly (all except CoreLayout, I think)
    • Update project structure
    • Update README with some explanation
  • Use pure Webpack HMR
    • Apply necessary module.hot instrumentation
    • Remove react-transform babel plugin from development builds
    • Use redbox-react to display render errors
    • Update to React-v15.0.0
  • Remove Redux DevTools Components
  • Improve idiomatic use of react and redux

Please leave questions/comments

@dvdzkwsk
Copy link
Owner

dvdzkwsk commented Apr 1, 2016

This looks super interesting. One question though, since I see you've incorporated Dan Abramov's suggestion for removing react hmr plugins: I've been following the discussion about explicitly using vanilla webpack HMR for hot reloading, but I'm not sure I completely understand the impetus or rationale -- it strikes me as caving to the "JavaScript Fatigue" atmosphere.

The original reason react-hmr even became a thing was so that you could maintain local component state and develop on the fly without having to constantly reload and reproduce app state. So, while this new solution solves some other problems, it totally negates the very reason that this plugin was invented in the first place. Thoughts?

Looking forward to the rest of the PR.

@justingreenberg
Copy link
Contributor Author

@davezuko with respect to react-transform, the background story breaks it down better than I could:

Hot module replacement as it is done today is A Bad Idea™

— James Kyle, the original tweet and responses from Dan Abramov and others (Feb 16, 2016)

What you see in this PR is what I recommend for Redux apps after React 15 comes out. I hope to work on a more complete solution that would replace react-transform later.

— Dan Abramov, RFC: remove React Transform from examples #1455 (Feb 28, 2016)

I’ve come to realize that it [react-transform] is not a viable solution in the long term, and I plan to sunset React Transform in a few months in favor of a different project.

— Dan Abramov, Hot Reloading in React (or, an Ode to Accidental Complexity) (March 8, 2016)

TL;DR:

  • Community leaders and other very smart people have been debating this for months, and ultimately decided that react-transform is not a sustainable solution
  • React Transform will be retired in the very near future, so the point is kinda moot anyway
  • In the meantime, the approach used in this PR is the recommended method

The one caveat—as you alluded to—is losing HMR for local component state. I would suggest that local state in a redux app is most likely an edge case (ie Modals) or a design flaw (app/ui state should really be in the store)... I don't think there's a huge impact on DX there, imo. Vanilla HMR as implemented in this PR maintains the store, catches runtime errors in redbox (and removes them), hot reloads all the things, etc. I would bet most people won't notice a difference.

Rationale from Dan's original RFC to replace react-tranform with vanilla HMR:

  • Both plugin transform and proxy wrapping are rather invasive and have edge cases
  • It doesn’t currently reload functional components or container functions like mapStateToProps
  • In Redux we rarely care about preserving component local state when hot reloading! Preserving store state is usually enough.

Pros

  • It is absolutely non-invasive and doesn’t change semantics of your code
  • It hot reloads any components, whether classes or functions
  • It preserves the store state on hot reload
  • It still handles errors on mount and hot reloading “fixes” them
  • It works for functions like mapStateToProps

Cons

  • It does not preserve the local state of components
  • It does not preserve the DOM so the components are remounted

It sounds like he's working on a new implementation of HMR that maintains component state. I suspect it will be a webpack v2 plugin, since facebook relies heavily on it. Any solution will eventually have to work with both stateless as well as stateful functional components (coming in a future version of react).

So yeah, imo vanilla HMR is the right move for now. With react at version 15.0.0-rc.2, we can work out any kinks and roll out with v15... Let me know what you think

@dvdzkwsk
Copy link
Owner

dvdzkwsk commented Apr 2, 2016

Thanks for the reply. I will get back to you on Monday to continue the conversation. As much as I'd like to dig into this more right now I'm supposed to be avoiding unnecessary work due to wrist pains... so... have a great weekend!

@codecov-io
Copy link

Current coverage is 69.41%

Merging #684 into master will decrease coverage by -1.47% as of 183ed96

@@            master    #684   diff @@
======================================
  Files           12      16     +4
  Stmts           79      85     +6
  Branches         0       0       
  Methods          0       0       
======================================
+ Hit             56      59     +3
  Partial          0       0       
- Missed          23      26     +3

Review entire Coverage Diff as of 183ed96

Powered by Codecov. Updated on successful CI builds.

@justingreenberg justingreenberg changed the title (RFC) Migrate to Fractal Project Structure [WIP] RFC: Migrate to Fractal Project Structure Apr 4, 2016
return combineReducers({ router, ...asyncReducers })
}

export const injectReducer = (store, name, asyncReducer) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

todo: object instead of args

@mlusetti
Copy link
Contributor

mlusetti commented Apr 4, 2016

What I like most is the structure. It definitely deserve a go despite HMR or NOT-HMR

@dvdzkwsk
Copy link
Owner

dvdzkwsk commented Apr 4, 2016

One more question which came up while talking about this with a coworker: what determines the need to create a fractal? Taking a fairly trivial CRUD app, imagine switching between a dashboard which deals with a variety of models and a detail view, wouldn't they both want access to the same global state (especially if you've normalized your data)? What about user management, are there two different contexts (a global one for global state [session, etc.]), and one for each app?

The structure looks appealing, but having never used it I'm wanting to better understand how it achieves modularity beyond folder structure, and how that modularity ties into a cohesive application.

@justingreenberg
Copy link
Contributor Author

The need to create fractals would really be determined by domain requirements and constraints. The goal (obviously) is to encapsulate logic and create natural split points in your codebase.

Trees (like, trees that live in a forest 🌲) are similar to fractal apps—the trunk is the router, branches are route bundles, and leaves are views composed of common/shared components/containers. Global application and UI state would ideally be placed on or close to the trunk (or perhaps at the base of a huge branch, eg. /app route).

Sometimes it is also useful to reflect the route hierarchy in the project's folder structure, ie.

routes/
  App/            # Application (UI State, Authenticated)
    routes/
      Dashboard/  # Dashbord (Default)
      List/       # Master/Detail View
  Auth/           # Session management (Anonymous Routes)

However, this nested route/folder structure is probably not necessary for a trivial CRUD app. Since bundling is handled by webpack compiler, the folder structure itself is really just for organization (development affordances ie. sub-repos, DLLs, etc). In small apps, it probably makes more sense to use a flat directory structure:

routes/
  App/         # Application (UI State, Authenticated)
  Auth/        # Session management (Anonymous Routes)
  Dashboard/   # Dashbord (Default)
  List/        # List/Detail View

In both approaches, the route definitions would be virtually the same (pseudocode, don't copy-paste!):

import App from 'App'

const App = {
  path: '/',
  onEnter: requireAuth,
  component: App(store), // reducers for application/ui state (sync)

  // async load route bundle and inject reducers (creates split point/branch)
  getComponent: (nextState, cb) =>
    System.import('./Dashboard')
      .then((Dashboard) => cb(null, Dashboard(store)))
}) 

const Auth = {
  path: '/auth'

  // auth routes (anonymous): login, logout, passwordReset, etc
  getComponent: (nextState, cb) =>
    System.import('./Auth')
      .then((Auth) => cb(null, Auth(store)))
}

const List = {
  path: '/list',
  component: ListLayout, // create named sidebar/main views

  // async load route bundle and inject reducers (creates split point/branch)
  getComponents: (nextState, cb) =>
    Promise.all([
      System.import('./List'),
      System.import('./Detail')
    ]).then(([ List, Detail ]) => {
      cb(null, {
        sidebar: List(store),
        main: Detail(store)
      }))
    }
}) 

This pattern becomes especially powerful as the app grows into multiple named routes, as you can dynamically load and replace leaf components on completely different branches of your application tree. For example, you could assign a new component to the 'sidebar' route based on an action in auth (ie. new permission)

I'm not the best at breaking down complicated topics, I hope this answers your question :)

@justingreenberg
Copy link
Contributor Author

I forgot to address your point about global normalized data... imho, creating materialized (derived) views from normalized data is really the essence of using container components with selectors, as opposed to reducing into global state atom... I added some comments to CounterContainer to explain the pattern:

// Note: mapStateToProps is where you should use `reselect` to create selectors, ie:

import { createSelector } from 'reselect'

const counter = (state) => state.counter
const tripleCount = createSelector(counter, (count) => count * 3)
const mapStateToProps = (state) => ({
  counter: tripleCount(state)
})

From https://github.com/reactjs/reselect docs:

  • Selectors can compute derived data, allowing Redux to store the minimal possible state.
  • Selectors are efficient. A selector is not recomputed unless one of its arguments change.
  • Selectors are composable. They can be used as input to other selectors.

@dvdzkwsk
Copy link
Owner

dvdzkwsk commented Apr 4, 2016

I totally understand the container component pattern, and a huge 👍 for reselect; I think I was just getting caught up in creating too many fractals (and with them stores) where there are a variety of competing redux states (which confused me as to how you would "select" from the right state atom). Do you think https://www.npmjs.com/package/react-redux-provide becomes useful at all in this setup?

@justingreenberg
Copy link
Contributor Author

i love the concept of redux-provide—i've been following it closely! another similar project is https://github.com/artsy/react-redux-controller. both of these look awesome, but they they are so new i'd need to see how they are maintained, tested, etc before relying too heavily on it. for now, i'm leaning towards something like this PR (maybe paired with redux-saga) and letting the dust settle. the main idea is getting our business logic into units, which we've already done to a large extent with redux modules.

I think I was just getting caught up in creating too many fractals (and with them stores)

i should note that fractals do not create their own stores; we pass the instantiated store to route config which uses injectReducer to create a key on global state atom. maybe it's the "fractal" terminology that's confusing, since nesting is optional?

update: i just noticed react-redux-provide got a rewrite, which is a good sign :) i would love to it as as the foundation for the kit once it's stable. i had some initial concerns around the fact it didn't play well with react-router (they give you their own page provider). i'm going to run through the source :)

@rsilvestre
Copy link
Contributor

I like the idea. I will fork the branch and work on the internationalisation like @juanda99 did for the current react redux starter kit

@SpencerCDixon
Copy link
Contributor

Big +1 for reselect. I really like all of the ideas presented here! @justingreenberg I think it will be important to do a good job documenting how/when/why to use the fractal pattern and link to resources on how people can get perf benefits by using code splitting when they need it

@dvdzkwsk
Copy link
Owner

dvdzkwsk commented Apr 5, 2016

I think it's unanimous that reselect is awesome, but I don't think it's necessary to include it in the starter kit. Right now I think a lot of people are caught up in the hype of tooling without understanding everything involved (a major source of issues for this project, and some of my own misunderstandings), so I'd really like to limit the amount of stuff included, allowing people to naturally discover tools as they understand the problems they aim to solve (though we can definitely point them in the right direction).

That aside, definitely 👍 for continuing down the proposed path.

@rsilvestre
Copy link
Contributor

I add ruffly the translation support on the fort made by @juanda99.

Here is the branch : https://github.com/juanda99/react-redux-starter-kit/tree/fractal

I will refactor the code later, time to sleep.

@justingreenberg : 👍 for your RFC. Can you have a look on the branch I'm talking about, I place the redux stuff for the language in the store/modules, but i'm pretty sure there is a better place ?

@@ -280,7 +266,7 @@ if (!__DEV__) {
).forEach((loader) => {
const [first, ...rest] = loader.loaders
loader.loader = ExtractTextPlugin.extract(first, rest.join('!'))
delete loader.loaders
Reflect.deleteProperty(loader, 'loaders')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Woah, never seen this before.

@dvdzkwsk
Copy link
Owner

Master branch has been updated to stable ^15.0.0 for React, so if we could rebase this we can definitely move forward.

@@ -297,6 +341,10 @@ Have more questions? Feel free to submit an issue or join the Gitter chat!
Troubleshooting
---------------

### Redux DevTools

We recommend using
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should just link to the section you added above?

@dvdzkwsk dvdzkwsk mentioned this pull request Apr 13, 2016
@justingreenberg
Copy link
Contributor Author

@davezuko sounds good 👍 i agree it's probably ok to merge without blueprints, since they can always be generated and then moved to a more permanent home if necessary. i'm not sure if redux-cli handles folder creation, but if not we could add a containers folder with .gitkeep to src

@justingreenberg justingreenberg changed the title RFC: Migrate to Fractal Project Structure Fractal Project Structure Apr 13, 2016
@@ -2,7 +2,7 @@ import { injectReducer } from '../../store/reducers'

export default (store) => ({
path: 'counter',
getComponents (location, next) {
getComponent (nextState, next) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

per latest release:
https://github.com/reactjs/react-router/blob/master/upgrade-guides/v2.2.0.md

looks like the roadmap has a strong emphasis on improving async routing, so this is def a step in the right direction

@dvdzkwsk
Copy link
Owner

Also: thinking about it, I may cut a 2.0.0 release and move this to 3.0.0-alpha because I'd like to have a stable version with the old HMR system.

@dvdzkwsk
Copy link
Owner

@justingreenberg hoping to get this merged today. I will cut a 2.0.0 release just so people have something to reference that more closely resembles the older structure, but then we can move forward with this. There are a few things I'd probably like to clean up but as long as master is working it's not that big of a deal.

@xquezme
Copy link

xquezme commented Apr 18, 2016

Use pure Webpack HMR

gaearon/react-hot-loader@0cb9e65

@dvdzkwsk
Copy link
Owner

@xquezme we can do that as a followup PR, already have an issue for React Hot Loader 3.0 here: #726.

@rsilvestre
Copy link
Contributor

rsilvestre commented Apr 19, 2016

The bug in CI has been fixed here for the factal branch of my fork : https://github.com/rsilvestre/react-redux-starter-kit/tree/fractal. commit : d8c0ba5

This fork include the internationalization

You need to delete this file : src/redux/configureStore.js

@justingreenberg
Copy link
Contributor Author

@rsilvestre good catch 👍 thank you!

@ralyodio
Copy link

any idea when this project will stabilize? This is the 2nd time I've had to start over.

@dvdzkwsk
Copy link
Owner

dvdzkwsk commented Apr 20, 2016

@chovy Sorry to hear that. Perhaps it deserves a better description, but this repo is more of a living project than a completely stable and static boilerplate. So much is constantly changing in the JS community that things get out of date quickly.

Because of that, I don't recommend trying to stay at the very tip of the development spear just because it exists. We do it for the starter kit because we're trying to provide our interpretation of the best tools and practices, but at almost any point in time (namely tagged releases) you should be able to just take that version and run with it. A few of projects I work on, at both work and home, are based on older versions of the starter kit and work perfectly fine. If you do try to stay with the latest and greatest you're going to experience growing pains, whether it's from broken/upgraded packages, design philosophy changes, or evolving best practices. It's up to you which to choose.

And, as far as this RFC, sometimes there are just better ways of doing things than we'd previously considered. @justingreenberg made a profoundly good case for moving in this direction and has received multiple nods of support, so that's the direction we'll go. I'd love to say "This is it" and declare the project stable, but that won't stop the community from continuing onward. The best I can do is create tagged releases, and, what I can do better than I am, is indicate to users what exactly those tagged releases include, so they can make the best decision for them and their team.

@dvdzkwsk
Copy link
Owner

@justingreenberg one final question, about injectReducer: do we also need some sort of removeReducer to remove async reducers after the route is unmounted? Or do we keep them instantiated in case somebody revisits the route, in which case we have the old state?


// Use Redux DevTools chrome extension
if (__DEBUG__) {
if (!window.devToolsExtension) window.devToolsExtension.open()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@justingreenberg this logic is actually flipped. It tries to open the extension if it doesn't exist.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just went ahead and fixed it: 9e03fbc

@dvdzkwsk dvdzkwsk merged commit 80b3d35 into dvdzkwsk:master Apr 21, 2016
@dvdzkwsk
Copy link
Owner

Merged 🎉 , great work @justingreenberg... as always, showing me how much I don't know.

@ethanve
Copy link

ethanve commented Apr 26, 2016

This project has become my defacto source for my projects so keep up the great work guys and awesome work @justingreenberg! Would you be able to explain how (and maybe add additional newbie docs) the new router/index.js works?

@dvdzkwsk
Copy link
Owner

All props go to @justingreenberg .

@ethanve I would love to get a solid wiki in place in addition to a full README rewrite/cleanup. So much has changed that the original ( ✨ pristine ✨ ) docs are getting a bit too disorganized.

@ghost
Copy link

ghost commented May 6, 2016

Tried very hard to understand the "Fractal Structure", and also convert the 2.0 base project to this structure. I could tell the lazy-load benefit, but no more than that. Also confused about the global components & container vs routes module component & container.

I'm appreciate all you @davezuko and @justingreenberg 's hard working. Love the project.
Willing to hear more about the Ideal case of using this structure.

Thanks.

@BorelTung
Copy link

BorelTung commented Jul 14, 2016

One question about the structure, where should we maintain global Redux modules, which is not belond to any routes (or say views)? Should we just bring src/redux back for this? @justingreenberg

@dvdzkwsk
Copy link
Owner

@BorelTung totally, or src/modules, whatever suits you.

@justingreenberg justingreenberg deleted the fractal branch October 3, 2016 18:26
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.

None yet

10 participants