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

Conversation

aweary
Copy link
Contributor

@aweary aweary commented Jan 24, 2017

One step towards #8852. This also adds tests for the deprecation warning, along with the React.__spread method. I didn't see a good place to stick these tests, so I just created a new __tests__ folder in src/isomorphic.

Brandon Dail added 3 commits January 23, 2017 18:14
This API was never fully implemented. Since mixins are no longer considered part of the future React API, it will be removed.
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.

warning(
warnedForCreateMixin,
'React.createMixin is deprecated and should not be used. You ' +
'can use your mixin directly instead.'
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!

Copy link
Collaborator

@gaearon gaearon left a comment

Choose a reason for hiding this comment

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

Please merge after CI passes.

@aweary aweary added this to the 15-lopri milestone Jan 24, 2017
@aweary aweary merged commit 2be0583 into facebook:master Jan 24, 2017
@aweary aweary deleted the remove-create-mixin branch January 24, 2017 06:18
@sophiebits
Copy link
Collaborator

CI is failing, looks related to this:

https://circleci.com/gh/facebook/react/1082

@gaearon
Copy link
Collaborator

gaearon commented Jan 24, 2017

I don't think so. It seems like this was broken by #8858: we didn't run the tests in Fiber mode and one of the tests seems to assert a different warning message.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants