@samitouri / QOS-React / commits / b1b3acbd6b

Inline fbjs/lib/emptyObject (#13055)

* Inline fbjs/lib/emptyObject * Explicit naming * Compare to undefined * Another approach for detecting whether we can mutate Each renderer would have its own local LegacyRefsObject function. While in general we don't want `instanceof`, here it lets us do a simple check: did *we* create the refs object? Then we can mutate it. If the check didn't pass, either we're attaching ref for the first time (so we know to use the constructor), or (unlikely) we're attaching a ref to a component owned by another renderer. In this case, to avoid "losing" refs, we assign them onto the new object. Even in that case it shouldn't "hop" between renderers anymore. * Clearer naming * Add test case for strings refs across renderers * Use a shared empty object for refs by reading it from React * Remove string refs from ReactART test It's not currently possible to resetModules() between several renderers without also resetting the `React` module. However, that leads to losing the referential identity of the empty ref object, and thus subsequent checks in the renderers for whether it is pooled fail (and cause assignments to a frozen object). This has always been the case, but we used to work around it by shimming fbjs/lib/emptyObject in tests and preserving its referential identity. This won't work anymore because we've inlined it. And preserving referential identity of React itself wouldn't be great because it could be confusing during testing (although we might want to revisit this in the future by moving its stateful parts into a separate package). For now, I'm removing string ref usage from this test because only this is the only place in our tests where we hit this problem, and it's only related to string refs, and not just ref mechanism in general. * Simplify the condition

Dan Abramov committed Jun 19, 2018 at 13:41 UTC b1b3acbd6b5fad40558db790f59855116ec3ee93

15 files changed +136 -46

packages/react-art/src/ReactARTHostConfig.js

+7 -3

@@ -9,13 +9,17 @@ import * as ReactScheduler from 'shared/ReactScheduler';

9 import Transform from 'art/core/transform';

10 import Mode from 'art/modes/current';

11 import invariant from 'fbjs/lib/invariant';

12 -import emptyObject from 'fbjs/lib/emptyObject';

13 import {TYPES, EVENT_TYPES, childrenAsString} from './ReactARTInternals';

15 const pooledTransform = new Transform();

17 +const NO_CONTEXT = {};

18 const UPDATE_SIGNAL = {};

19 +if (__DEV__) {

20 + Object.freeze(NO_CONTEXT);

21 + Object.freeze(UPDATE_SIGNAL);

22 +}

24 /** Helper Methods */

@@ -318,11 +322,11 @@ export function shouldDeprioritizeSubtree(type, props) {

322 }

323

324 export function getRootHostContext() {

321 - return emptyObject;

325 + return NO_CONTEXT;

326 }

327

328 export function getChildHostContext() {

325 - return emptyObject;

329 + return NO_CONTEXT;

330 }

331

332 export const scheduleDeferredCallback = ReactScheduler.scheduleWork;

packages/react-art/src/__tests__/ReactART-test.js

+14 -8

@@ -71,6 +71,8 @@ describe('ReactART', () => {

71 Surface = ReactART.Surface;

73 TestComponent = class extends React.Component {

74 + group = React.createRef();

75 +

76 render() {

77 const a = (

78 <Shape

@@ -104,7 +106,7 @@ describe('ReactART', () => {

106

107 return (

108 <Surface width={150} height={200}>

107 - <Group ref="group">

109 + <Group ref={this.group}>

110 {this.props.flipped ? [b, a, c] : [a, b, c]}

111 </Group>

112 </Surface>

@@ -121,7 +123,7 @@ describe('ReactART', () => {

123 it('should have the correct lifecycle state', () => {

124 let instance = <TestComponent />;

125 instance = ReactTestUtils.renderIntoDocument(instance);

124 - const group = instance.refs.group;

126 + const group = instance.group.current;

127 // Duck type test for an ART group

128 expect(typeof group.indicate).toBe('function');

129 });

@@ -260,15 +262,17 @@ describe('ReactART', () => {

262 let ref = null;

263

264 class Outer extends React.Component {

265 + test = React.createRef();

266 +

267 componentDidMount() {

264 - ref = this.refs.test;

268 + ref = this.test.current;

269 }

270

271 render() {

272 return (

273 <Surface>

274 <Group>

271 - <CustomShape ref="test" />

275 + <CustomShape ref={this.test} />

276 </Group>

277 </Surface>

278 );

@@ -289,26 +293,28 @@ describe('ReactART', () => {

293 let ref = {};

294

295 class Outer extends React.Component {

296 + test = React.createRef();

297 +

298 componentDidMount() {

293 - ref = this.refs.test;

299 + ref = this.test.current;

300 }

301

302 componentDidUpdate() {

297 - ref = this.refs.test;

303 + ref = this.test.current;

304 }

305

306 render() {

307 return (

308 <Surface>

309 <Group>

304 - {this.props.mountCustomShape && <CustomShape ref="test" />}

310 + {this.props.mountCustomShape && <CustomShape ref={this.test} />}

311 </Group>

312 </Surface>

313 );

314 }

315 }

316 ReactDOM.render(<Outer />, container);

311 - expect(ref).not.toBeDefined();

317 + expect(ref).toBe(null);

318 ReactDOM.render(<Outer mountCustomShape={true} />, container);

319 expect(ref.constructor).toBe(CustomShape);

320 });

packages/react-dom/src/__tests__/refs-test.js

+43

@@ -412,3 +412,46 @@ describe('creating element with ref in constructor', () => {

412 );

413 });

414 });

415 +

416 +describe('strings refs across renderers', () => {

417 + it('does not break', () => {

418 + class Parent extends React.Component {

419 + render() {

420 + // This component owns both refs.

421 + return (

422 + <Indirection

423 + child1={<div ref="child1" />}

424 + child2={<div ref="child2" />}

425 + />

426 + );

427 + }

428 + }

429 +

430 + class Indirection extends React.Component {

431 + componentDidUpdate() {

432 + // One ref is being rendered later using another renderer copy.

433 + jest.resetModules();

434 + const AnotherCopyOfReactDOM = require('react-dom');

435 + AnotherCopyOfReactDOM.render(this.props.child2, div2);

436 + }

437 + render() {

438 + // The other one is being rendered directly.

439 + return this.props.child1;

440 + }

441 + }

442 +

443 + const div1 = document.createElement('div');

444 + const div2 = document.createElement('div');

445 + const inst = ReactDOM.render(<Parent />, div1);

446 + // Only the first ref has rendered yet.

447 + expect(inst.refs.child1.tagName).toBe('DIV');

448 + expect(inst.refs.child1).toBe(div1.firstChild);

449 +

450 + // Now both refs should be rendered.

451 + ReactDOM.render(<Parent />, div1);

452 + expect(inst.refs.child1.tagName).toBe('DIV');

453 + expect(inst.refs.child1).toBe(div1.firstChild);

454 + expect(inst.refs.child2.tagName).toBe('DIV');

455 + expect(inst.refs.child2).toBe(div2.firstChild);

456 + });

457 +});

packages/react-dom/src/server/ReactPartialRenderer.js

+5 -1

@@ -15,7 +15,6 @@ import type {

15 } from 'shared/ReactTypes';

17 import React from 'react';

18 -import emptyObject from 'fbjs/lib/emptyObject';

18 import invariant from 'fbjs/lib/invariant';

19 import lowPriorityWarning from 'shared/lowPriorityWarning';

20 import warning from 'fbjs/lib/warning';

@@ -284,6 +283,11 @@ function flattenOptionChildren(children: mixed): string {

283 return content;

284 }

285

286 +const emptyObject = {};

287 +if (__DEV__) {

288 + Object.freeze(emptyObject);

289 +}

290 +

291 function maskContext(type, context) {

292 const contextTypes = type.contextTypes;

293 if (!contextTypes) {

packages/react-native-renderer/src/ReactNativeFiberInspector.js

+5 -1

@@ -15,13 +15,17 @@ import {

15 } from 'react-reconciler/reflection';

16 import getComponentName from 'shared/getComponentName';

17 import {HostComponent} from 'shared/ReactTypeOfWork';

18 -import emptyObject from 'fbjs/lib/emptyObject';

18 import invariant from 'fbjs/lib/invariant';

19 // Module provided by RN:

20 import UIManager from 'UIManager';

22 import {getClosestInstanceFromNode} from './ReactNativeComponentTree';

24 +const emptyObject = {};

25 +if (__DEV__) {

26 + Object.freeze(emptyObject);

27 +}

28 +

29 let getInspectorDataForViewTag;

31 if (__DEV__) {

packages/react-native-renderer/src/ReactNativeHostConfig.js

+6 -2

@@ -9,7 +9,6 @@

10 import type {ReactNativeBaseComponentViewConfig} from './ReactNativeTypes';

12 -import emptyObject from 'fbjs/lib/emptyObject';

12 import invariant from 'fbjs/lib/invariant';

14 // Modules provided by RN:

@@ -43,6 +42,11 @@ export type HostContext = $ReadOnly<{|

42 export type UpdatePayload = Object; // Unused

43 export type ChildSet = void; // Unused

45 +const UPDATE_SIGNAL = {};

46 +if (__DEV__) {

47 + Object.freeze(UPDATE_SIGNAL);

48 +}

49 +

50 // Counter for uniquely identifying views.

51 // % 10 === 1 means it is a rootTag.

52 // % 2 === 0 means it is a Fabric tag.

@@ -218,7 +222,7 @@ export function prepareUpdate(

222 rootContainerInstance: Container,

223 hostContext: HostContext,

224 ): null | Object {

221 - return emptyObject;

225 + return UPDATE_SIGNAL;

226 }

227

228 export function resetAfterCommit(containerInfo: Container): void {

packages/react-noop-renderer/src/createReactNoop.js

+9 -4

@@ -19,7 +19,6 @@ import type {UpdateQueue} from 'react-reconciler/src/ReactUpdateQueue';

19 import type {ReactNodeList} from 'shared/ReactTypes';

21 import * as ReactPortal from 'shared/ReactPortal';

22 -import emptyObject from 'fbjs/lib/emptyObject';

22 import expect from 'expect';

24 type Container = {rootID: string, children: Array<Instance | TextInstance>};

@@ -32,8 +31,14 @@ type Instance = {|

31 |};

32 type TextInstance = {|text: string, id: number|};

34 +const NO_CONTEXT = {};

35 +const UPDATE_SIGNAL = {};

36 +if (__DEV__) {

37 + Object.freeze(NO_CONTEXT);

38 + Object.freeze(UPDATE_SIGNAL);

39 +}

40 +

41 function createReactNoop(reconciler: Function, useMutation: boolean) {

36 - const UPDATE_SIGNAL = {};

42 let scheduledCallback = null;

44 let instanceCounter = 0;

@@ -84,11 +89,11 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {

89 if (failInBeginPhase) {

90 throw new Error('Error in host config.');

91 }

87 - return emptyObject;

92 + return NO_CONTEXT;

93 },

95 getChildHostContext() {

91 - return emptyObject;

96 + return NO_CONTEXT;

97 },

99 getPublicInstance(instance) {

packages/react-reconciler/src/ReactChildFiber.js

+6 -2

@@ -28,7 +28,6 @@ import {

28 Fragment,

29 } from 'shared/ReactTypeOfWork';

30 import {getStackAddendumByWorkInProgressFiber} from 'shared/ReactFiberComponentTreeHook';

31 -import emptyObject from 'fbjs/lib/emptyObject';

31 import invariant from 'fbjs/lib/invariant';

32 import warning from 'fbjs/lib/warning';

@@ -39,6 +38,7 @@ import {

38 createFiberFromText,

39 createFiberFromPortal,

40 } from './ReactFiber';

41 +import {emptyRefsObject} from './ReactFiberClassComponent';

42 import ReactDebugCurrentFiber from './ReactDebugCurrentFiber';

43 import {StrictMode} from './ReactTypeOfMode';

@@ -157,7 +157,11 @@ function coerceRef(

157 return current.ref;

158 }

159 const ref = function(value) {

160 - const refs = inst.refs === emptyObject ? (inst.refs = {}) : inst.refs;

160 + let refs = inst.refs;

161 + if (refs === emptyRefsObject) {

162 + // This is a lazy pooled frozen object, so we need to initialize.

163 + refs = inst.refs = {};

164 + }

165 if (value === null) {

166 delete refs[stringRef];

167 } else {

packages/react-reconciler/src/ReactFiberClassComponent.js

+8 -3

@@ -10,6 +10,7 @@

10 import type {Fiber} from './ReactFiber';

11 import type {ExpirationTime} from './ReactFiberExpirationTime';

13 +import React from 'react';

14 import {Update, Snapshot} from 'shared/ReactTypeOfSideEffect';

15 import {

16 debugRenderPhaseSideEffects,

@@ -20,7 +21,6 @@ import ReactStrictModeWarnings from './ReactStrictModeWarnings';

21 import {isMounted} from 'react-reconciler/reflection';

22 import * as ReactInstanceMap from 'shared/ReactInstanceMap';

23 import shallowEqual from 'shared/shallowEqual';

23 -import emptyObject from 'fbjs/lib/emptyObject';

24 import getComponentName from 'shared/getComponentName';

25 import invariant from 'fbjs/lib/invariant';

26 import warning from 'fbjs/lib/warning';

@@ -43,6 +43,7 @@ import {

43 getUnmaskedContext,

44 isContextConsumer,

45 hasContextChanged,

46 + emptyContextObject,

47 } from './ReactFiberContext';

48 import {

49 recalculateCurrentTime,

@@ -53,6 +54,10 @@ import {

54 const fakeInternalInstance = {};

55 const isArray = Array.isArray;

57 +// React.Component uses a shared frozen object by default.

58 +// We'll use it to determine whether we need to initialize legacy refs.

59 +export const emptyRefsObject = new React.Component().refs;

60 +

61 let didWarnAboutStateAssignmentForComponent;

62 let didWarnAboutUninitializedState;

63 let didWarnAboutGetSnapshotBeforeUpdateWithoutDidUpdate;

@@ -469,7 +474,7 @@ function constructClassInstance(

474 const needsContext = isContextConsumer(workInProgress);

475 const context = needsContext

476 ? getMaskedContext(workInProgress, unmaskedContext)

472 - : emptyObject;

477 + : emptyContextObject;

478

479 // Instantiate twice to help detect side-effects.

480 if (__DEV__) {

@@ -658,7 +663,7 @@ function mountClassInstance(

663

664 instance.props = props;

665 instance.state = workInProgress.memoizedState;

661 - instance.refs = emptyObject;

666 + instance.refs = emptyRefsObject;

667 instance.context = getMaskedContext(workInProgress, unmaskedContext);

668

669 if (__DEV__) {

packages/react-reconciler/src/ReactFiberContext.js

+10 -6

@@ -13,7 +13,6 @@ import type {StackCursor} from './ReactFiberStack';

13 import {isFiberMounted} from 'react-reconciler/reflection';

14 import {ClassComponent, HostRoot} from 'shared/ReactTypeOfWork';

15 import getComponentName from 'shared/getComponentName';

16 -import emptyObject from 'fbjs/lib/emptyObject';

16 import invariant from 'fbjs/lib/invariant';

17 import warning from 'fbjs/lib/warning';

18 import checkPropTypes from 'prop-types/checkPropTypes';

@@ -28,14 +27,19 @@ if (__DEV__) {

27 warnedAboutMissingGetChildContext = {};

28 }

30 +export const emptyContextObject = {};

31 +if (__DEV__) {

32 + Object.freeze(emptyContextObject);

33 +}

34 +

35 // A cursor to the current merged context object on the stack.

32 -let contextStackCursor: StackCursor<Object> = createCursor(emptyObject);

36 +let contextStackCursor: StackCursor<Object> = createCursor(emptyContextObject);

37 // A cursor to a boolean indicating whether the context has changed.

38 let didPerformWorkStackCursor: StackCursor<boolean> = createCursor(false);

39 // Keep track of the previous context object that was on the stack.

40 // We use this to get access to the parent context after we have already

41 // pushed the next context provider, and now need to merge their contexts.

38 -let previousContext: Object = emptyObject;

42 +let previousContext: Object = emptyContextObject;

44 function getUnmaskedContext(workInProgress: Fiber): Object {

45 const hasOwnContext = isContextProvider(workInProgress);

@@ -66,7 +70,7 @@ function getMaskedContext(

70 const type = workInProgress.type;

71 const contextTypes = type.contextTypes;

72 if (!contextTypes) {

69 - return emptyObject;

73 + return emptyContextObject;

74 }

76 // Avoid recreating masked context unless unmasked context has changed.

@@ -137,7 +141,7 @@ function pushTopLevelContextObject(

141 didChange: boolean,

142 ): void {

143 invariant(

140 - contextStackCursor.current === emptyObject,

144 + contextStackCursor.current === emptyContextObject,

145 'Unexpected context found on stack. ' +

146 'This error is likely caused by a bug in React. Please file an issue.',

147 );

@@ -219,7 +223,7 @@ function pushContextProvider(workInProgress: Fiber): boolean {

223 // and replace it on the stack later when invalidating the context.

224 const memoizedMergedChildContext =

225 (instance && instance.__reactInternalMemoizedMergedChildContext) ||

222 - emptyObject;

226 + emptyContextObject;

227

228 // Remember the parent context so we can merge with it later.

229 // Inherit the parent's did-perform-work value to avoid inadvertently blocking updates.

packages/react-reconciler/src/ReactFiberReconciler.js

+2 -2

@@ -24,7 +24,6 @@ import {

24 } from 'react-reconciler/reflection';

25 import * as ReactInstanceMap from 'shared/ReactInstanceMap';

26 import {HostComponent} from 'shared/ReactTypeOfWork';

27 -import emptyObject from 'fbjs/lib/emptyObject';

27 import getComponentName from 'shared/getComponentName';

28 import invariant from 'fbjs/lib/invariant';

29 import warning from 'fbjs/lib/warning';

@@ -34,6 +33,7 @@ import {

33 findCurrentUnmaskedContext,

34 isContextProvider,

35 processChildContext,

36 + emptyContextObject,

37 } from './ReactFiberContext';

38 import {createFiberRoot} from './ReactFiberRoot';

39 import * as ReactFiberDevToolsHook from './ReactFiberDevToolsHook';

@@ -86,7 +86,7 @@ function getContextForSubtree(

86 parentComponent: ?React$Component<any, any>,

87 ): Object {

88 if (!parentComponent) {

89 - return emptyObject;

89 + return emptyContextObject;

90 }

92 const fiber = ReactInstanceMap.get(parentComponent);

packages/react-test-renderer/src/ReactShallowRenderer.js

+5 -1

@@ -11,10 +11,14 @@ import {isForwardRef} from 'react-is';

11 import describeComponentFrame from 'shared/describeComponentFrame';

12 import getComponentName from 'shared/getComponentName';

13 import shallowEqual from 'shared/shallowEqual';

14 -import emptyObject from 'fbjs/lib/emptyObject';

14 import invariant from 'fbjs/lib/invariant';

15 import checkPropTypes from 'prop-types/checkPropTypes';

17 +const emptyObject = {};

18 +if (__DEV__) {

19 + Object.freeze(emptyObject);

20 +}

21 +

22 class ReactShallowRenderer {

23 static createRenderer = function() {

24 return new ReactShallowRenderer();

packages/react-test-renderer/src/ReactTestHostConfig.js

+9 -6

@@ -7,8 +7,6 @@

7 * @flow

8 */

10 -import emptyObject from 'fbjs/lib/emptyObject';

11 -

10 import * as TestRendererScheduling from './ReactTestRendererScheduling';

12 export type Type = string;

@@ -35,11 +33,16 @@ export type HostContext = Object;

33 export type UpdatePayload = Object;

34 export type ChildSet = void; // Unused

38 -const UPDATE_SIGNAL = {};

39 -

36 export * from 'shared/HostConfigWithNoPersistence';

37 export * from 'shared/HostConfigWithNoHydration';

39 +const NO_CONTEXT = {};

40 +const UPDATE_SIGNAL = {};

41 +if (__DEV__) {

42 + Object.freeze(NO_CONTEXT);

43 + Object.freeze(UPDATE_SIGNAL);

44 +}

45 +

46 export function getPublicInstance(inst: Instance | TextInstance): * {

47 switch (inst.tag) {

48 case 'INSTANCE':

@@ -88,7 +91,7 @@ export function removeChild(

91 export function getRootHostContext(

92 rootContainerInstance: Container,

93 ): HostContext {

91 - return emptyObject;

94 + return NO_CONTEXT;

95 }

97 export function getChildHostContext(

@@ -96,7 +99,7 @@ export function getChildHostContext(

99 type: string,

100 rootContainerInstance: Container,

101 ): HostContext {

99 - return emptyObject;

102 + return NO_CONTEXT;

103 }

104

105 export function prepareForCommit(containerInfo: Container): void {

packages/react/src/ReactBaseClasses.js

+7 -1

@@ -5,18 +5,23 @@

5 * LICENSE file in the root directory of this source tree.

6 */

8 -import emptyObject from 'fbjs/lib/emptyObject';

8 import invariant from 'fbjs/lib/invariant';

9 import lowPriorityWarning from 'shared/lowPriorityWarning';

11 import ReactNoopUpdateQueue from './ReactNoopUpdateQueue';

13 +const emptyObject = {};

14 +if (__DEV__) {

15 + Object.freeze(emptyObject);

16 +}

17 +

18 /**

19 * Base class helpers for the updating state of a component.

20 */

21 function Component(props, context, updater) {

22 this.props = props;

23 this.context = context;

24 + // If a component has string refs, we will assign a different object later.

25 this.refs = emptyObject;

26 // We initialize the default updater but the real one gets injected by the

27 // renderer.

@@ -126,6 +131,7 @@ ComponentDummy.prototype = Component.prototype;

131 function PureComponent(props, context, updater) {

132 this.props = props;

133 this.context = context;

134 + // If a component has string refs, we will assign a different object later.

135 this.refs = emptyObject;

136 this.updater = updater || ReactNoopUpdateQueue;

137 }

scripts/jest/setupEnvironment.js

-6

@@ -46,9 +46,3 @@ if (typeof window !== 'undefined') {

46 }

47 });

48 }

49 -

50 -// Preserve the empty object identity across module resets.

51 -// This is needed for some tests that rely on string refs

52 -// but reset modules between loading different renderers.

53 -const obj = require.requireActual('fbjs/lib/emptyObject');

54 -jest.mock('fbjs/lib/emptyObject', () => obj);