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

Deprecate React.createMixin #8853

Merged
merged 4 commits into from Jan 24, 2017
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions scripts/fiber/tests-passing.txt
Expand Up @@ -108,6 +108,10 @@ src/addons/transitions/__tests__/ReactTransitionGroup-test.js
* should handle entering/leaving several elements at once
* should warn for duplicated keys

src/isomorphic/__tests__/React-test.js
* should log a deprecation warning once when using React.__spread
* should log a deprecation warning once when using React.createMixin

src/isomorphic/children/__tests__/ReactChildren-test.js
* should support identity for simple
* should treat single arrayless child as being in array
Expand Down
26 changes: 19 additions & 7 deletions src/isomorphic/React.js
Expand Up @@ -35,20 +35,35 @@ if (__DEV__) {
}

var __spread = Object.assign;
var createMixin = function(mixin) {
return mixin;
};

if (__DEV__) {
var warned = false;
var warnedForSpread = false;
var warnedForCreateMixin = false;
__spread = function() {
warning(
warned,
warnedForSpread,
'React.__spread is deprecated and should not be used. Use ' +
'Object.assign directly or another helper function with similar ' +
'semantics. You may be seeing this warning due to your compiler. ' +
'See https://fb.me/react-spread-deprecation for more details.'
);
warned = true;
warnedForSpread = true;
return Object.assign.apply(null, arguments);
};

createMixin = function(mixin) {
warning(
warnedForCreateMixin,
'React.createMixin is deprecated and should not be used. You ' +
'can use your mixin directly instead.'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I considered adding some messaging here about mixins being considered harmful, but I'm not sure if that's overstepping since they're not actually considered deprecated.

Copy link
Collaborator

Choose a reason for hiding this comment

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

"You can use your mixin" sounds a bit aggressive, maybe "You can use this mixin"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed!

);
warnedForCreateMixin = true;
return mixin;
};

}

var React = {
Expand All @@ -75,10 +90,7 @@ var React = {
PropTypes: ReactPropTypes,
createClass: ReactClass.createClass,
createFactory: createFactory,
createMixin: function(mixin) {
// Currently a noop. Will be used to validate and trace mixins.
return mixin;
},
createMixin: createMixin,

// This looks DOM specific but these are actually isomorphic helpers
// since they are just generating DOM strings.
Expand Down
42 changes: 42 additions & 0 deletions src/isomorphic/__tests__/React-test.js
@@ -0,0 +1,42 @@
/**
* Copyright 2013-present, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*
* @emails react-core
*/

'use strict';

describe('React', () => {

var React;

beforeEach(() => {
React = require('React');
});

it('should log a deprecation warning once when using React.__spread', () => {
spyOn(console, 'error');
React.__spread({});
React.__spread({});
expectDev(console.error.calls.count()).toBe(1);
expectDev(console.error.calls.argsFor(0)[0]).toContain(
'React.__spread is deprecated and should not be used'
);
});

it('should log a deprecation warning once when using React.createMixin', () => {
spyOn(console, 'error');
React.createMixin();
React.createMixin();
expectDev(console.error.calls.count()).toBe(1);
expectDev(console.error.calls.argsFor(0)[0]).toContain(
'React.createMixin is deprecated and should not be used'
);
});

});