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

added applyRouterMiddleware #3327

Merged
merged 1 commit into from
Apr 16, 2016
Merged

added applyRouterMiddleware #3327

merged 1 commit into from
Apr 16, 2016

Conversation

ryanflorence
Copy link
Member

@ryanflorence ryanflorence commented Apr 15, 2016

I'll add some docs in @knowbody's new docs site thing, but here's the quick version:

// end users put the middleware in the `render` prop
<Router render={applyRouterMiddleware(useFoo())}/>

// a middleware returns an object w/ two methods
const useFoo = () => ({
  // this will wrap `RouterContext` to participate in rendering at the top level of
  // the Router rendering tree
  renderRouterContext: (child, renderProps) => (
    <FooRootContainer>{child}</FooRootContainer>
  ),

  // this will wrap the route components when RouterContext renders them
  renderRouteComponent: (child, routeComponentProps) => (
    <FooContainer>{child}</FooContainer>
  )
})

// typically these things just add something to context
const FooRootContainer = React.createClass({
  propTypes: { children: React.PropTypes.node.isRequired },
  childContextTypes: { foo: React.PropTypes.string },
  getChildContext() { return { foo: FOO_ROOT_CONTAINER_TEXT } },
  render() {
    return this.props.children
  }
})

// so that the route component containers or other components anywhere in the tree
// can access it
const FooContainer = React.createClass({
  propTypes: { children: React.PropTypes.node.isRequired },
  contextTypes: { foo: React.PropTypes.string.isRequired },
  render() {
    const { children, ...props } = this.props
    const fooFromContext = this.context.foo
    return cloneElement(children, { ...props, fooFromContext })
  }
})

@ryanflorence ryanflorence force-pushed the apply-router-middleware branch 2 times, most recently from c8da3ee to b8a7baf Compare April 15, 2016 18:50
@ryanflorence ryanflorence added this to the next-2.3.0 milestone Apr 15, 2016
import RouterContext from './RouterContext'

export default (...middlewares) => {
const withContextContainer = middlewares.filter(
Copy link
Contributor

Choose a reason for hiding this comment

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

Slightly more optimized to write this as

middlewares.map(m => m.renderRouteComponent).filter(f => f)

and then skip destructuring in the reduce callback below.

@taion
Copy link
Contributor

taion commented Apr 15, 2016

What do you think of renaming the callbacks to:

  • renderRouterContext
  • renderRouteComponent

They're not constrained to be containers – e.g. useBaz specifically can just have

renderRouteComponent: child => cloneElement(child, { bazInjected })

@ryanflorence ryanflorence force-pushed the apply-router-middleware branch from b8a7baf to 58e2c9a Compare April 15, 2016 19:39
@@ -2,9 +2,11 @@
> Unreleased

- **Minor:** Speed up checking index path active status ([#3313])
- **Minor:** Added `applyRouterMiddleware` ([#3327])
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be Feature:, I think.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@taion
Copy link
Contributor

taion commented Apr 15, 2016

LGTM.

@ryanflorence ryanflorence force-pushed the apply-router-middleware branch 2 times, most recently from 547a4d1 to e51310b Compare April 15, 2016 19:43
@edygar
Copy link
Contributor

edygar commented Aug 24, 2016

So, if I had a custom render previously, I would have to inject the enhanced RouterContext, right? Kind of as following:

const RouterContextFactory = applyRouterMiddleware(useFoo());

<Router
  render={renderProps =>
    <Fetcher {...renderProps}>
      {RouterContextFactory(renderProps)}
    </Fetcher>
  }
/>

@GGAlanSmithee
Copy link

@ryanflorence thanks for adding this. Do you know if this ended up being documentet anywhere? Could not find anything in the official docs or in the referenced @knowbody docs. Would you want a PR to either?

@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
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