@samitouri / QOS-React / commits / 8ecb248e6d

Make numerical refs work in Fiber (#8334)

Ben Alpert committed Nov 17, 2016 at 18:56 UTC 8ecb248e6d0f7e33bea9816bfbb7c49551d7bc5f

5 files changed +24 -15

scripts/fiber/tests-failing.txt

-1

@@ -384,7 +384,6 @@ src/renderers/shared/shared/event/__tests__/EventPluginHub-test.js

384

385 src/renderers/shared/stack/reconciler/__tests__/ReactComponent-test.js

386 * should throw on invalid render targets

387 -* should throw when supplying a ref outside of render method

387 * throws usefully when rendering badly-typed elements

388

389 src/renderers/shared/stack/reconciler/__tests__/ReactComponentLifeCycle-test.js

scripts/fiber/tests-passing.txt

@@ -1019,6 +1019,7 @@ src/renderers/shared/shared/event/eventPlugins/__tests__/ResponderEventPlugin-te

1019 * should cancel correctly

1020

1021 src/renderers/shared/stack/reconciler/__tests__/ReactComponent-test.js

1022 +* should throw when supplying a ref outside of render method

1023 * should warn when children are mutated during render

1024 * should warn when children are mutated during update

1025 * should support refs on owned components

@@ -1234,6 +1235,7 @@ src/renderers/shared/stack/reconciler/__tests__/refs-test.js

1235 * always has a value for this.refs

1236 * ref called correctly for stateless component when __DEV__ = false

1237 * ref called correctly for stateless component when __DEV__ = true

1238 +* coerces numbers to strings

1239

1240 src/renderers/shared/utils/__tests__/accumulateInto-test.js

1241 * throws if the second item is null

src/renderers/shared/fiber/ReactChildFiber.js

+12 -13

@@ -60,16 +60,16 @@ const {

60 Deletion,

61 } = ReactTypeOfSideEffect;

63 -function transferRef(current: ?Fiber, workInProgress: Fiber, element: ReactElement<any>) {

64 - if (typeof element.ref === 'string') {

63 +function coerceRef(current: ?Fiber, element: ReactElement<any>) {

64 + let mixedRef = element.ref;

65 + if (mixedRef != null && typeof mixedRef !== 'function') {

66 if (element._owner) {

67 const ownerFiber : ?Fiber = (element._owner : any);

68 if (ownerFiber && ownerFiber.tag === ClassComponent) {

68 - const stringRef = element.ref;

69 + const stringRef = String(mixedRef);

70 // Check if previous string ref matches new string ref

71 if (current && current.ref && current.ref._stringRef === stringRef) {

71 - workInProgress.ref = current.ref;

72 - return;

72 + return current.ref;

73 }

74 const inst = ownerFiber.stateNode;

75 const ref = function(value) {

@@ -77,12 +77,11 @@ function transferRef(current: ?Fiber, workInProgress: Fiber, element: ReactEleme

77 refs[stringRef] = value;

78 };

79 ref._stringRef = stringRef;

80 - workInProgress.ref = ref;

80 + return ref;

81 }

82 }

83 - } else {

84 - workInProgress.ref = element.ref;

83 }

84 + return mixedRef;

85 }

87 // This wrapper function exists because I expect to clone the code in each path

@@ -244,13 +243,13 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {

243 if (current == null || current.type !== element.type) {

244 // Insert

245 const created = createFiberFromElement(element, priority);

247 - transferRef(current, created, element);

246 + created.ref = coerceRef(current, element);

247 created.return = returnFiber;

248 return created;

249 } else {

250 // Move based on index

251 const existing = useFiber(current, priority);

253 - transferRef(current, existing, element);

252 + existing.ref = coerceRef(current, element);

253 existing.pendingProps = element.props;

254 existing.return = returnFiber;

255 return existing;

@@ -342,7 +341,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {

341 switch (newChild.$$typeof) {

342 case REACT_ELEMENT_TYPE: {

343 const created = createFiberFromElement(newChild, priority);

345 - transferRef(null, created, newChild);

344 + created.ref = coerceRef(null, newChild);

345 created.return = returnFiber;

346 return created;

347 }

@@ -679,7 +678,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {

678 if (child.type === element.type) {

679 deleteRemainingChildren(returnFiber, child.sibling);

680 const existing = useFiber(child, priority);

682 - transferRef(child, existing, element);

681 + existing.ref = coerceRef(child, element);

682 existing.pendingProps = element.props;

683 existing.return = returnFiber;

684 return existing;

@@ -694,7 +693,7 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {

693 }

694

695 const created = createFiberFromElement(element, priority);

697 - transferRef(currentFirstChild, created, element);

696 + created.ref = coerceRef(currentFirstChild, element);

697 created.return = returnFiber;

698 return created;

699 }

src/renderers/shared/stack/reconciler/__tests__/ReactUpdates-test.js

-1

@@ -897,7 +897,6 @@ describe('ReactUpdates', () => {

897

898 var component = ReactTestUtils.renderIntoDocument(<A />);

899

900 - console.log('xxx', expect().toThrowError);

900 expect(() => component.setState({}, 'no')).toThrowError(

901 'setState(...): Expected the last optional `callback` argument ' +

902 'to be a function. Instead received: string.'

src/renderers/shared/stack/reconciler/__tests__/refs-test.js

+10

@@ -273,4 +273,14 @@ describe('ref swapping', () => {

273 testRefCall();

274 __DEV__ = originalDev;

275 });

276 +

277 + it('coerces numbers to strings', () => {

278 + class A extends React.Component {

279 + render() {

280 + return <div ref={1} />;

281 + }

282 + }

283 + const a = ReactTestUtils.renderIntoDocument(<A />);

284 + expect(a.refs[1].nodeName).toBe('DIV');

285 + });

286 });