From cd05f7739f98da7a3235d763fe0f50533ecbb733 Mon Sep 17 00:00:00 2001 From: Jonathan Adshead Date: Fri, 10 Jan 2020 12:01:10 -0700 Subject: [PATCH 1/2] refactor(router-context): remove UNSAFE_componentWillMount --- modules/ContextUtils.js | 36 ++++++++++++++++-------------------- modules/RouterContext.js | 30 +++++++++++++----------------- 2 files changed, 29 insertions(+), 37 deletions(-) diff --git a/modules/ContextUtils.js b/modules/ContextUtils.js index 483fe451f7..397550ad2b 100644 --- a/modules/ContextUtils.js +++ b/modules/ContextUtils.js @@ -19,18 +19,14 @@ import React, { Component } from 'react' // https://github.com/facebook/react/issues/2517 // https://github.com/reactjs/react-router/issues/470 -export function makeContextName(name) { - return `@@contextSubscriber/${name}` +const createNamedContext = (name) => { + const context = React.createContext({}) + context.displayName = name + return context } -export const RouterContext = React.createContext({}) - -const contextName = makeContextName('router') - -// subscriber keys -const lastRenderedEventIndexKey = `${contextName}/lastRenderedEventIndex` -const handleContextUpdateKey = `${contextName}/handleContextUpdate` -const unsubscribeKey = `${contextName}/unsubscribe` +export const contextName = '@@OneAppRouterContext' +export const RouterContext = createNamedContext(contextName) function contextHOC(Subscriber) { return class WrappedSubscriber extends Component { @@ -50,7 +46,7 @@ class ContextSubscriberBase extends Component { const initialState = {} if(context[contextName]) { - initialState[lastRenderedEventIndexKey] = context[contextName].eventIndex + initialState.lastRenderedEventIndex = context[contextName].eventIndex } this.state = initialState @@ -62,7 +58,7 @@ class ContextSubscriberBase extends Component { } return { - [lastRenderedEventIndexKey]: props.context[contextName].eventIndex + lastRenderedEventIndex: props.context[contextName].eventIndex } } @@ -71,23 +67,23 @@ class ContextSubscriberBase extends Component { return } - this[unsubscribeKey] = this.context[contextName].subscribe( - this[handleContextUpdateKey] + this.unsubscribe = this.context[contextName].subscribe( + this.handleContextUpdate ) } componentWillUnmount() { - if (!this[unsubscribeKey]) { + if (!this.unsubscribe) { return } - this[unsubscribeKey]() - this[unsubscribeKey] = null + this.unsubscribe() + this.unsubscribe = null } - [handleContextUpdateKey] = (eventIndex) => { - if (eventIndex !== this.state[lastRenderedEventIndexKey]) { - this.setState({ [lastRenderedEventIndexKey]: eventIndex }) + handleContextUpdate = (eventIndex) => { + if (eventIndex !== this.state.lastRenderedEventIndex) { + this.setState({ lastRenderedEventIndex: eventIndex }) } } } diff --git a/modules/RouterContext.js b/modules/RouterContext.js index 1e3819725f..0434dedc75 100644 --- a/modules/RouterContext.js +++ b/modules/RouterContext.js @@ -17,14 +17,9 @@ import React, { Component } from 'react' import { array, func, object } from 'prop-types' import getRouteParams from './getRouteParams' -import { RouterContext as ProvidableRouterContext, makeContextName } from './ContextUtils' +import { RouterContext as ProvidableRouterContext, contextName } from './ContextUtils' import { isReactChildren } from './RouteUtils' -const contextName = makeContextName('router') -const listenersKey = `${contextName}/listeners` -const eventIndexKey = `${contextName}/eventIndex` -const subscribeKey = `${contextName}/subscribe` - /** * A renders the component tree for a given router state * and sets the history object and the current location in context. @@ -45,27 +40,28 @@ class RouterContext extends Component { createElement: React.createElement } - UNSAFE_componentWillMount() { - this[listenersKey] = [] - this[eventIndexKey] = 0 + constructor(params) { + super(params) + this.listeners = [] + this.eventIndex = 0 } UNSAFE_componentWillReceiveProps() { - this[eventIndexKey]++ + this.eventIndex++ } componentDidUpdate() { - this[listenersKey].forEach(listener => - listener(this[eventIndexKey]) + this.listeners.forEach(listener => + listener(this.eventIndex) ) } - [subscribeKey] = (listener) => { + subscribeKey = (listener) => { // No need to immediately call listener here. - this[listenersKey].push(listener) + this.listeners.push(listener) return () => { - this[listenersKey] = this[listenersKey].filter(item => + this.listeners = this.listeners.filter(item => item !== listener ) } @@ -135,8 +131,8 @@ class RouterContext extends Component { value={{ router: this.props.router, [contextName]: { - eventIndex: this[eventIndexKey], - subscribe: this[subscribeKey] + eventIndex: this.eventIndex, + subscribe: this.subscribe } }} > From caf1375e404116e476f3057c889705d11065ee9d Mon Sep 17 00:00:00 2001 From: Jonathan Adshead Date: Fri, 10 Jan 2020 14:13:43 -0700 Subject: [PATCH 2/2] refactor(contextUtil): remove as no longer necessary --- modules/ContextUtils.js | 91 ----------------------- modules/Link.js | 6 +- modules/RouterContext.js | 48 +++--------- modules/__tests__/RouterContext-test.js | 5 +- modules/__tests__/transitionHooks-test.js | 2 +- modules/withRouter.js | 6 +- 6 files changed, 20 insertions(+), 138 deletions(-) delete mode 100644 modules/ContextUtils.js diff --git a/modules/ContextUtils.js b/modules/ContextUtils.js deleted file mode 100644 index 397550ad2b..0000000000 --- a/modules/ContextUtils.js +++ /dev/null @@ -1,91 +0,0 @@ -/* - * Copyright (c) 2019 American Express Travel Related Services Company, Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except - * in compliance with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under the License - * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express - * or implied. See the License for the specific language governing permissions and limitations under - * the License. - */ - -import React, { Component } from 'react' - -// Works around issues with context updates failing to propagate. -// Caveat: the context value is expected to never change its identity. -// https://github.com/facebook/react/issues/2517 -// https://github.com/reactjs/react-router/issues/470 - -const createNamedContext = (name) => { - const context = React.createContext({}) - context.displayName = name - return context -} - -export const contextName = '@@OneAppRouterContext' -export const RouterContext = createNamedContext(contextName) - -function contextHOC(Subscriber) { - return class WrappedSubscriber extends Component { - static contextType = RouterContext - - render() { - const newProps = { ...this.props, context: this.context } - return - } - } -} - -class ContextSubscriberBase extends Component { - constructor(props, context) { - super(props) - - const initialState = {} - - if(context[contextName]) { - initialState.lastRenderedEventIndex = context[contextName].eventIndex - } - - this.state = initialState - } - - static getDerivedStateFromProps(props) { - if (!props.context[contextName]) { - return - } - - return { - lastRenderedEventIndex: props.context[contextName].eventIndex - } - } - - componentDidMount() { - if (!this.context[contextName]) { - return - } - - this.unsubscribe = this.context[contextName].subscribe( - this.handleContextUpdate - ) - } - - componentWillUnmount() { - if (!this.unsubscribe) { - return - } - - this.unsubscribe() - this.unsubscribe = null - } - - handleContextUpdate = (eventIndex) => { - if (eventIndex !== this.state.lastRenderedEventIndex) { - this.setState({ lastRenderedEventIndex: eventIndex }) - } - } -} - -export const ContextSubscriber = contextHOC(ContextSubscriberBase) diff --git a/modules/Link.js b/modules/Link.js index d58e53cb10..094ab4df88 100644 --- a/modules/Link.js +++ b/modules/Link.js @@ -15,7 +15,7 @@ import React from 'react' import { bool, object, string, func, oneOfType } from 'prop-types' import invariant from 'invariant' -import { ContextSubscriber } from './ContextUtils' +import { Context as RouterContext } from './RouterContext' function isLeftClickEvent(event) { return event.button === 0 @@ -51,9 +51,9 @@ function resolveToLocation(to, router) { * * */ -class Link extends ContextSubscriber { +class Link extends React.Component { static displayName = 'Link' - + static contextType = RouterContext static propTypes = { to: oneOfType([ string, object, func ]), activeStyle: object, diff --git a/modules/RouterContext.js b/modules/RouterContext.js index 0434dedc75..9e02c36e4a 100644 --- a/modules/RouterContext.js +++ b/modules/RouterContext.js @@ -17,9 +17,16 @@ import React, { Component } from 'react' import { array, func, object } from 'prop-types' import getRouteParams from './getRouteParams' -import { RouterContext as ProvidableRouterContext, contextName } from './ContextUtils' import { isReactChildren } from './RouteUtils' +const createNamedContext = (name) => { + const context = React.createContext({}) + context.displayName = name + return context +} + +export const Context = createNamedContext('@@OneAppRouterContext') + /** * A renders the component tree for a given router state * and sets the history object and the current location in context. @@ -40,33 +47,6 @@ class RouterContext extends Component { createElement: React.createElement } - constructor(params) { - super(params) - this.listeners = [] - this.eventIndex = 0 - } - - UNSAFE_componentWillReceiveProps() { - this.eventIndex++ - } - - componentDidUpdate() { - this.listeners.forEach(listener => - listener(this.eventIndex) - ) - } - - subscribeKey = (listener) => { - // No need to immediately call listener here. - this.listeners.push(listener) - - return () => { - this.listeners = this.listeners.filter(item => - item !== listener - ) - } - } - createElement = (component, props) => { return component == null ? null : this.props.createElement(component, props) } @@ -127,17 +107,11 @@ class RouterContext extends Component { ) return ( - {element} - + ) } } diff --git a/modules/__tests__/RouterContext-test.js b/modules/__tests__/RouterContext-test.js index a8f10425ac..d63a9df266 100644 --- a/modules/__tests__/RouterContext-test.js +++ b/modules/__tests__/RouterContext-test.js @@ -17,9 +17,8 @@ import React from 'react' import { render, unmountComponentAtNode } from 'react-dom' import match from '../match' -import RouterContext from '../RouterContext' +import RouterContext, { Context } from '../RouterContext' import { createRouterObject } from '../RouterUtils' -import { RouterContext as ProvidableRouterContext } from '../ContextUtils' describe('RouterContext', () => { let node, routes, context, history, transitionManager, router @@ -55,7 +54,7 @@ describe('RouterContext', () => { render() { return null } } - Component.contextType = ProvidableRouterContext + Component.contextType = Context routes = { path: '/', component: Component } }) diff --git a/modules/__tests__/transitionHooks-test.js b/modules/__tests__/transitionHooks-test.js index c2e5ed049f..b014f0ee58 100644 --- a/modules/__tests__/transitionHooks-test.js +++ b/modules/__tests__/transitionHooks-test.js @@ -20,7 +20,7 @@ import execSteps from './execSteps' import Router from '../Router' import Route from '../Route' import match from '../match' -import { RouterContext } from '../ContextUtils' +import { Context as RouterContext } from '../RouterContext' describe('When a router enters a branch', function () { let diff --git a/modules/withRouter.js b/modules/withRouter.js index 7bdbd65eb2..d64a675fea 100644 --- a/modules/withRouter.js +++ b/modules/withRouter.js @@ -15,7 +15,7 @@ import invariant from 'invariant' import React from 'react' import hoistStatics from 'hoist-non-react-statics' -import { ContextSubscriber } from './ContextUtils' +import { Context as RouterContext } from './RouterContext' import { routerShape } from './PropTypes' function getDisplayName(WrappedComponent) { @@ -25,9 +25,9 @@ function getDisplayName(WrappedComponent) { export default function withRouter(WrappedComponent, options) { const withRef = options && options.withRef - class WithRouter extends ContextSubscriber { + class WithRouter extends React.Component { static displayName = 'WithRouter' - + static contextType = RouterContext static propTypes = { router: routerShape } getWrappedInstance = () => {