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.

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

Resolve host configs at build time #12792

Merged
merged 15 commits into from
May 19, 2018
7 changes: 1 addition & 6 deletions packages/react-art/src/ReactART.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,11 @@
*/

import React from 'react';
import ReactFiberReconciler from 'react-reconciler';
import * as ARTRenderer from 'react-reconciler/inline.art';
import Transform from 'art/core/transform';
import Mode from 'art/modes/current';
import FastNoSideEffects from 'art/modes/fast-noSideEffects';

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

Mode.setCurrent(
Expand Down Expand Up @@ -132,10 +131,6 @@ class Text extends React.Component {
}
}

/** ART Renderer */

const ARTRenderer = ReactFiberReconciler(ReactARTHostConfig);

/** API */

export const ClippingRectangle = TYPES.CLIPPING_RECTANGLE;
Expand Down
116 changes: 63 additions & 53 deletions packages/react-art/src/ReactARTHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -234,18 +234,20 @@ function applyTextProps(instance, props, prevProps = {}) {
}
}

const ReactARTHostConfig = {
appendInitialChild(parentInstance, child) {
export * from 'shared/HostConfigWithNoPersistence';
export * from 'shared/HostConfigWithNoHydration';

export function appendInitialChild(parentInstance, child) {
if (typeof child === 'string') {
// Noop for string children of Text (eg <Text>{'foo'}{'bar'}</Text>)
invariant(false, 'Text children should already be flattened.');
return;
}

child.inject(parentInstance);
},
}

createInstance(type, props, internalInstanceHandle) {
export function createInstance(type, props, internalInstanceHandle) {
let instance;

switch (type) {
Expand Down Expand Up @@ -277,114 +279,122 @@ const ReactARTHostConfig = {
instance._applyProps(instance, props);

return instance;
},
}

createTextInstance(text, rootContainerInstance, internalInstanceHandle) {
export function createTextInstance(
text,
rootContainerInstance,
internalInstanceHandle,
) {
return text;
},
}

finalizeInitialChildren(domElement, type, props) {
export function finalizeInitialChildren(domElement, type, props) {
return false;
},
}

getPublicInstance(instance) {
export function getPublicInstance(instance) {
return instance;
},
}

prepareForCommit() {
export function prepareForCommit() {
// Noop
},
}

prepareUpdate(domElement, type, oldProps, newProps) {
export function prepareUpdate(domElement, type, oldProps, newProps) {
return UPDATE_SIGNAL;
},
}

resetAfterCommit() {
export function resetAfterCommit() {
// Noop
},
}

resetTextContent(domElement) {
export function resetTextContent(domElement) {
// Noop
},
}

shouldDeprioritizeSubtree(type, props) {
export function shouldDeprioritizeSubtree(type, props) {
return false;
},
}

getRootHostContext() {
export function getRootHostContext() {
return emptyObject;
},
}

getChildHostContext() {
export function getChildHostContext() {
return emptyObject;
},
}

scheduleDeferredCallback: ReactScheduler.scheduleWork,
export const scheduleDeferredCallback = ReactScheduler.scheduleWork;
export const cancelDeferredCallback = ReactScheduler.cancelScheduledWork;

shouldSetTextContent(type, props) {
export function shouldSetTextContent(type, props) {
return (
typeof props.children === 'string' || typeof props.children === 'number'
);
},
}

now: ReactScheduler.now,
export const now = ReactScheduler.now;

// The ART renderer is secondary to the React DOM renderer.
isPrimaryRenderer: false,
export const isPrimaryRenderer = false;

mutation: {
appendChild(parentInstance, child) {
export const supportsMutation = true;

export function appendChild(parentInstance, child) {
if (child.parentNode === parentInstance) {
child.eject();
}
child.inject(parentInstance);
},
}

appendChildToContainer(parentInstance, child) {
export function appendChildToContainer(parentInstance, child) {
if (child.parentNode === parentInstance) {
child.eject();
}
child.inject(parentInstance);
},
}

insertBefore(parentInstance, child, beforeChild) {
export function insertBefore(parentInstance, child, beforeChild) {
invariant(
child !== beforeChild,
'ReactART: Can not insert node before itself',
);
child.injectBefore(beforeChild);
},
}

insertInContainerBefore(parentInstance, child, beforeChild) {
export function insertInContainerBefore(parentInstance, child, beforeChild) {
invariant(
child !== beforeChild,
'ReactART: Can not insert node before itself',
);
child.injectBefore(beforeChild);
},
}

removeChild(parentInstance, child) {
export function removeChild(parentInstance, child) {
destroyEventListeners(child);
child.eject();
},
}

removeChildFromContainer(parentInstance, child) {
export function removeChildFromContainer(parentInstance, child) {
destroyEventListeners(child);
child.eject();
},
}

commitTextUpdate(textInstance, oldText, newText) {
export function commitTextUpdate(textInstance, oldText, newText) {
// Noop
},
}

commitMount(instance, type, newProps) {
export function commitMount(instance, type, newProps) {
// Noop
},
}

commitUpdate(instance, updatePayload, type, oldProps, newProps) {
export function commitUpdate(
instance,
updatePayload,
type,
oldProps,
newProps,
) {
instance._applyProps(instance, newProps, oldProps);
},
},
};

export default ReactARTHostConfig;
}
38 changes: 22 additions & 16 deletions packages/react-art/src/__tests__/ReactART-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,26 @@ const React = require('react');
const ReactDOM = require('react-dom');
const ReactTestUtils = require('react-dom/test-utils');

let Group;
let Shape;
let Surface;
let TestComponent;

const Missing = {};
// Isolate test renderer.
jest.resetModules();
const ReactTestRenderer = require('react-test-renderer');

// Isolate ART renderer.
Copy link
Contributor

Choose a reason for hiding this comment

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

It's unfortunate that forgetting to do this causes a totally non-obvious (to me) failure:
screen shot 2018-05-18 at 2 24 12 pm

I guess this is the only test where we need to do this though?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah. But good point. I think we might want to try to fail early although I’m not yet sure how.

jest.resetModules();
const ReactART = require('react-art');
const ARTSVGMode = require('art/modes/svg');
const ARTCurrentMode = require('art/modes/current');

const renderer = require('react-test-renderer');
const Circle = require('react-art/Circle');
const Rectangle = require('react-art/Rectangle');
const Wedge = require('react-art/Wedge');

let Group;
let Shape;
let Surface;
let TestComponent;

const Missing = {};

function testDOMNodeStructure(domNode, expectedStructure) {
expect(domNode).toBeDefined();
expect(domNode.nodeName).toBe(expectedStructure.nodeName);
Expand Down Expand Up @@ -362,7 +366,7 @@ describe('ReactART', () => {

// Using test renderer instead of the DOM renderer here because async
// testing APIs for the DOM renderer don't exist.
const testRenderer = renderer.create(
const testRenderer = ReactTestRenderer.create(
<CurrentRendererContext.Provider value="Test">
<Yield value="A" />
<Yield value="B" />
Expand Down Expand Up @@ -397,15 +401,17 @@ describe('ReactART', () => {

describe('ReactARTComponents', () => {
it('should generate a <Shape> with props for drawing the Circle', () => {
const circle = renderer.create(
const circle = ReactTestRenderer.create(
<Circle radius={10} stroke="green" strokeWidth={3} fill="blue" />,
);
expect(circle.toJSON()).toMatchSnapshot();
});

it('should warn if radius is missing on a Circle component', () => {
expect(() =>
renderer.create(<Circle stroke="green" strokeWidth={3} fill="blue" />),
ReactTestRenderer.create(
<Circle stroke="green" strokeWidth={3} fill="blue" />,
),
).toWarnDev(
'Warning: Failed prop type: The prop `radius` is marked as required in `Circle`, ' +
'but its value is `undefined`.' +
Expand All @@ -414,15 +420,15 @@ describe('ReactARTComponents', () => {
});

it('should generate a <Shape> with props for drawing the Rectangle', () => {
const rectangle = renderer.create(
const rectangle = ReactTestRenderer.create(
<Rectangle width={50} height={50} stroke="green" fill="blue" />,
);
expect(rectangle.toJSON()).toMatchSnapshot();
});

it('should warn if width/height is missing on a Rectangle component', () => {
expect(() =>
renderer.create(<Rectangle stroke="green" fill="blue" />),
ReactTestRenderer.create(<Rectangle stroke="green" fill="blue" />),
).toWarnDev([
'Warning: Failed prop type: The prop `width` is marked as required in `Rectangle`, ' +
'but its value is `undefined`.' +
Expand All @@ -434,21 +440,21 @@ describe('ReactARTComponents', () => {
});

it('should generate a <Shape> with props for drawing the Wedge', () => {
const wedge = renderer.create(
const wedge = ReactTestRenderer.create(
<Wedge outerRadius={50} startAngle={0} endAngle={360} fill="blue" />,
);
expect(wedge.toJSON()).toMatchSnapshot();
});

it('should return null if startAngle equals to endAngle on Wedge', () => {
const wedge = renderer.create(
const wedge = ReactTestRenderer.create(
<Wedge outerRadius={50} startAngle={0} endAngle={0} fill="blue" />,
);
expect(wedge.toJSON()).toBeNull();
});

it('should warn if outerRadius/startAngle/endAngle is missing on a Wedge component', () => {
expect(() => renderer.create(<Wedge fill="blue" />)).toWarnDev([
expect(() => ReactTestRenderer.create(<Wedge fill="blue" />)).toWarnDev([
'Warning: Failed prop type: The prop `outerRadius` is marked as required in `Wedge`, ' +
'but its value is `undefined`.' +
'\n in Wedge (at **)',
Expand Down
5 changes: 1 addition & 4 deletions packages/react-dom/src/client/ReactDOM.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import type {Container} from './ReactDOMHostConfig';
import '../shared/checkReact';
import './ReactDOMClientInjection';

import ReactFiberReconciler from 'react-reconciler';
import * as DOMRenderer from 'react-reconciler/inline.dom';
import * as ReactPortal from 'shared/ReactPortal';
import ExecutionEnvironment from 'fbjs/lib/ExecutionEnvironment';
import * as ReactGenericBatching from 'events/ReactGenericBatching';
Expand All @@ -35,7 +35,6 @@ import invariant from 'fbjs/lib/invariant';
import lowPriorityWarning from 'shared/lowPriorityWarning';
import warning from 'fbjs/lib/warning';

import ReactDOMHostConfig from './ReactDOMHostConfig';
import * as ReactDOMComponentTree from './ReactDOMComponentTree';
import * as ReactDOMFiberComponent from './ReactDOMFiberComponent';
import * as ReactDOMEventListener from '../events/ReactDOMEventListener';
Expand Down Expand Up @@ -447,8 +446,6 @@ function shouldHydrateDueToLegacyHeuristic(container) {
);
}

const DOMRenderer = ReactFiberReconciler(ReactDOMHostConfig);

ReactGenericBatching.injection.injectRenderer(DOMRenderer);

let warnedAboutHydrateAPI = false;
Expand Down
Loading