Skip to content

Replace legacy context API with the new React.createContext, with a fallback #427

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.

Sign up for GitHub

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

Closed
wants to merge 5 commits into from
Closed

Replace legacy context API with the new React.createContext, with a fallback #427

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d72883d
Replace legacy context API with the new React.createContext with a po…
errendir Oct 23, 2018
e4332d0
Don't use create-react-context
errendir Oct 23, 2018
320b0e5
Use react-context-toolbox for passing context value into props. Remov…
errendir Oct 25, 2018
aafaecd
Undo the ref change
errendir Feb 14, 2019
ab6d376
Merge remote-tracking branch 'origin/master' into new-context-backwar…
errendir Feb 14, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,14 @@
]
},
"peerDependencies": {
"react": ">=15.0.0",
"react-dom": ">=15.0.0"
"react": ">=16.3.0",
"react-dom": ">=16.3.0"
},
"dependencies": {
"dom-helpers": "^3.3.1",
"loose-envify": "^1.4.0",
"prop-types": "^15.6.2",
"react-context-toolbox": "^1.2.2",
"react-lifecycles-compat": "^3.0.4"
},
"devDependencies": {
Expand All @@ -75,8 +76,8 @@
"babel-plugin-transform-react-remove-prop-types": "^0.4.18",
"babel-preset-jason": "^6.0.1",
"cross-env": "^5.2.0",
"enzyme": "^3.6.0",
"enzyme-adapter-react-16": "^1.5.0",
"enzyme": "^3.7.0",
"enzyme-adapter-react-16": "^1.6.0",
"eslint": "^5.6.0",
"eslint-config-jason": "^4.1.0",
"eslint-config-prettier": "^3.1.0",
Expand All @@ -87,9 +88,9 @@
"husky": "^1.0.0-rc.15",
"jest": "^23.6.0",
"prettier": "^1.14.3",
"react": "^16.5.2",
"react-dom": "^16.5.2",
"react-test-renderer": "^16.5.2",
"react": "^16.6.0",
"react-dom": "^16.6.0",
"react-test-renderer": "^16.6.0",
"release-script": "^1.0.2",
"rimraf": "^2.6.1",
"rollup": "^1.1.0",
Expand Down
63 changes: 35 additions & 28 deletions src/Transition.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import { polyfill } from 'react-lifecycles-compat'

import { timeoutsShape } from './utils/PropTypes'

import { Provider as TransitionGroupContextProvider, withTransitionGroup, transitionGroupContextPropType } from './TransitionGroupContext'

export const UNMOUNTED = 'unmounted'
export const EXITED = 'exited'
export const ENTERING = 'entering'
Expand Down Expand Up @@ -107,17 +109,10 @@ export const EXITING = 'exiting'
* > `CSSTransition`.
*/
class Transition extends React.Component {
static contextTypes = {
transitionGroup: PropTypes.object,
}
static childContextTypes = {
transitionGroup: () => {},
}

constructor(props, context) {
super(props, context)
constructor(props) {
super(props)

let parentGroup = context.transitionGroup
let parentGroup = props.transitionGroup
// In the context of a TransitionGroup all enters are really appears
let appear =
parentGroup && !parentGroup.isMounting ? props.enter : props.appear
Expand Down Expand Up @@ -146,10 +141,6 @@ class Transition extends React.Component {
this.nextCallback = null
}

getChildContext() {
return { transitionGroup: null } // allows for nested Transitions
}

static getDerivedStateFromProps({ in: nextIn }, prevState) {
if (nextIn && prevState.status === UNMOUNTED) {
return { status: EXITED }
Expand Down Expand Up @@ -235,8 +226,8 @@ class Transition extends React.Component {

performEnter(node, mounting) {
const { enter } = this.props
const appearing = this.context.transitionGroup
? this.context.transitionGroup.isMounting
const appearing = this.props.transitionGroup
? this.props.transitionGroup.isMounting
: mounting

const timeouts = this.getTimeouts()
Expand Down Expand Up @@ -359,17 +350,31 @@ class Transition extends React.Component {
delete childProps.onExit
delete childProps.onExiting
delete childProps.onExited
delete childProps.transitionGroup

if (typeof children === 'function') {
return children(status, childProps)
return (
<TransitionGroupContextProvider value={null}>
{children(status, childProps)}
</TransitionGroupContextProvider>
)
}

const child = React.Children.only(children)
return React.cloneElement(child, childProps)
return (
<TransitionGroupContextProvider value={null}>
{React.cloneElement(child, childProps)}
</TransitionGroupContextProvider>
)
}
}

Transition.propTypes = {
// Name the function so it is clearer in the documentation
function noop() {}

const TransitionWithContext = withTransitionGroup(polyfill(Transition))

TransitionWithContext.propTypes = {
/**
* A `function` child can be used instead of a React element.
* This function is called with the current transition status
Expand Down Expand Up @@ -509,10 +514,12 @@ Transition.propTypes = {
onExited: PropTypes.func,
}

// Name the function so it is clearer in the documentation
function noop() {}
Transition.propTypes = {
...TransitionWithContext.propTypes,
transitionGroup: transitionGroupContextPropType,
}

Transition.defaultProps = {
TransitionWithContext.defaultProps = {
in: false,
mountOnEnter: false,
unmountOnExit: false,
Expand All @@ -529,10 +536,10 @@ Transition.defaultProps = {
onExited: noop,
}

Transition.UNMOUNTED = 0
Transition.EXITED = 1
Transition.ENTERING = 2
Transition.ENTERED = 3
Transition.EXITING = 4
TransitionWithContext.UNMOUNTED = 0
TransitionWithContext.EXITED = 1
TransitionWithContext.ENTERING = 2
TransitionWithContext.ENTERED = 3
TransitionWithContext.EXITING = 4

export default polyfill(Transition)
export default TransitionWithContext
28 changes: 14 additions & 14 deletions src/TransitionGroup.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import {
getNextChildMapping,
} from './utils/ChildMapping'

import { Provider as TransitionGroupContextProvider } from './TransitionGroupContext'

const values = Object.values || (obj => Object.keys(obj).map(k => obj[k]))

const defaultProps = {
Expand All @@ -31,12 +33,8 @@ const defaultProps = {
* items.
*/
class TransitionGroup extends React.Component {
static childContextTypes = {
transitionGroup: PropTypes.object.isRequired,
}

constructor(props, context) {
super(props, context)
constructor(props) {
super(props)

const handleExited = this.handleExited.bind(this)

Expand All @@ -47,12 +45,6 @@ class TransitionGroup extends React.Component {
}
}

getChildContext() {
return {
transitionGroup: { isMounting: !this.appeared },
}
}

componentDidMount() {
this.appeared = true
this.mounted = true
Expand Down Expand Up @@ -102,9 +94,17 @@ class TransitionGroup extends React.Component {
delete props.exit

if (Component === null) {
return children
return (
<TransitionGroupContextProvider value={{ isMounting: !this.appeared }}>
{children}
</TransitionGroupContextProvider>
)
}
return <Component {...props}>{children}</Component>
return (
<TransitionGroupContextProvider value={{ isMounting: !this.appeared }}>
<Component {...props}>{children}</Component>
</TransitionGroupContextProvider>
)
}
}

Expand Down
15 changes: 15 additions & 0 deletions src/TransitionGroupContext.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import * as PropTypes from 'prop-types'
import React from 'react'
import mapContextToProps from 'react-context-toolbox/lib/mapContextToProps'

const { Provider, Consumer } = React.createContext(null)

export const transitionGroupContextPropType = PropTypes.shape({
isMounting: PropTypes.bool.isRequired
})

export function withTransitionGroup(ComponentToWrap) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

since we are gonna use just the React api's, lets remove this, and go for the new contextType api in Transition maybe? Alternatively keep the HoC but lets use forwardRef as well

Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually for a HoC lets use react-context-toolbox's mapContextToProps helper which takes care of this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, done. See the comment on the PR about some issues with the "react-context-toolbox" module.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jquense Do you still want to go with react-context-toolbox package? contextType looks the best way. Why HOC?

return mapContextToProps(Consumer, value => ({ transitionGroup: value }), ComponentToWrap)
}

export { Provider }
4 changes: 2 additions & 2 deletions stories/CSSTransitionGroupFixture.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ import TransitionGroup from '../src/TransitionGroup';
import StoryFixture from './StoryFixture';

class CSSTransitionGroupFixture extends React.Component {
constructor(props, context) {
super(props, context);
constructor(props) {
super(props);

let items = props.items || [];

Expand Down
4 changes: 2 additions & 2 deletions stories/NestedTransition.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ const FadeAndScale = (props) => (


export default class Example extends React.Component {
constructor(props, context) {
super(props, context);
constructor(props) {
super(props);
this.state = { showNested: false };
}

Expand Down
26 changes: 15 additions & 11 deletions test/Transition-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ jasmine.addMatchers({
}),
})

function findStatefulInstance(wrapper) {
return wrapper.find('Transition').instance()
}

describe('Transition', () => {
it('should not transition on mount', () => {
let wrapper = mount(
Expand All @@ -34,7 +38,7 @@ describe('Transition', () => {
</Transition>
)

expect(wrapper.state('status')).toEqual(ENTERED)
expect(findStatefulInstance(wrapper).state.status).toEqual(ENTERED)
})

it('should transition on mount with `appear`', done => {
Expand Down Expand Up @@ -146,7 +150,7 @@ describe('Transition', () => {
let onEnter = sinon.spy()
let onEntering = sinon.spy()

expect(wrapper.state('status')).toEqual(EXITED)
expect(findStatefulInstance(wrapper).state.status).toEqual(EXITED)

wrapper.setProps({
in: true,
Expand All @@ -167,23 +171,23 @@ describe('Transition', () => {
it('should move to each transition state', done => {
let count = 0

expect(wrapper.state('status')).toEqual(EXITED)
expect(findStatefulInstance(wrapper).state.status).toEqual(EXITED)

wrapper.setProps({
in: true,

onEnter() {
count++
expect(wrapper.state('status')).toEqual(EXITED)
expect(findStatefulInstance(wrapper).state.status).toEqual(EXITED)
},

onEntering() {
count++
expect(wrapper.state('status')).toEqual(ENTERING)
expect(findStatefulInstance(wrapper).state.status).toEqual(ENTERING)
},

onEntered() {
expect(wrapper.state('status')).toEqual(ENTERED)
expect(findStatefulInstance(wrapper).state.status).toEqual(ENTERED)
expect(count).toEqual(2)
done()
},
Expand All @@ -206,7 +210,7 @@ describe('Transition', () => {
let onExit = sinon.spy()
let onExiting = sinon.spy()

expect(wrapper.state('status')).toEqual(ENTERED)
expect(findStatefulInstance(wrapper).state.status).toEqual(ENTERED)

wrapper.setProps({
in: false,
Expand All @@ -227,23 +231,23 @@ describe('Transition', () => {
it('should move to each transition state', done => {
let count = 0

expect(wrapper.state('status')).toEqual(ENTERED)
expect(findStatefulInstance(wrapper).state.status).toEqual(ENTERED)

wrapper.setProps({
in: false,

onExit() {
count++
expect(wrapper.state('status')).toEqual(ENTERED)
expect(findStatefulInstance(wrapper).state.status).toEqual(ENTERED)
},

onExiting() {
count++
expect(wrapper.state('status')).toEqual(EXITING)
expect(findStatefulInstance(wrapper).state.status).toEqual(EXITING)
},

onExited() {
expect(wrapper.state('status')).toEqual(EXITED)
expect(findStatefulInstance(wrapper).state.status).toEqual(EXITED)
expect(count).toEqual(2)
done()
},
Expand Down
Loading