Skip to content

Commit

Permalink
More fixes for issues introduced by rebasing
Browse files Browse the repository at this point in the history
**what is the change?:**
- Remove some outdated 'require' statements that got orphaned in 'React.js'
- Change 'warning' to 'lowPriorityWarning' for 'React.createClass'
- Fix syntax issues in 'React-test'
- Use 'creatReactClass' instead of ES6 class in ReactART

**why make this change?:**
These were bugs introduced by rebasing and tests caught the regressions.

**test plan:**
`yarn test`

**issue:**
#9398
  • Loading branch information
flarnie committed May 25, 2017
1 parent a4cc843 commit f619d51
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 20 deletions.
5 changes: 1 addition & 4 deletions src/isomorphic/React.js
Expand Up @@ -13,16 +13,13 @@

var ReactBaseClasses = require('ReactBaseClasses');
var ReactChildren = require('ReactChildren');
var ReactComponent = require('ReactComponent');
var ReactPureComponent = require('ReactPureComponent');
var ReactDOMFactories = require('ReactDOMFactories');
var ReactElement = require('ReactElement');
var ReactPropTypes = require('ReactPropTypes');
var ReactVersion = require('ReactVersion');

var createReactClass = require('createClass');
var onlyChild = require('onlyChild');
var warning = require('warning');

var createElement = ReactElement.createElement;
var createFactory = ReactElement.createFactory;
Expand Down Expand Up @@ -112,7 +109,7 @@ if (__DEV__) {

Object.defineProperty(React, 'createClass', {
get: function() {
warning(
lowPriorityWarning(
warnedForCreateClass,
'React.createClass is no longer supported. Use a plain JavaScript ' +
"class instead. If you're not yet ready to migrate, " +
Expand Down
12 changes: 6 additions & 6 deletions src/isomorphic/__tests__/React-test.js
Expand Up @@ -39,12 +39,12 @@ describe('React', () => {
});

it('should warn once when attempting to access React.createClass', () => {
spyOn(console, 'error');
spyOn(console, 'warn');
let createClass = React.createClass;
createClass = React.createClass;
expect(createClass).not.toBe(undefined);
expectDev(console.error.calls.count()).toBe(1);
expectDev(console.error.calls.argsFor(0)[0]).toContain(
expect(console.warn.calls.count()).toBe(1);
expect(console.warn.calls.argsFor(0)[0]).toContain(
'React.createClass is no longer supported. Use a plain ' +
"JavaScript class instead. If you're not yet ready to migrate, " +
'create-react-class is available on npm as a temporary, ' +
Expand All @@ -53,12 +53,12 @@ describe('React', () => {
});

it('should warn once when attempting to access React.PropTypes', () => {
spyOn(console, 'error');
spyOn(console, 'warn');
let PropTypes = React.PropTypes;
PropTypes = React.PropTypes;
expect(PropTypes).not.toBe(undefined);
expectDev(console.error.calls.count()).toBe(1);
expectDev(console.error.calls.argsFor(0)[0]).toContain(
expect(console.warn.calls.count()).toBe(1);
expect(console.warn.calls.argsFor(0)[0]).toContain(
'PropTypes have moved out of the react package. ' +
'Use the prop-types package from npm instead.',
);
Expand Down
25 changes: 15 additions & 10 deletions src/renderers/art/ReactART.js
Expand Up @@ -24,6 +24,7 @@ const ReactInstanceMap = require('ReactInstanceMap');
const ReactMultiChild = require('ReactMultiChild');
const ReactUpdates = require('ReactUpdates');

const createReactClass = require('createClass');
const emptyObject = require('emptyObject');
const invariant = require('invariant');

Expand Down Expand Up @@ -171,8 +172,12 @@ const ContainerMixin = assign({}, ReactMultiChild.Mixin, {
// Surface is a React DOM Component, not an ART component. It serves as the
// entry point into the ART reconciler.

class Surface extends React.Component {
componentDidMount() {
const Surface = createReactClass({
displayName: 'Surface',

mixins: [ContainerMixin],

componentDidMount: function() {
const domNode = ReactDOM.findDOMNode(this);

this.node = Mode.Surface(+this.props.width, +this.props.height, domNode);
Expand All @@ -186,9 +191,9 @@ class Surface extends React.Component {
ReactInstanceMap.get(this)._context,
);
ReactUpdates.ReactReconcileTransaction.release(transaction);
}
},

componentDidUpdate(oldProps) {
componentDidUpdate: function(oldProps) {
const node = this.node;
if (
this.props.width != oldProps.width ||
Expand All @@ -210,13 +215,13 @@ class Surface extends React.Component {
if (node.render) {
node.render();
}
}
},

componentWillUnmount() {
componentWillUnmount: function() {
this.unmountChildren();
}
},

render() {
render: function() {
// This is going to be a placeholder because we don't know what it will
// actually resolve to because ART may render canvas, vml or svg tags here.
// We only allow a subset of properties since others might conflict with
Expand All @@ -234,8 +239,8 @@ class Surface extends React.Component {
title={props.title}
/>
);
}
}
},
});

// Various nodes that can go into a surface

Expand Down

0 comments on commit f619d51

Please sign in to comment.