Skip to content

Remove createClass #9232

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

Merged
merged 8 commits into from
Apr 11, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions mocks/ReactElementTestChild.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@

var React = require('React');

var Child = React.createClass({
render: function() {
class Child extends React.Component {
render() {
return React.createElement('div');
},
});
}
}

module.exports = Child;
21 changes: 8 additions & 13 deletions mocks/ReactMockedComponentTestComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,23 +13,18 @@

var React = require('React');

var ReactMockedComponentTestComponent = React.createClass({
getDefaultProps: function() {
return {bar: 'baz'};
},
class ReactMockedComponentTestComponent extends React.Component {
state = {foo: 'bar'};

getInitialState: function() {
return {foo: 'bar'};
},

hasCustomMethod: function() {
hasCustomMethod() {
return true;
},
}

render: function() {
render() {
return <span />;
},
}

});
}
ReactMockedComponentTestComponent.defaultProps = {bar: 'baz'};

module.exports = ReactMockedComponentTestComponent;
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
"coffee-script": "^1.8.0",
"core-js": "^2.2.1",
"coveralls": "^2.11.6",
"create-react-class": "^15.5.0",
"del": "^2.0.2",
"derequire": "^2.0.3",
"escape-string-regexp": "^1.0.5",
Expand Down
50 changes: 5 additions & 45 deletions scripts/fiber/tests-passing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ scripts/shared/__tests__/evalToString-test.js

src/isomorphic/__tests__/React-test.js
* should log a deprecation warning once when using React.createMixin
* should warn once when attempting to access React.createClass

src/isomorphic/children/__tests__/ReactChildren-test.js
* should support identity for simple
Expand Down Expand Up @@ -89,23 +90,8 @@ src/isomorphic/classic/__tests__/ReactContextValidator-test.js
* should warn (but not error) if getChildContext method is missing
* should pass parent context if getChildContext method is missing

src/isomorphic/classic/class/__tests__/ReactBind-test.js
* Holds reference to instance
* works with mixins
* warns if you try to bind to this
* does not warn if you pass an auto-bound method to setState

src/isomorphic/classic/class/__tests__/ReactBindOptout-test.js
* should work with manual binding
* should not hold reference to instance
* works with mixins that have not opted out of autobinding
* works with mixins that have opted out of autobinding
* does not warn if you try to bind to this
* does not warn if you pass an manually bound method to setState

src/isomorphic/classic/class/__tests__/ReactClass-test.js
src/isomorphic/classic/class/__tests__/create-react-class-integration-test.js
* should throw when `render` is not specified
* should copy `displayName` onto the Constructor
* should copy prop types onto the Constructor
* should warn on invalid prop types
* should warn on invalid context types
Expand All @@ -119,34 +105,13 @@ src/isomorphic/classic/class/__tests__/ReactClass-test.js
* should throw with non-object getInitialState() return values
* should work with a null getInitialState() return value
* should throw when using legacy factories

src/isomorphic/classic/class/__tests__/ReactClassMixin-test.js
* should support merging propTypes and statics
* should support chaining delegate functions
* should chain functions regardless of spec property order
* should validate prop types via mixins
* should override mixin prop types with class prop types
* should support mixins with getInitialState()
* should throw with conflicting getInitialState() methods
* should not mutate objects returned by getInitialState()
* should support statics in mixins
* should throw if mixins override each others' statics
* should throw if mixins override functions in statics
* should warn if the mixin is undefined
* should warn if the mixin is null
* should warn if an undefined mixin is included in another mixin
* should warn if a null mixin is included in another mixin
* should throw if the mixin is a React component
* should throw if the mixin is a React component class
* should have bound the mixin methods to the component
* should include the mixin keys in even if their values are falsy
* should work with a null getInitialState return value and a mixin
* replaceState and callback works
* isMounted works

src/isomorphic/classic/element/__tests__/ReactElement-test.js
* uses the fallback value when in an environment without Symbol
* returns a complete element according to spec
* should warn when `key` is being accessed on createClass element
* should warn when `key` is being accessed on ES class element
* should warn when `key` is being accessed on composite element
* should warn when `key` is being accessed on a host element
* should warn when `ref` is being accessed
* allows a string to be passed as the type
Expand All @@ -165,7 +130,6 @@ src/isomorphic/classic/element/__tests__/ReactElement-test.js
* merges rest arguments onto the children prop in an array
* allows static methods to be called using the type property
* identifies valid elements
* allows the use of PropTypes validators in statics
* is indistinguishable from a plain object
* should use default prop value when removing a prop
* should normalize props with default values
Expand Down Expand Up @@ -495,8 +459,6 @@ src/renderers/__tests__/ReactCompositeComponent-test.js
* should react to state changes from callbacks
* should rewire refs when rendering to different child types
* should not cache old DOM nodes when switching constructors
* should auto bind methods and values correctly
* should not pass this to getDefaultProps
* should use default values for undefined props
* should not mutate passed-in props object
* should warn about `forceUpdate` on unmounted components
Expand Down Expand Up @@ -718,7 +680,6 @@ src/renderers/__tests__/ReactUpdates-test.js
* does not call render after a component as been deleted
* marks top-level updates
* throws in setState if the update callback is not a function
* throws in replaceState if the update callback is not a function
* throws in forceUpdate if the update callback is not a function
* does not update one component twice in a batch (#2410)
* does not update one component twice in a batch (#6371)
Expand Down Expand Up @@ -1299,7 +1260,6 @@ src/renderers/dom/shared/__tests__/ReactServerRendering-test.js
* allows setState in componentWillMount without using DOM
* renders components with different batching strategies
* warns with a no-op when an async setState is triggered
* warns with a no-op when an async replaceState is triggered
* warns with a no-op when an async forceUpdate is triggered
* should warn when children are mutated during render

Expand Down
48 changes: 31 additions & 17 deletions src/isomorphic/React.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,21 @@

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

var onlyChild = require('onlyChild');
var warning = require('fbjs/lib/warning');
var checkPropTypes = require('checkPropTypes');

var createElement = ReactElement.createElement;
var createFactory = ReactElement.createFactory;
var cloneElement = ReactElement.cloneElement;

if (__DEV__) {
var warning = require('fbjs/lib/warning');
var canDefineProperty = require('canDefineProperty');
var ReactElementValidator = require('ReactElementValidator');
createElement = ReactElementValidator.createElement;
createFactory = ReactElementValidator.createFactory;
Expand All @@ -38,20 +38,6 @@ var createMixin = function(mixin) {
return mixin;
};

if (__DEV__) {
var warnedForCreateMixin = false;

createMixin = function(mixin) {
warning(
warnedForCreateMixin,
'React.createMixin is deprecated and should not be used. You ' +
'can use this mixin directly instead.',
);
warnedForCreateMixin = true;
return mixin;
};
}

var React = {
// Modern

Expand All @@ -75,7 +61,6 @@ var React = {
// Classic

PropTypes: ReactPropTypes,
createClass: ReactClass.createClass,
createFactory: createFactory,
createMixin: createMixin,

Expand All @@ -96,6 +81,35 @@ if (__DEV__) {
ReactComponentTreeHook: require('ReactComponentTreeHook'),
ReactDebugCurrentFrame: require('ReactDebugCurrentFrame'),
});

let warnedForCreateMixin = false;
let warnedForCreateClass = false;

React.createMixin = function(mixin) {
warning(
warnedForCreateMixin,
'React.createMixin is deprecated and should not be used. You ' +
'can use this mixin directly instead.',
);
warnedForCreateMixin = true;
return mixin;
};

if (canDefineProperty) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Curious why not move canDefineProperty and warning require statements into this __DEV__ block so you can get rid of this check? They're only used here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't the check there to see if you can use Object.defineProperty though?

Copy link
Contributor

Choose a reason for hiding this comment

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

😆 Yes, ignore me.

Object.defineProperty(React, 'createClass', {
get: function() {
warning(
warnedForCreateClass,
Copy link
Collaborator

Choose a reason for hiding this comment

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

What do you think about waiting for first call and including the class displayName in the message?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We can do that in 15.5 (I have a separate branch that I haven't pushed to yet) but in 16 React.createClass is undefined. We could use a void function and warn in there but that would lead to confusing errors in prod.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ahh okay.

'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.',
);
warnedForCreateClass = true;
return undefined;
},
});
}
}

module.exports = React;
14 changes: 14 additions & 0 deletions src/isomorphic/__tests__/React-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,18 @@ describe('React', () => {
'React.createMixin is deprecated and should not be used',
);
});

it('should warn once when attempting to access React.createClass', () => {
spyOn(console, 'error');
let createClass = React.createClass;
createClass = React.createClass;
expect(createClass).toBe(undefined);
expectDev(console.error.calls.count()).toBe(1);
expectDev(console.error.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, ' +
'drop-in replacement.',
);
});
});
Loading