Skip to content

Commit

Permalink
New PreAsyncComponent warns about unsafe lifecycles in tree
Browse files Browse the repository at this point in the history
Works like AsyncComponent except that it does not actually enable async rendering. This component is exposed via React.unstable_PreAsyncComponent (following precedent).

I also tidied up ReactBaseClass a little because the duplication was bothering me. I will revert this if there's any concern.

This branch is stacked on top of 12046-part-2 and PR facebook#12060
  • Loading branch information
bvaughn committed Jan 23, 2018
1 parent a1b5f0f commit 5db93d4
Show file tree
Hide file tree
Showing 6 changed files with 119 additions and 36 deletions.
7 changes: 5 additions & 2 deletions packages/react-reconciler/src/ReactDebugAsyncWarnings.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import type {Fiber} from './ReactFiber';

import getComponentName from 'shared/getComponentName';
import {getStackAddendumByWorkInProgressFiber} from 'shared/ReactFiberComponentTreeHook';
import {AsyncUpdates} from './ReactTypeOfInternalContext';
import {AsyncUpdates, PreAsyncUpdates} from './ReactTypeOfInternalContext';
import warning from 'fbjs/lib/warning';

type LIFECYCLE =
Expand Down Expand Up @@ -94,7 +94,10 @@ if (__DEV__) {
let maybeAsyncRoot = null;

while (fiber !== null) {
if (fiber.internalContextTag & AsyncUpdates) {
if (
fiber.internalContextTag & AsyncUpdates ||
fiber.internalContextTag & PreAsyncUpdates
) {
maybeAsyncRoot = fiber;
}

Expand Down
18 changes: 13 additions & 5 deletions packages/react-reconciler/src/ReactFiberClassComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import invariant from 'fbjs/lib/invariant';
import warning from 'fbjs/lib/warning';

import {startPhaseTimer, stopPhaseTimer} from './ReactDebugFiberPerf';
import {AsyncUpdates} from './ReactTypeOfInternalContext';
import {AsyncUpdates, PreAsyncUpdates} from './ReactTypeOfInternalContext';
import {
cacheContext,
getMaskedContext,
Expand Down Expand Up @@ -637,16 +637,24 @@ export default function(
if (
enableAsyncSubtreeAPI &&
workInProgress.type != null &&
workInProgress.type.prototype != null &&
workInProgress.type.prototype.unstable_isAsyncReactComponent === true
workInProgress.type.prototype != null
) {
workInProgress.internalContextTag |= AsyncUpdates;
const prototype = workInProgress.type.prototype;

if (prototype.unstable_isAsyncReactComponent === true) {
workInProgress.internalContextTag |= AsyncUpdates;
} else if (prototype.unstable_isPreAsyncReactComponent === true) {
workInProgress.internalContextTag |= PreAsyncUpdates;
}
}

if (__DEV__) {
// If we're inside of an async sub-tree,
// Warn about any unsafe lifecycles on this class component.
if (workInProgress.internalContextTag & AsyncUpdates) {
if (
workInProgress.internalContextTag & AsyncUpdates ||
workInProgress.internalContextTag & PreAsyncUpdates
) {
ReactDebugAsyncWarnings.recordLifecycleWarnings(
workInProgress,
instance,
Expand Down
5 changes: 3 additions & 2 deletions packages/react-reconciler/src/ReactTypeOfInternalContext.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,6 @@

export type TypeOfInternalContext = number;

export const NoContext = 0;
export const AsyncUpdates = 1;
export const NoContext = 0b00000000;
export const AsyncUpdates = 0b00000001;
export const PreAsyncUpdates = 0b00000010;
8 changes: 7 additions & 1 deletion packages/react/src/React.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,12 @@ import assign from 'object-assign';
import ReactVersion from 'shared/ReactVersion';
import {REACT_FRAGMENT_TYPE} from 'shared/ReactSymbols';

import {Component, PureComponent, AsyncComponent} from './ReactBaseClasses';
import {
Component,
PureComponent,
AsyncComponent,
PreAsyncComponent,
} from './ReactBaseClasses';
import {forEach, map, count, toArray, only} from './ReactChildren';
import ReactCurrentOwner from './ReactCurrentOwner';
import {
Expand Down Expand Up @@ -37,6 +42,7 @@ const React = {
Component,
PureComponent,
unstable_AsyncComponent: AsyncComponent,
unstable_PreAsyncComponent: PreAsyncComponent,

Fragment: REACT_FRAGMENT_TYPE,

Expand Down
64 changes: 38 additions & 26 deletions packages/react/src/ReactBaseClasses.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,44 +117,56 @@ if (__DEV__) {
}
}

/**
* Base class helpers for the updating state of a component.
*/
function ComponentDummy() {}
ComponentDummy.prototype = Component.prototype;

function configurePrototype(ComponentSubclass, prototypeProperties) {
const prototype = (ComponentSubclass.prototype = new ComponentDummy());
prototype.constructor = ComponentSubclass;

// Avoid an extra prototype jump for these methods.
Object.assign(prototype, Component.prototype);

// Mixin additional properties
Object.assign(prototype, prototypeProperties);
}

// Convenience component with default shallow equality check for sCU.
function PureComponent(props, context, updater) {
// Duplicated from Component.
this.props = props;
this.context = context;
this.refs = emptyObject;
// We initialize the default updater but the real one gets injected by the
// renderer.
this.updater = updater || ReactNoopUpdateQueue;
}
configurePrototype(PureComponent, {isPureReactComponent: true});

function ComponentDummy() {}
ComponentDummy.prototype = Component.prototype;
const pureComponentPrototype = (PureComponent.prototype = new ComponentDummy());
pureComponentPrototype.constructor = PureComponent;
// Avoid an extra prototype jump for these methods.
Object.assign(pureComponentPrototype, Component.prototype);
pureComponentPrototype.isPureReactComponent = true;

// Special component type that opts subtree into async rendering mode.
function AsyncComponent(props, context, updater) {
// Duplicated from Component.
this.props = props;
this.context = context;
this.refs = emptyObject;
// We initialize the default updater but the real one gets injected by the
// renderer.
this.updater = updater || ReactNoopUpdateQueue;
}
configurePrototype(AsyncComponent, {
unstable_isAsyncReactComponent: true,
render: function render() {
return this.props.children;
},
});

const asyncComponentPrototype = (AsyncComponent.prototype = new ComponentDummy());
asyncComponentPrototype.constructor = AsyncComponent;
// Avoid an extra prototype jump for these methods.
Object.assign(asyncComponentPrototype, Component.prototype);
asyncComponentPrototype.unstable_isAsyncReactComponent = true;
asyncComponentPrototype.render = function() {
return this.props.children;
};
// Special component type that enables async rendering dev warnings.
// This helps detect unsafe lifecycles without enabling actual async behavior.
function PreAsyncComponent(props, context, updater) {
this.props = props;
this.context = context;
this.refs = emptyObject;
this.updater = updater || ReactNoopUpdateQueue;
}
configurePrototype(PreAsyncComponent, {
unstable_isPreAsyncReactComponent: true,
render: function render() {
return this.props.children;
},
});

export {Component, PureComponent, AsyncComponent};
export {Component, PureComponent, AsyncComponent, PreAsyncComponent};
Original file line number Diff line number Diff line change
Expand Up @@ -419,5 +419,58 @@ describe('ReactAsyncClassComponent', () => {

expect(caughtError).not.toBe(null);
});

it('should also warn inside of pre-async trees', () => {
class SyncRoot extends React.Component {
UNSAFE_componentWillMount() {}
UNSAFE_componentWillUpdate() {}
UNSAFE_componentWillReceiveProps() {}
render() {
return <PreAsyncRoot />;
}
}
class PreAsyncRoot extends React.unstable_PreAsyncComponent {
UNSAFE_componentWillMount() {}
render() {
return <Wrapper />;
}
}
function Wrapper({children}) {
return (
<div>
<Bar />
<Foo />
</div>
);
}
class Foo extends React.Component {
UNSAFE_componentWillReceiveProps() {}
render() {
return null;
}
}
class Bar extends React.Component {
UNSAFE_componentWillReceiveProps() {}
render() {
return null;
}
}

expect(() => ReactTestRenderer.create(<SyncRoot />)).toWarnDev(
'Unsafe lifecycle methods were found within the following async tree:' +
'\n in PreAsyncRoot (at **)' +
'\n in SyncRoot (at **)' +
'\n\ncomponentWillMount: Please update the following components ' +
'to use componentDidMount instead: PreAsyncRoot' +
'\n\ncomponentWillReceiveProps: Please update the following components ' +
'to use static getDerivedStateFromProps instead: Bar, Foo' +
'\n\nLearn more about this warning here:' +
'\nhttps://fb.me/react-async-component-lifecycle-hooks',
);

// Dedupe
const rendered = ReactTestRenderer.create(<SyncRoot />);
rendered.update(<SyncRoot />);
});
});
});

0 comments on commit 5db93d4

Please sign in to comment.