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.

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

Support class component static contextType attribute #13728

Merged
merged 2 commits into from
Sep 25, 2018
Merged

Support class component static contextType attribute #13728

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
870c695
Support class component static contextType attribute
Sep 25, 2018
353cc7e
PR feedback:
Sep 25, 2018
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
2 changes: 1 addition & 1 deletion packages/react-reconciler/src/ReactFiberBeginWork.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ import warningWithoutStack from 'shared/warningWithoutStack';
import * as ReactCurrentFiber from './ReactCurrentFiber';
import {cancelWorkTimer} from './ReactDebugFiberPerf';

import {applyDerivedStateFromProps} from './ReactFiberClassComponent';
import {
mountChildFibers,
reconcileChildFibers,
Expand Down Expand Up @@ -97,6 +96,7 @@ import {
} from './ReactFiberHydrationContext';
import {
adoptClassInstance,
applyDerivedStateFromProps,
constructClassInstance,
mountClassInstance,
resumeMountClassInstance,
Expand Down
160 changes: 114 additions & 46 deletions packages/react-reconciler/src/ReactFiberClassComponent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ let didWarnAboutUndefinedDerivedState;
let warnOnUndefinedDerivedState;
let warnOnInvalidCallback;
let didWarnAboutDirectlyAssigningPropsToState;
let didWarnAboutContextTypeAndContextTypes;
let didWarnAboutInvalidateContextType;

if (__DEV__) {
didWarnAboutStateAssignmentForComponent = new Set();
Expand All @@ -73,6 +75,8 @@ if (__DEV__) {
didWarnAboutLegacyLifecyclesAndDerivedState = new Set();
didWarnAboutDirectlyAssigningPropsToState = new Set();
didWarnAboutUndefinedDerivedState = new Set();
didWarnAboutContextTypeAndContextTypes = new Set();
didWarnAboutInvalidateContextType = new Set();

const didWarnOnInvalidCallback = new Set();

Expand Down Expand Up @@ -234,15 +238,15 @@ function checkShouldComponentUpdate(
newProps,
oldState,
newState,
nextLegacyContext,
nextContext,
) {
const instance = workInProgress.stateNode;
if (typeof instance.shouldComponentUpdate === 'function') {
startPhaseTimer(workInProgress, 'shouldComponentUpdate');
const shouldUpdate = instance.shouldComponentUpdate(
newProps,
newState,
nextLegacyContext,
nextContext,
);
stopPhaseTimer();

Expand Down Expand Up @@ -319,13 +323,50 @@ function checkClassInstance(workInProgress: Fiber, ctor: any, newProps: any) {
'property to define propTypes instead.',
name,
);
const noInstanceContextType = !instance.contextType;
warningWithoutStack(
noInstanceContextType,
'contextType was defined as an instance property on %s. Use a static ' +
'property to define contextType instead.',
name,
);
const noInstanceContextTypes = !instance.contextTypes;
warningWithoutStack(
noInstanceContextTypes,
'contextTypes was defined as an instance property on %s. Use a static ' +
'property to define contextTypes instead.',
name,
);

if (
ctor.contextType &&
ctor.contextTypes &&
!didWarnAboutContextTypeAndContextTypes.has(ctor)
) {
didWarnAboutContextTypeAndContextTypes.add(ctor);
warningWithoutStack(
false,
'%s declares both contextTypes and contextType static properties. ' +
'The legacy contextTypes property will be ignored.',
name,
);
}

if (
ctor.contextType &&
typeof ctor.contextType.unstable_read !== 'function' &&
!didWarnAboutInvalidateContextType.has(ctor)
) {
didWarnAboutInvalidateContextType.add(ctor);
warningWithoutStack(
false,
'%s defines an invalid contextType. ' +
'contextType should point to the Context object returned by React.createContext(). ' +
'Did you accidentally pass the Context.Provider instead?',
name,
);
}

const noComponentShouldUpdate =
typeof instance.componentShouldUpdate !== 'function';
warningWithoutStack(
Expand Down Expand Up @@ -475,12 +516,24 @@ function constructClassInstance(
props: any,
renderExpirationTime: ExpirationTime,
): any {
const unmaskedContext = getUnmaskedContext(workInProgress, ctor, true);
const contextTypes = ctor.contextTypes;
const isContextConsumer = contextTypes !== null && contextTypes !== undefined;
const context = isContextConsumer
? getMaskedContext(workInProgress, unmaskedContext)
: emptyContextObject;
let isContextConsumer = false;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we name this isLegacyContextConsumer to be more specific?

let unmaskedContext = emptyContextObject;
let context = null;
const contextType = ctor.contextType;
if (
typeof contextType === 'object' &&
contextType !== null &&
typeof contextType.unstable_read === 'function'
) {
context = (contextType: any).unstable_read();
} else {
unmaskedContext = getUnmaskedContext(workInProgress, ctor, true);
const contextTypes = ctor.contextTypes;
isContextConsumer = contextTypes !== null && contextTypes !== undefined;
context = isContextConsumer
? getMaskedContext(workInProgress, unmaskedContext)
: emptyContextObject;
}

// Instantiate twice to help detect side-effects.
if (__DEV__) {
Expand Down Expand Up @@ -625,15 +678,15 @@ function callComponentWillReceiveProps(
workInProgress,
instance,
newProps,
nextLegacyContext,
nextContext,
) {
const oldState = instance.state;
startPhaseTimer(workInProgress, 'componentWillReceiveProps');
if (typeof instance.componentWillReceiveProps === 'function') {
instance.componentWillReceiveProps(newProps, nextLegacyContext);
instance.componentWillReceiveProps(newProps, nextContext);
}
if (typeof instance.UNSAFE_componentWillReceiveProps === 'function') {
instance.UNSAFE_componentWillReceiveProps(newProps, nextLegacyContext);
instance.UNSAFE_componentWillReceiveProps(newProps, nextContext);
}
stopPhaseTimer();

Expand Down Expand Up @@ -668,12 +721,21 @@ function mountClassInstance(
}

const instance = workInProgress.stateNode;
const unmaskedContext = getUnmaskedContext(workInProgress, ctor, true);

instance.props = newProps;
instance.state = workInProgress.memoizedState;
instance.refs = emptyRefsObject;
instance.context = getMaskedContext(workInProgress, unmaskedContext);

const contextType = ctor.contextType;
if (
typeof contextType === 'object' &&
contextType !== null &&
typeof contextType.unstable_read === 'function'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need this extra check? Seems like we can just throw.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess we don't, since we also provide a nice DEV mode warning if it's not a function. I'll remove this type check.

Copy link
Contributor Author

@bvaughn bvaughn Sep 26, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Actually, if we don't check in at least the instantiation path, we'll throw before our DEV warning. So I think it's best to leave these checks in place after all.

If you feel strongly about it, let's talk and come up with another plan. I think the DEV warnings are useful to preserve though.

Disregard. I'll just move the warning earlier. #13736

) {
instance.context = (contextType: any).unstable_read();
} else {
const unmaskedContext = getUnmaskedContext(workInProgress, ctor, true);
instance.context = getMaskedContext(workInProgress, unmaskedContext);
}

if (__DEV__) {
if (instance.state === newProps) {
Expand Down Expand Up @@ -774,15 +836,22 @@ function resumeMountClassInstance(
instance.props = oldProps;

const oldContext = instance.context;
const nextLegacyUnmaskedContext = getUnmaskedContext(
workInProgress,
ctor,
true,
);
const nextLegacyContext = getMaskedContext(
workInProgress,
nextLegacyUnmaskedContext,
);
const contextType = ctor.contextType;
let nextContext;
if (
typeof contextType === 'object' &&
contextType !== null &&
typeof contextType.unstable_read === 'function'
) {
nextContext = (contextType: any).unstable_read();
} else {
const nextLegacyUnmaskedContext = getUnmaskedContext(
workInProgress,
ctor,
true,
);
nextContext = getMaskedContext(workInProgress, nextLegacyUnmaskedContext);
}

const getDerivedStateFromProps = ctor.getDerivedStateFromProps;
const hasNewLifecycles =
Expand All @@ -800,12 +869,12 @@ function resumeMountClassInstance(
(typeof instance.UNSAFE_componentWillReceiveProps === 'function' ||
typeof instance.componentWillReceiveProps === 'function')
) {
if (oldProps !== newProps || oldContext !== nextLegacyContext) {
if (oldProps !== newProps || oldContext !== nextContext) {
callComponentWillReceiveProps(
workInProgress,
instance,
newProps,
nextLegacyContext,
nextContext,
);
}
}
Expand Down Expand Up @@ -858,7 +927,7 @@ function resumeMountClassInstance(
newProps,
oldState,
newState,
nextLegacyContext,
nextContext,
);

if (shouldUpdate) {
Expand Down Expand Up @@ -898,7 +967,7 @@ function resumeMountClassInstance(
// if shouldComponentUpdate returns false.
instance.props = newProps;
instance.state = newState;
instance.context = nextLegacyContext;
instance.context = nextContext;

return shouldUpdate;
}
Expand All @@ -917,15 +986,18 @@ function updateClassInstance(
instance.props = oldProps;

const oldContext = instance.context;
const nextLegacyUnmaskedContext = getUnmaskedContext(
workInProgress,
ctor,
true,
);
const nextLegacyContext = getMaskedContext(
workInProgress,
nextLegacyUnmaskedContext,
);
const contextType = ctor.contextType;
let nextContext;
if (
typeof contextType === 'object' &&
contextType !== null &&
typeof contextType.unstable_read === 'function'
) {
nextContext = (contextType: any).unstable_read();
} else {
const nextUnmaskedContext = getUnmaskedContext(workInProgress, ctor, true);
nextContext = getMaskedContext(workInProgress, nextUnmaskedContext);
}

const getDerivedStateFromProps = ctor.getDerivedStateFromProps;
const hasNewLifecycles =
Expand All @@ -943,12 +1015,12 @@ function updateClassInstance(
(typeof instance.UNSAFE_componentWillReceiveProps === 'function' ||
typeof instance.componentWillReceiveProps === 'function')
) {
if (oldProps !== newProps || oldContext !== nextLegacyContext) {
if (oldProps !== newProps || oldContext !== nextContext) {
callComponentWillReceiveProps(
workInProgress,
instance,
newProps,
nextLegacyContext,
nextContext,
);
}
}
Expand Down Expand Up @@ -1015,7 +1087,7 @@ function updateClassInstance(
newProps,
oldState,
newState,
nextLegacyContext,
nextContext,
);

if (shouldUpdate) {
Expand All @@ -1028,14 +1100,10 @@ function updateClassInstance(
) {
startPhaseTimer(workInProgress, 'componentWillUpdate');
if (typeof instance.componentWillUpdate === 'function') {
instance.componentWillUpdate(newProps, newState, nextLegacyContext);
instance.componentWillUpdate(newProps, newState, nextContext);
}
if (typeof instance.UNSAFE_componentWillUpdate === 'function') {
instance.UNSAFE_componentWillUpdate(
newProps,
newState,
nextLegacyContext,
);
instance.UNSAFE_componentWillUpdate(newProps, newState, nextContext);
}
stopPhaseTimer();
}
Expand Down Expand Up @@ -1075,7 +1143,7 @@ function updateClassInstance(
// if shouldComponentUpdate returns false.
instance.props = newProps;
instance.state = newState;
instance.context = nextLegacyContext;
instance.context = nextContext;

return shouldUpdate;
}
Expand Down
2 changes: 2 additions & 0 deletions packages/react/src/__tests__/ReactCoffeeScriptClass-test.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,7 @@ describe 'ReactCoffeeScriptClass', ->
class Foo extends React.Component
constructor: ->
@contextTypes = {}
@contextType = {}
@propTypes = {}

getInitialState: ->
Expand All @@ -413,6 +414,7 @@ describe 'ReactCoffeeScriptClass', ->
'getDefaultProps was defined on Foo, a plain JavaScript class.',
'propTypes was defined as an instance property on Foo.',
'contextTypes was defined as an instance property on Foo.',
'contextType was defined as an instance property on Foo.',
], {withoutStack: true})
expect(getInitialStateWasCalled).toBe false
expect(getDefaultPropsWasCalled).toBe false
Expand Down
Loading