Skip to content

Commit

Permalink
Deprecate React.createFactory
Browse files Browse the repository at this point in the history
  • Loading branch information
koba04 committed Feb 10, 2017
1 parent 2e095b4 commit 056ccca
Show file tree
Hide file tree
Showing 7 changed files with 65 additions and 37 deletions.
1 change: 1 addition & 0 deletions scripts/fiber/tests-passing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ src/isomorphic/classic/class/__tests__/ReactClassMixin-test.js
* should work with a null getInitialState return value and a mixin

src/isomorphic/classic/element/__tests__/ReactElement-test.js
* should warn when `createFactory` is used
* 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
Expand Down
4 changes: 2 additions & 2 deletions src/isomorphic/classic/element/ReactDOMFactories.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ var ReactElement = require('ReactElement');
*
* @private
*/
var createDOMFactory = ReactElement.createFactory;
var createDOMFactory = type => ReactElement.createElement.bind(null, type);
if (__DEV__) {
var ReactElementValidator = require('ReactElementValidator');
createDOMFactory = ReactElementValidator.createFactory;
createDOMFactory = type => ReactElementValidator.createElement.bind(null, type);
}

/**
Expand Down
11 changes: 11 additions & 0 deletions src/isomorphic/classic/element/ReactElementValidator.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ var getComponentName = require('getComponentName');
var getIteratorFn = require('getIteratorFn');
var warning = require('warning');

var didWarncreateFactory = false;

function getDeclarationErrorAddendum() {
if (ReactCurrentOwner.current) {
var name = getComponentName(ReactCurrentOwner.current);
Expand Down Expand Up @@ -273,6 +275,15 @@ var ReactElementValidator = {
validatedFactory.type = type;

if (__DEV__) {
if (!didWarncreateFactory) {
warning(
false,
'React.createFactory is deprecated: ' +
'You can create own createFactory, which is a one-liner. ' +
'`var createFacotry = (type) => ReactElement.createElement.bind(null, type)`'
);
didWarncreateFactory = true;
}
if (canDefineProperty) {
Object.defineProperty(
validatedFactory,
Expand Down
53 changes: 34 additions & 19 deletions src/isomorphic/classic/element/__tests__/ReactElement-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ var ReactDOM;
var ReactTestUtils;
var ReactDOMFeatureFlags;


describe('ReactElement', () => {
var ComponentClass;
var originalSymbol;
Expand Down Expand Up @@ -45,12 +46,27 @@ describe('ReactElement', () => {
global.Symbol = originalSymbol;
});

it('should warn when `createFactory` is used', () => {
spyOn(console, 'error');
expectDev(console.error.calls.count()).toBe(0);
React.createFactory('div');
expectDev(console.error.calls.count()).toBe(1);
expectDev(console.error.calls.argsFor(0)[0]).toContain(
'React.createFactory is deprecated: ' +
'You can create own createFactory, which is a one-liner. ' +
'`var createFacotry = (type) => ReactElement.createElement.bind(null, type)`'
);

React.createFactory('div');
expectDev(console.error.calls.count()).toBe(1);
});

it('uses the fallback value when in an environment without Symbol', () => {
expect(<div />.$$typeof).toBe(0xeac7);
});

it('returns a complete element according to spec', () => {
var element = React.createFactory(ComponentClass)();
var element = React.createElement(ComponentClass);
expect(element.type).toBe(ComponentClass);
expect(element.key).toBe(null);
expect(element.ref).toBe(null);
Expand Down Expand Up @@ -162,7 +178,7 @@ describe('ReactElement', () => {
});

it('allows a string to be passed as the type', () => {
var element = React.createFactory('div')();
var element = React.createElement('div');
expect(element.type).toBe('div');
expect(element.key).toBe(null);
expect(element.ref).toBe(null);
Expand All @@ -172,26 +188,26 @@ describe('ReactElement', () => {
});

it('returns an immutable element', () => {
var element = React.createFactory(ComponentClass)();
var element = React.createElement(ComponentClass);
expect(() => element.type = 'div').toThrow();
});

it('does not reuse the original config object', () => {
var config = {foo: 1};
var element = React.createFactory(ComponentClass)(config);
var element = React.createElement(ComponentClass, config);
expect(element.props.foo).toBe(1);
config.foo = 2;
expect(element.props.foo).toBe(1);
});

it('does not fail if config has no prototype', () => {
var config = Object.create(null, {foo: {value: 1, enumerable: true}});
var element = React.createFactory(ComponentClass)(config);
var element = React.createElement(ComponentClass, config);
expect(element.props.foo).toBe(1);
});

it('extracts key and ref from the config', () => {
var element = React.createFactory(ComponentClass)({
var element = React.createElement(ComponentClass, {
key: '12',
ref: '34',
foo: '56',
Expand All @@ -205,7 +221,7 @@ describe('ReactElement', () => {
});

it('extracts null key and ref', () => {
var element = React.createFactory(ComponentClass)({
var element = React.createElement(ComponentClass, {
key: null,
ref: null,
foo: '12',
Expand All @@ -224,7 +240,7 @@ describe('ReactElement', () => {
key: undefined,
ref: undefined,
};
var element = React.createFactory(ComponentClass)(props);
var element = React.createElement(ComponentClass, props);
expect(element.type).toBe(ComponentClass);
expect(element.key).toBe(null);
expect(element.ref).toBe(null);
Expand All @@ -241,7 +257,7 @@ describe('ReactElement', () => {
});

it('coerces the key to a string', () => {
var element = React.createFactory(ComponentClass)({
var element = React.createElement(ComponentClass, {
key: 12,
foo: '56',
});
Expand All @@ -254,12 +270,11 @@ describe('ReactElement', () => {
});

it('preserves the owner on the element', () => {
var Component = React.createFactory(ComponentClass);
var element;

var Wrapper = React.createClass({
render: function() {
element = Component();
element = React.createElement(ComponentClass);
return element;
},
});
Expand All @@ -278,7 +293,7 @@ describe('ReactElement', () => {
it('merges an additional argument onto the children prop', () => {
spyOn(console, 'error');
var a = 1;
var element = React.createFactory(ComponentClass)({
var element = React.createElement(ComponentClass, {
children: 'text',
}, a);
expect(element.props.children).toBe(a);
Expand All @@ -287,7 +302,7 @@ describe('ReactElement', () => {

it('does not override children if no rest args are provided', () => {
spyOn(console, 'error');
var element = React.createFactory(ComponentClass)({
var element = React.createElement(ComponentClass, {
children: 'text',
});
expect(element.props.children).toBe('text');
Expand All @@ -296,7 +311,7 @@ describe('ReactElement', () => {

it('overrides children if null is provided as an argument', () => {
spyOn(console, 'error');
var element = React.createFactory(ComponentClass)({
var element = React.createElement(ComponentClass, {
children: 'text',
}, null);
expect(element.props.children).toBe(null);
Expand All @@ -308,7 +323,7 @@ describe('ReactElement', () => {
var a = 1;
var b = 2;
var c = 3;
var element = React.createFactory(ComponentClass)(null, a, b, c);
var element = React.createElement(ComponentClass, null, a, b, c);
expect(element.props.children).toEqual([1, 2, 3]);
expectDev(console.error.calls.count()).toBe(0);
});
Expand Down Expand Up @@ -529,7 +544,7 @@ describe('ReactElement', () => {

});

describe('comparing jsx vs .createFactory() vs .createElement()', () => {
describe('comparing jsx vs .createElement()', () => {
var Child;

beforeEach(() => {
Expand Down Expand Up @@ -573,6 +588,7 @@ describe('comparing jsx vs .createFactory() vs .createElement()', () => {
describe('when using parent that uses .createFactory()', () => {
var factory, instance;
beforeEach(() => {
spyOn(console, 'error');
var childFactory = React.createFactory(Child);
var Parent = React.createClass({
render: function() {
Expand All @@ -598,15 +614,14 @@ describe('comparing jsx vs .createFactory() vs .createElement()', () => {
});

describe('when using parent that uses .createElement()', () => {
var factory, instance;
var instance;
beforeEach(() => {
var Parent = React.createClass({
render: function() {
return React.DOM.div({}, React.createElement(Child, { ref: 'child', foo: 'foo value' }, 'children value'));
},
});
factory = React.createFactory(Parent);
instance = ReactTestUtils.renderIntoDocument(factory());
instance = ReactTestUtils.renderIntoDocument(React.createElement(Parent));
});

it('should scry children but cannot', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ describe('ReactElementClone', () => {
});

it('should ignore undefined key and ref', () => {
var element = React.createFactory(ComponentClass)({
var element = React.createElement(ComponentClass, {
key: '12',
ref: '34',
foo: '56',
Expand All @@ -343,7 +343,7 @@ describe('ReactElementClone', () => {
});

it('should extract null key and ref', () => {
var element = React.createFactory(ComponentClass)({
var element = React.createElement(ComponentClass, {
key: '12',
ref: '34',
foo: '56',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ describe('ReactElementValidator', () => {

it('warns for keys for arrays of elements in rest args', () => {
spyOn(console, 'error');
var Component = React.createFactory(ComponentClass);
var Component = React.createElement.bind(null, ComponentClass);

Component(null, [Component(), Component()]);

Expand All @@ -52,7 +52,7 @@ describe('ReactElementValidator', () => {

it('warns for keys for arrays of elements with owner info', () => {
spyOn(console, 'error');
var Component = React.createFactory(ComponentClass);
var Component = React.createElement.bind(null, ComponentClass);

var InnerClass = React.createClass({
displayName: 'InnerClass',
Expand All @@ -61,7 +61,7 @@ describe('ReactElementValidator', () => {
},
});

var InnerComponent = React.createFactory(InnerClass);
var InnerComponent = React.createElement.bind(null, InnerClass);

var ComponentWrapper = React.createClass({
displayName: 'ComponentWrapper',
Expand Down Expand Up @@ -185,7 +185,7 @@ describe('ReactElementValidator', () => {

it('warns for keys for iterables of elements in rest args', () => {
spyOn(console, 'error');
var Component = React.createFactory(ComponentClass);
var Component = React.createElement.bind(null, ComponentClass);

var iterable = {
'@@iterator': function() {
Expand All @@ -209,7 +209,7 @@ describe('ReactElementValidator', () => {

it('does not warns for arrays of elements with keys', () => {
spyOn(console, 'error');
var Component = React.createFactory(ComponentClass);
var Component = React.createElement.bind(null, ComponentClass);

Component(null, [Component({key: '#1'}), Component({key: '#2'})]);

Expand All @@ -218,7 +218,7 @@ describe('ReactElementValidator', () => {

it('does not warns for iterable elements with keys', () => {
spyOn(console, 'error');
var Component = React.createFactory(ComponentClass);
var Component = React.createElement.bind(null, ComponentClass);

var iterable = {
'@@iterator': function() {
Expand All @@ -242,7 +242,7 @@ describe('ReactElementValidator', () => {

it('does not warn when the element is directly in rest args', () => {
spyOn(console, 'error');
var Component = React.createFactory(ComponentClass);
var Component = React.createElement.bind(null, ComponentClass);

Component(null, Component(), Component());

Expand All @@ -251,7 +251,7 @@ describe('ReactElementValidator', () => {

it('does not warn when the array contains a non-element', () => {
spyOn(console, 'error');
var Component = React.createFactory(ComponentClass);
var Component = React.createElement.bind(null, ComponentClass);

Component(null, [{}, {}]);

Expand Down Expand Up @@ -479,14 +479,15 @@ describe('ReactElementValidator', () => {
});
var TestFactory = React.createFactory(TestComponent);
expect(TestFactory.type).toBe(TestComponent);
expectDev(console.error.calls.count()).toBe(1);
expectDev(console.error.calls.argsFor(0)[0]).toBe(
// including a deprecated warning
expectDev(console.error.calls.count()).toBe(2);
expectDev(console.error.calls.argsFor(1)[0]).toBe(
'Warning: Factory.type is deprecated. Access the class directly before ' +
'passing it to createFactory.'
);
// Warn once, not again
expect(TestFactory.type).toBe(TestComponent);
expectDev(console.error.calls.count()).toBe(1);
expectDev(console.error.calls.count()).toBe(2);
});

it('does not warn when using DOM node as children', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,15 @@ describe 'ReactCoffeeScriptClass', ->
container = document.createElement 'div'
attachedListener = null
renderedName = null
div = React.createFactory 'div'
span = React.createFactory 'span'
div = React.createElement.bind null, 'div'
span = React.createElement.bind null, 'span'
class InnerComponent extends React.Component
getName: -> this.props.name
render: ->
attachedListener = this.props.onClick
renderedName = this.props.name
return div className: this.props.name
Inner = React.createFactory InnerComponent
Inner = React.createElement.bind null, InnerComponent

test = (element, expectedTag, expectedClassName) ->
instance = ReactDOM.render(element, container)
Expand Down

0 comments on commit 056ccca

Please sign in to comment.