Skip to content

Commit 80bdebc

Browse files
iamdustangaearon
authored andcommitted
Fix ReactFiberReconciler annotation to include PI (#8751)
* Fix ReactFiberReconciler annotation to include `PI` #8628 (comment) * Make getPublicInstance type safe * attempting to trigger the issue @sebmarkbage described #8751 (comment) * Unify renderer-specific getPublicInstance with getRootInstance * Switch on fiber type HostComponent for getPublicRootInstance * Fix test that was too dynamic (and failing) * Use PI in reconciler public API * Prettier
1 parent af3ba36 commit 80bdebc

6 files changed

Lines changed: 117 additions & 6 deletions

File tree

scripts/fiber/tests-passing.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -610,6 +610,7 @@ src/renderers/__tests__/refs-test.js
610610
* coerces numbers to strings
611611
* attaches, detaches from fiber component with stack layer
612612
* attaches, detaches from stack component with fiber layer
613+
* attaches and detaches root refs
613614

614615
src/renderers/art/__tests__/ReactART-test.js
615616
* should have the correct lifecycle state
@@ -1972,6 +1973,7 @@ src/renderers/testing/__tests__/ReactTestRenderer-test.js
19721973
* can update text nodes
19731974
* toTree() renders simple components returning host components
19741975
* toTree() handles null rendering components
1976+
* root instance and createNodeMock ref return the same value
19751977
* toTree() renders complicated trees of composites and hosts
19761978
* can update text nodes when rendered as root
19771979
* can render and update root fragments

src/renderers/__tests__/refs-test.js

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -409,3 +409,87 @@ describe('string refs between fiber and stack', () => {
409409
}
410410
});
411411
});
412+
413+
describe('root level refs', () => {
414+
beforeEach(() => {
415+
var ReactFeatureFlags = require('ReactFeatureFlags');
416+
ReactFeatureFlags.disableNewFiberFeatures = false;
417+
});
418+
419+
it('attaches and detaches root refs', () => {
420+
var ReactDOM = require('react-dom');
421+
var inst = null;
422+
423+
// host node
424+
var ref = jest.fn(value => (inst = value));
425+
var container = document.createElement('div');
426+
var result = ReactDOM.render(<div ref={ref} />, container);
427+
expect(ref).toHaveBeenCalledTimes(1);
428+
expect(ref.mock.calls[0][0]).toBeInstanceOf(HTMLDivElement);
429+
expect(result).toBe(ref.mock.calls[0][0]);
430+
ReactDOM.unmountComponentAtNode(container);
431+
expect(ref).toHaveBeenCalledTimes(2);
432+
expect(ref.mock.calls[1][0]).toBe(null);
433+
434+
// composite
435+
class Comp extends React.Component {
436+
method() {
437+
return true;
438+
}
439+
render() {
440+
return <div>Comp</div>;
441+
}
442+
}
443+
444+
inst = null;
445+
ref = jest.fn(value => (inst = value));
446+
result = ReactDOM.render(<Comp ref={ref} />, container);
447+
448+
expect(ref).toHaveBeenCalledTimes(1);
449+
expect(inst).toBeInstanceOf(Comp);
450+
expect(result).toBe(inst);
451+
452+
// ensure we have the correct instance
453+
expect(result.method()).toBe(true);
454+
expect(inst.method()).toBe(true);
455+
456+
ReactDOM.unmountComponentAtNode(container);
457+
expect(ref).toHaveBeenCalledTimes(2);
458+
expect(ref.mock.calls[1][0]).toBe(null);
459+
460+
if (ReactDOMFeatureFlags.useFiber) {
461+
// fragment
462+
inst = null;
463+
ref = jest.fn(value => (inst = value));
464+
var divInst = null;
465+
var ref2 = jest.fn(value => (divInst = value));
466+
result = ReactDOM.render(
467+
[<Comp ref={ref} key="a" />, 5, <div ref={ref2} key="b">Hello</div>],
468+
container,
469+
);
470+
471+
// first call should be `Comp`
472+
expect(ref).toHaveBeenCalledTimes(1);
473+
expect(ref.mock.calls[0][0]).toBeInstanceOf(Comp);
474+
expect(result).toBe(ref.mock.calls[0][0]);
475+
476+
expect(ref2).toHaveBeenCalledTimes(1);
477+
expect(divInst).toBeInstanceOf(HTMLDivElement);
478+
expect(result).not.toBe(divInst);
479+
480+
ReactDOM.unmountComponentAtNode(container);
481+
expect(ref).toHaveBeenCalledTimes(2);
482+
expect(ref.mock.calls[1][0]).toBe(null);
483+
expect(ref2).toHaveBeenCalledTimes(2);
484+
expect(ref2.mock.calls[1][0]).toBe(null);
485+
486+
// null
487+
result = ReactDOM.render(null, container);
488+
expect(result).toBe(null);
489+
490+
// primitives
491+
result = ReactDOM.render(5, container);
492+
expect(result).toBeInstanceOf(Text);
493+
}
494+
});
495+
});

src/renderers/shared/fiber/ReactFiberCommitWork.js

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -518,8 +518,14 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
518518
function commitAttachRef(finishedWork: Fiber) {
519519
const ref = finishedWork.ref;
520520
if (ref !== null) {
521-
const instance = getPublicInstance(finishedWork.stateNode);
522-
ref(instance);
521+
const instance = finishedWork.stateNode;
522+
switch (finishedWork.tag) {
523+
case HostComponent:
524+
ref(getPublicInstance(instance));
525+
break;
526+
default:
527+
ref(instance);
528+
}
523529
}
524530
}
525531

src/renderers/shared/fiber/ReactFiberReconciler.js

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ var {
2828
} = require('ReactFiberContext');
2929
var {createFiberRoot} = require('ReactFiberRoot');
3030
var ReactFiberScheduler = require('ReactFiberScheduler');
31+
var {HostComponent} = require('ReactTypeOfWork');
3132

3233
if (__DEV__) {
3334
var warning = require('fbjs/lib/warning');
@@ -167,6 +168,8 @@ getContextForSubtree._injectFiber(function(fiber: Fiber) {
167168
module.exports = function<T, P, I, TI, PI, C, CX, PL>(
168169
config: HostConfig<T, P, I, TI, PI, C, CX, PL>,
169170
): Reconciler<C, I, TI> {
171+
var {getPublicInstance} = config;
172+
170173
var {
171174
scheduleUpdate,
172175
getPriorityContext,
@@ -269,15 +272,20 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
269272

270273
getPublicRootInstance(
271274
container: OpaqueRoot,
272-
): ReactComponent<any, any, any> | I | TI | null {
275+
): ReactComponent<any, any, any> | PI | null {
273276
const containerFiber = container.current;
274277
if (!containerFiber.child) {
275278
return null;
276279
}
277-
return containerFiber.child.stateNode;
280+
switch (containerFiber.child.tag) {
281+
case HostComponent:
282+
return getPublicInstance(containerFiber.child.stateNode);
283+
default:
284+
return containerFiber.child.stateNode;
285+
}
278286
},
279287

280-
findHostInstance(fiber: Fiber): I | TI | null {
288+
findHostInstance(fiber: Fiber): PI | null {
281289
const hostFiber = findCurrentHostFiber(fiber);
282290
if (hostFiber === null) {
283291
return null;

src/renderers/testing/ReactTestRendererFiber.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ var TestRenderer = ReactFiberReconciler({
219219

220220
useSyncScheduling: true,
221221

222-
getPublicInstance(inst) {
222+
getPublicInstance(inst: Instance | TextInstance): * {
223223
switch (inst.tag) {
224224
case 'INSTANCE':
225225
const createNodeMock = inst.rootContainerInstance.createNodeMock;

src/renderers/testing/__tests__/ReactTestRenderer-test.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -568,6 +568,17 @@ describe('ReactTestRenderer', () => {
568568
});
569569
});
570570

571+
it('root instance and createNodeMock ref return the same value', () => {
572+
var createNodeMock = ref => ({node: ref});
573+
var refInst = null;
574+
var renderer = ReactTestRenderer.create(
575+
<div ref={ref => (refInst = ref)} />,
576+
{createNodeMock},
577+
);
578+
var root = renderer.getInstance();
579+
expect(root).toEqual(refInst);
580+
});
581+
571582
it('toTree() renders complicated trees of composites and hosts', () => {
572583
// SFC returning host. no children props.
573584
var Qoo = () => <span className="Qoo">Hello World!</span>;

0 commit comments

Comments
 (0)