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
- Update 'prop-type' dependency to use no higher than 15.7 because 15.8 limits the number of warnings, and this causes a test to fail.
- Fix some mixed-up and misnamed variables in `React.js`
- Rebase onto commit that updates deprecation messages
- Update a test based on new deprecation messages

**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 3d1062b commit 27bbbec
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 31 deletions.
2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -85,7 +85,7 @@
},
"dependencies": {
"create-react-class": "^15.5.2",
"prop-types": "^15.5.6"
"prop-types": "15.5.7"
},
"commonerConfig": {
"version": 7
Expand Down
8 changes: 2 additions & 6 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 @@ -107,7 +104,6 @@ var React = {
};

if (__DEV__) {
let warnedForCreateMixin = false;
let warnedForCreateClass = false;
if (canDefineProperty) {
Object.defineProperty(React, 'PropTypes', {
Expand All @@ -128,14 +124,14 @@ 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, " +
'create-react-class is available on npm as a temporary, ' +
'drop-in replacement.',
);
didWarnPropTypesDeprecated = true;
warnedForCreateClass = true;
return createReactClass;
},
});
Expand Down
20 changes: 12 additions & 8 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,14 +53,18 @@ 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(
'PropTypes have moved out of the react package. ' +
'Use the prop-types package from npm instead.',
expect(console.warn.calls.count()).toBe(1);
expect(console.warn.calls.argsFor(0)[0]).toContain(
'Warning: Accessing PropTypes via the main React package is ' +
'deprecated, and will be removed in React v16.0. ' +
'Use the prop-types package from npm instead. ' +
'Version 15.5.10 provides a drop-in replacement. ' +
'For info on usage, compatibility, migration and more, ' +
'see https://fb.me/prop-types-docs',
);
});
});
Expand Up @@ -324,7 +324,8 @@ describe('create-react-class-integration', () => {
},
});
expect(() =>
ReactTestUtils.renderIntoDocument(<Component />)).not.toThrow();
ReactTestUtils.renderIntoDocument(<Component />),
).not.toThrow();
});

it('should throw when using legacy factories', () => {
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
2 changes: 1 addition & 1 deletion src/renderers/shared/utils/Transaction.js
Expand Up @@ -134,7 +134,7 @@ var TransactionImpl = {
E,
F,
G,
T: (a: A, b: B, c: C, d: D, e: E, f: F) => G
T: (a: A, b: B, c: C, d: D, e: E, f: F) => G,
>(method: T, scope: any, a: A, b: B, c: C, d: D, e: E, f: F): G {
/* eslint-enable space-before-function-paren */
invariant(
Expand Down
7 changes: 3 additions & 4 deletions yarn.lock
Expand Up @@ -4293,12 +4293,11 @@ promise@^7.1.1:
dependencies:
asap "~2.0.3"

prop-types@^15.5.6:
version "15.5.10"
resolved "https://registry.yarnpkg.com/prop-types/-/prop-types-15.5.10.tgz#2797dfc3126182e3a95e3dfbb2e893ddd7456154"
prop-types@15.5.7:
version "15.5.7"
resolved "https://registry.yarnpkg.com/prop-types/-/prop-types-15.5.7.tgz#231c4f29cdd82e355011d4889386ca9059544dd1"
dependencies:
fbjs "^0.8.9"
loose-envify "^1.3.1"

prr@~0.0.0:
version "0.0.0"
Expand Down

0 comments on commit 27bbbec

Please sign in to comment.