From 61f0a8c0c473244241f5457d7627a61948aab700 Mon Sep 17 00:00:00 2001 From: Michael Jackson Date: Tue, 24 Feb 2015 14:38:53 -0800 Subject: [PATCH] [changed] Deprecate Navigation/State mixins Fixes #835 Fixes #744 --- modules/Navigation.js | 52 +++++++++------ modules/NavigationContext.js | 28 -------- modules/PropTypes.js | 22 ++++-- modules/RouteHandlerMixin.js | 9 ++- modules/State.js | 65 ++++++++++-------- modules/StateContext.js | 101 ---------------------------- modules/TestUtils.js | 8 ++- modules/__tests__/Router-test.js | 8 +-- modules/__tests__/State-test.js | 18 ++--- modules/components/Link.js | 15 ++--- modules/createRouter.js | 111 ++++++++++++++++++++++++------- 11 files changed, 199 insertions(+), 238 deletions(-) delete mode 100644 modules/NavigationContext.js delete mode 100644 modules/StateContext.js diff --git a/modules/Navigation.js b/modules/Navigation.js index 2086a18480..2103f3eb1f 100644 --- a/modules/Navigation.js +++ b/modules/Navigation.js @@ -1,5 +1,17 @@ +var warning = require('react/lib/warning'); var PropTypes = require('./PropTypes'); +function deprecatedMethod(routerMethodName, fn) { + return function () { + warning( + false, + `Router.Navigation is deprecated. Please use this.context.router.${routerMethodName}() instead` + ); + + return fn.apply(this, arguments); + }; +} + /** * A mixin for components that modify the URL. * @@ -7,11 +19,11 @@ var PropTypes = require('./PropTypes'); * * var MyLink = React.createClass({ * mixins: [ Router.Navigation ], - * handleClick: function (event) { + * handleClick(event) { * event.preventDefault(); * this.transitionTo('aRoute', { the: 'params' }, { the: 'query' }); * }, - * render: function () { + * render() { * return ( * Click me! * ); @@ -21,51 +33,47 @@ var PropTypes = require('./PropTypes'); var Navigation = { contextTypes: { - makePath: PropTypes.func.isRequired, - makeHref: PropTypes.func.isRequired, - transitionTo: PropTypes.func.isRequired, - replaceWith: PropTypes.func.isRequired, - goBack: PropTypes.func.isRequired + router: PropTypes.router.isRequired }, /** * Returns an absolute URL path created from the given route * name, URL parameters, and query values. */ - makePath: function (to, params, query) { - return this.context.makePath(to, params, query); - }, + makePath: deprecatedMethod('makePath', function (to, params, query) { + return this.context.router.makePath(to, params, query); + }), /** * Returns a string that may safely be used as the href of a * link to the route with the given name. */ - makeHref: function (to, params, query) { - return this.context.makeHref(to, params, query); - }, + makeHref: deprecatedMethod('makeHref', function (to, params, query) { + return this.context.router.makeHref(to, params, query); + }), /** * Transitions to the URL specified in the arguments by pushing * a new URL onto the history stack. */ - transitionTo: function (to, params, query) { - this.context.transitionTo(to, params, query); - }, + transitionTo: deprecatedMethod('transitionTo', function (to, params, query) { + this.context.router.transitionTo(to, params, query); + }), /** * Transitions to the URL specified in the arguments by replacing * the current URL in the history stack. */ - replaceWith: function (to, params, query) { - this.context.replaceWith(to, params, query); - }, + replaceWith: deprecatedMethod('replaceWith', function (to, params, query) { + this.context.router.replaceWith(to, params, query); + }), /** * Transitions to the previous URL. */ - goBack: function () { - return this.context.goBack(); - } + goBack: deprecatedMethod('goBack', function () { + return this.context.router.goBack(); + }) }; diff --git a/modules/NavigationContext.js b/modules/NavigationContext.js deleted file mode 100644 index f1cf909638..0000000000 --- a/modules/NavigationContext.js +++ /dev/null @@ -1,28 +0,0 @@ -var PropTypes = require('./PropTypes'); - -/** - * Provides the router with context for Router.Navigation. - */ -var NavigationContext = { - - childContextTypes: { - makePath: PropTypes.func.isRequired, - makeHref: PropTypes.func.isRequired, - transitionTo: PropTypes.func.isRequired, - replaceWith: PropTypes.func.isRequired, - goBack: PropTypes.func.isRequired - }, - - getChildContext: function () { - return { - makePath: this.constructor.makePath.bind(this.constructor), - makeHref: this.constructor.makeHref.bind(this.constructor), - transitionTo: this.constructor.transitionTo.bind(this.constructor), - replaceWith: this.constructor.replaceWith.bind(this.constructor), - goBack: this.constructor.goBack.bind(this.constructor) - }; - } - -}; - -module.exports = NavigationContext; diff --git a/modules/PropTypes.js b/modules/PropTypes.js index a4488acbbe..c2cfc7ee6c 100644 --- a/modules/PropTypes.js +++ b/modules/PropTypes.js @@ -1,16 +1,28 @@ var assign = require('react/lib/Object.assign'); var ReactPropTypes = require('react').PropTypes; +var Route = require('./Route'); -var PropTypes = assign({ +var PropTypes = assign({}, ReactPropTypes, { /** - * Requires that the value of a prop be falsy. + * Indicates that a prop should be falsy. */ falsy(props, propName, componentName) { if (props[propName]) - return new Error('<' + componentName + '> may not have a "' + propName + '" prop'); - } + return new Error(`<${componentName}> may not have a "${propName}" prop`); + }, -}, ReactPropTypes); + /** + * Indicates that a prop should be a Route object. + */ + route: ReactPropTypes.instanceOf(Route), + + /** + * Indicates that a prop should be a Router object. + */ + //router: ReactPropTypes.instanceOf(Router) // TODO + router: ReactPropTypes.func + +}); module.exports = PropTypes; diff --git a/modules/RouteHandlerMixin.js b/modules/RouteHandlerMixin.js index a68a0d3921..59a0a0f1f0 100644 --- a/modules/RouteHandlerMixin.js +++ b/modules/RouteHandlerMixin.js @@ -7,9 +7,8 @@ var REF_NAME = '__routeHandler__'; var RouteHandlerMixin = { contextTypes: { - getRouteAtDepth: PropTypes.func.isRequired, - setRouteComponentAtDepth: PropTypes.func.isRequired, - routeHandlers: PropTypes.array.isRequired + routeHandlers: PropTypes.array.isRequired, + router: PropTypes.router.isRequired }, childContextTypes: { @@ -35,7 +34,7 @@ var RouteHandlerMixin = { }, _updateRouteComponent: function (component) { - this.context.setRouteComponentAtDepth(this.getRouteDepth(), component); + this.context.router.setRouteComponentAtDepth(this.getRouteDepth(), component); }, getRouteDepth: function () { @@ -43,7 +42,7 @@ var RouteHandlerMixin = { }, createChildRouteHandler: function (props) { - var route = this.context.getRouteAtDepth(this.getRouteDepth()); + var route = this.context.router.getRouteAtDepth(this.getRouteDepth()); return route ? React.createElement(route.handler, assign({}, props || this.props, { ref: REF_NAME })) : null; } diff --git a/modules/State.js b/modules/State.js index 105a272305..c6bb834950 100644 --- a/modules/State.js +++ b/modules/State.js @@ -1,5 +1,17 @@ +var warning = require('react/lib/warning'); var PropTypes = require('./PropTypes'); +function deprecatedMethod(routerMethodName, fn) { + return function () { + warning( + false, + `Router.State is deprecated. Please use this.context.router.${routerMethodName}() instead` + ); + + return fn.apply(this, arguments); + }; +} + /** * A mixin for components that need to know the path, routes, URL * params and query that are currently active. @@ -8,7 +20,7 @@ var PropTypes = require('./PropTypes'); * * var AboutLink = React.createClass({ * mixins: [ Router.State ], - * render: function () { + * render() { * var className = this.props.className; * * if (this.isActive('about')) @@ -21,56 +33,51 @@ var PropTypes = require('./PropTypes'); var State = { contextTypes: { - getCurrentPath: PropTypes.func.isRequired, - getCurrentRoutes: PropTypes.func.isRequired, - getCurrentPathname: PropTypes.func.isRequired, - getCurrentParams: PropTypes.func.isRequired, - getCurrentQuery: PropTypes.func.isRequired, - isActive: PropTypes.func.isRequired + router: PropTypes.router.isRequired }, /** * Returns the current URL path. */ - getPath: function () { - return this.context.getCurrentPath(); - }, - - /** - * Returns an array of the routes that are currently active. - */ - getRoutes: function () { - return this.context.getCurrentRoutes(); - }, + getPath: deprecatedMethod('getCurrentPath', function () { + return this.context.router.getCurrentPath(); + }), /** * Returns the current URL path without the query string. */ - getPathname: function () { - return this.context.getCurrentPathname(); - }, + getPathname: deprecatedMethod('getCurrentPathname', function () { + return this.context.router.getCurrentPathname(); + }), /** * Returns an object of the URL params that are currently active. */ - getParams: function () { - return this.context.getCurrentParams(); - }, + getParams: deprecatedMethod('getCurrentParams', function () { + return this.context.router.getCurrentParams(); + }), /** * Returns an object of the query params that are currently active. */ - getQuery: function () { - return this.context.getCurrentQuery(); - }, + getQuery: deprecatedMethod('getCurrentQuery', function () { + return this.context.router.getCurrentQuery(); + }), + + /** + * Returns an array of the routes that are currently active. + */ + getRoutes: deprecatedMethod('getCurrentRoutes', function () { + return this.context.router.getCurrentRoutes(); + }), /** * A helper method to determine if a given route, params, and query * are active. */ - isActive: function (to, params, query) { - return this.context.isActive(to, params, query); - } + isActive: deprecatedMethod('isActive', function (to, params, query) { + return this.context.router.isActive(to, params, query); + }) }; diff --git a/modules/StateContext.js b/modules/StateContext.js deleted file mode 100644 index 57c78e8666..0000000000 --- a/modules/StateContext.js +++ /dev/null @@ -1,101 +0,0 @@ -var assign = require('react/lib/Object.assign'); -var PropTypes = require('./PropTypes'); -var PathUtils = require('./PathUtils'); - -function routeIsActive(activeRoutes, routeName) { - return activeRoutes.some(function (route) { - return route.name === routeName; - }); -} - -function paramsAreActive(activeParams, params) { - for (var property in params) - if (String(activeParams[property]) !== String(params[property])) - return false; - - return true; -} - -function queryIsActive(activeQuery, query) { - for (var property in query) - if (String(activeQuery[property]) !== String(query[property])) - return false; - - return true; -} - -/** - * Provides the router with context for Router.State. - */ -var StateContext = { - - /** - * Returns the current URL path + query string. - */ - getCurrentPath: function () { - return this.state.path; - }, - - /** - * Returns a read-only array of the currently active routes. - */ - getCurrentRoutes: function () { - return this.state.routes.slice(0); - }, - - /** - * Returns the current URL path without the query string. - */ - getCurrentPathname: function () { - return this.state.pathname; - }, - - /** - * Returns a read-only object of the currently active URL parameters. - */ - getCurrentParams: function () { - return assign({}, this.state.params); - }, - - /** - * Returns a read-only object of the currently active query parameters. - */ - getCurrentQuery: function () { - return assign({}, this.state.query); - }, - - /** - * Returns true if the given route, params, and query are active. - */ - isActive: function (to, params, query) { - if (PathUtils.isAbsolute(to)) - return to === this.state.path; - - return routeIsActive(this.state.routes, to) && - paramsAreActive(this.state.params, params) && - (query == null || queryIsActive(this.state.query, query)); - }, - - childContextTypes: { - getCurrentPath: PropTypes.func.isRequired, - getCurrentRoutes: PropTypes.func.isRequired, - getCurrentPathname: PropTypes.func.isRequired, - getCurrentParams: PropTypes.func.isRequired, - getCurrentQuery: PropTypes.func.isRequired, - isActive: PropTypes.func.isRequired - }, - - getChildContext: function () { - return { - getCurrentPath: this.getCurrentPath, - getCurrentRoutes: this.getCurrentRoutes, - getCurrentPathname: this.getCurrentPathname, - getCurrentParams: this.getCurrentParams, - getCurrentQuery: this.getCurrentQuery, - isActive: this.isActive - }; - } - -}; - -module.exports = StateContext; diff --git a/modules/TestUtils.js b/modules/TestUtils.js index 1700c68cb8..9573de7280 100644 --- a/modules/TestUtils.js +++ b/modules/TestUtils.js @@ -1,6 +1,6 @@ var React = require('react'); var RouteHandler = require('./components/RouteHandler'); -var State = require('./State'); +var PropTypes = require('./PropTypes'); exports.Nested = React.createClass({ render: function () { @@ -111,8 +111,10 @@ exports.EchoFooProp = React.createClass({ }); exports.EchoBarParam = React.createClass({ - mixins: [ State ], + contextTypes: { + router: PropTypes.router.isRequired + }, render: function () { - return
{this.getParams().bar}
; + return
{this.context.router.getCurrentParams().bar}
; } }); diff --git a/modules/__tests__/Router-test.js b/modules/__tests__/Router-test.js index aaae2343bf..4465af305b 100644 --- a/modules/__tests__/Router-test.js +++ b/modules/__tests__/Router-test.js @@ -945,14 +945,10 @@ describe('Router.run', function () { }); describe('locations', function () { - it('defaults to HashLocation', function (done) { + it('defaults to HashLocation', function () { var routes = ; - var div = document.createElement('div'); Router.run(routes, function (Handler) { - React.render(, div, function () { - expect(this.getLocation()).toBe(Router.HashLocation); - done(); - }); + expect(this.getLocation()).toBe(Router.HashLocation); }); }); }); diff --git a/modules/__tests__/State-test.js b/modules/__tests__/State-test.js index bf5a2d0190..dc3918760e 100644 --- a/modules/__tests__/State-test.js +++ b/modules/__tests__/State-test.js @@ -12,14 +12,16 @@ describe('State', function () { it('is active', function (done) { var location = new TestLocation([ '/foo' ]); var div = document.createElement('div'); - var routes = ( ); + var router; + Router.run(routes, location, function (Handler) { + router = this; React.render(, div, function () { - assert(this.isActive('foo')); + assert(router.isActive('foo')); done(); }); }); @@ -27,15 +29,15 @@ describe('State', function () { }); describe('and the right params are given', function () { - var component, location; + var location, router; var div = document.createElement('div'); var routes = ; beforeEach(function (done) { location = new TestLocation([ '/products/123/456?search=abc&limit=789' ]); Router.run(routes, location, function (Handler) { + router = this; React.render(, div, function () { - component = this; done(); }); }); @@ -47,25 +49,25 @@ describe('State', function () { describe('and no query is used', function () { it('is active', function () { - assert(component.isActive('products', { id: 123, variant: '456' })); + assert(router.isActive('products', { id: 123, variant: '456' })); }); }); describe('and a matching query is used', function () { it('is active', function () { - assert(component.isActive('products', { id: 123 }, { search: 'abc' })); + assert(router.isActive('products', { id: 123 }, { search: 'abc' })); }); }); describe('but the query does not match', function () { it('is not active', function () { - assert(component.isActive('products', { id: 123 }, { search: 'def' }) === false); + assert(router.isActive('products', { id: 123 }, { search: 'def' }) === false); }); }); describe('and the wrong params are given', function () { it('is not active', function () { - assert(component.isActive('products', { id: 345 }) === false); + assert(router.isActive('products', { id: 345 }) === false); }); }); diff --git a/modules/components/Link.js b/modules/components/Link.js index 2e03620f50..58dbedc70e 100644 --- a/modules/components/Link.js +++ b/modules/components/Link.js @@ -1,10 +1,7 @@ var React = require('react'); var classSet = require('react/lib/cx'); var assign = require('react/lib/Object.assign'); -var Navigation = require('../Navigation'); -var State = require('../State'); var PropTypes = require('../PropTypes'); -var Route = require('../Route'); function isLeftClickEvent(event) { return event.button === 0; @@ -36,13 +33,15 @@ var Link = React.createClass({ displayName: 'Link', - mixins: [ Navigation, State ], + contextTypes: { + router: PropTypes.router.isRequired + }, propTypes: { activeClassName: PropTypes.string.isRequired, to: PropTypes.oneOfType([ PropTypes.string, - PropTypes.instanceOf(Route) + PropTypes.route ]), params: PropTypes.object, query: PropTypes.object, @@ -72,14 +71,14 @@ var Link = React.createClass({ event.preventDefault(); if (allowTransition) - this.transitionTo(this.props.to, this.props.params, this.props.query); + this.context.router.transitionTo(this.props.to, this.props.params, this.props.query); }, /** * Returns the value of the "href" attribute to use on the DOM element. */ getHref: function () { - return this.makeHref(this.props.to, this.props.params, this.props.query); + return this.context.router.makeHref(this.props.to, this.props.params, this.props.query); }, /** @@ -99,7 +98,7 @@ var Link = React.createClass({ }, getActiveState: function () { - return this.isActive(this.props.to, this.props.params, this.props.query); + return this.context.router.isActive(this.props.to, this.props.params, this.props.query); }, render: function () { diff --git a/modules/createRouter.js b/modules/createRouter.js index 258c5fc22b..5007620976 100644 --- a/modules/createRouter.js +++ b/modules/createRouter.js @@ -9,9 +9,7 @@ var HashLocation = require('./locations/HashLocation'); var HistoryLocation = require('./locations/HistoryLocation'); var RefreshLocation = require('./locations/RefreshLocation'); var StaticLocation = require('./locations/StaticLocation'); -var NavigationContext = require('./NavigationContext'); var ScrollHistory = require('./ScrollHistory'); -var StateContext = require('./StateContext'); var createRoutesFromReactChildren = require('./createRoutesFromReactChildren'); var isReactChildren = require('./isReactChildren'); var Transition = require('./Transition'); @@ -83,6 +81,28 @@ function addRoutesToNamedRoutes(routes, namedRoutes) { } } +function routeIsActive(activeRoutes, routeName) { + return activeRoutes.some(function (route) { + return route.name === routeName; + }); +} + +function paramsAreActive(activeParams, params) { + for (var property in params) + if (String(activeParams[property]) !== String(params[property])) + return false; + + return true; +} + +function queryIsActive(activeQuery, query) { + for (var property in query) + if (String(activeQuery[property]) !== String(query[property])) + return false; + + return true; +} + /** * Creates and returns a new router using the given options. A router * is a ReactComponent class that knows how to react to changes in the @@ -425,29 +445,87 @@ function createRouter(options) { this.isRunning = false; }, + getLocation: function () { + return location; + }, + getScrollBehavior: function () { return scrollBehavior; + }, + + getRouteAtDepth: function (depth) { + var routes = state.routes; + return routes && routes[depth]; + }, + + setRouteComponentAtDepth: function (depth, component) { + mountedComponents[depth] = component; + }, + + /** + * Returns the current URL path + query string. + */ + getCurrentPath: function () { + return state.path; + }, + + /** + * Returns the current URL path without the query string. + */ + getCurrentPathname: function () { + return state.pathname; + }, + + /** + * Returns an object of the currently active URL parameters. + */ + getCurrentParams: function () { + return state.params; + }, + + /** + * Returns an object of the currently active query parameters. + */ + getCurrentQuery: function () { + return state.query; + }, + + /** + * Returns an array of the currently active routes. + */ + getCurrentRoutes: function () { + return state.routes; + }, + + /** + * Returns true if the given route, params, and query are active. + */ + isActive: function (to, params, query) { + if (PathUtils.isAbsolute(to)) + return to === state.path; + + return routeIsActive(state.routes, to) && + paramsAreActive(state.params, params) && + (query == null || queryIsActive(state.query, query)); } }, - mixins: [ NavigationContext, StateContext, ScrollHistory ], + mixins: [ ScrollHistory ], propTypes: { children: PropTypes.falsy }, childContextTypes: { - getRouteAtDepth: React.PropTypes.func.isRequired, - setRouteComponentAtDepth: React.PropTypes.func.isRequired, - routeHandlers: React.PropTypes.array.isRequired + routeHandlers: PropTypes.array.isRequired, + router: PropTypes.router.isRequired }, getChildContext: function () { return { - getRouteAtDepth: this.getRouteAtDepth, - setRouteComponentAtDepth: this.setRouteComponentAtDepth, - routeHandlers: [ this ] + routeHandlers: [ this ], + router: Router }; }, @@ -463,21 +541,8 @@ function createRouter(options) { Router.stop(); }, - getLocation: function () { - return location; - }, - - getRouteAtDepth: function (depth) { - var routes = this.state.routes; - return routes && routes[depth]; - }, - - setRouteComponentAtDepth: function (depth, component) { - mountedComponents[depth] = component; - }, - render: function () { - var route = this.getRouteAtDepth(0); + var route = Router.getRouteAtDepth(0); return route ? React.createElement(route.handler, this.props) : null; }