Improve deprecation warnings by more info and links #9768

flarnie · 2017-05-24T19:18:46Z

Not sure who to tag for review, but this and adding the warnings for React.createClass should be the last two things blocking the 15.6RC. So anyone who wants to unblock by reviewing feel free~ 🚀

what is the change?:
Updates the warnings for -

React.createMixin (was never implemented!)
React.PropTypes
React.DOM.*

I think we never added the warning for React.createClass to the 15.5 line, unless I'm missing it, so
a follow-up PR will add that, with a link to docs etc.

Does not update the older warnings for -

Factory.type
'isMounted' and 'replaceState'
ReactPerf

We could do a second pass if we want to improve those three warnings, but for now I don't think they are as hi-pri.

Still TODO:

Do an initial release of the react-addons-dom-factories package on npm, making it 1.0.
Improve the docs for react-addons-dom-factories adding documentation and mention the codemod

why make this change?:

We want to make updating as easy as possible. Warning messages should increase clarity, and in the past they have caused confusion.

test plan:
yarn test and running these messages past some folks who use React.

issue:
#9398

**what is the change?:** Updates the warnings for - - `React.createMixin` (was never implemented!) - `React.PropTypes` - `React.createClass` - `React.DOM.*` We never added the warning for `React.createClass` to the 15.5 line, so a follow-up PR will add that, with a link to docs etc. Does *not* update the older warnings for - - Factory.type - 'isMounted' and 'replaceState' - ReactPerf We could do a second pass if we want to improve those three warnings, but for now I don't think they are as hi-pri. Still TODO: - Do an initial release of the [`react-addons-dom-factories`](https://github.com/facebook/react/tree/master/packages/react-dom-factories#react-addons-dom-factories) package on npm, making it 1.0. - Improve the docs for `react-addons-dom-factories` adding documentation and mention the [codemod](https://github.com/reactjs/react-codemod/blob/master/transforms/React-DOM-to-react-dom-factories.js) **why make this change?:** - We want to make updating as easy as possible. Warning messages should increase clarity, and in the past they have caused confusion. **test plan:** `yarn test` and running these messages past some folks who use React. **issue:** facebook#9398

sophiebits · 2017-05-24T23:24:37Z

src/isomorphic/React.js

-        'can use this mixin directly instead.',
+      'In React 16.0, React.createMixin is deprecated and should not be used. ' +
+        'You can use this mixin directly instead.' +
+        ' See (https://fb.me/createmixin-was-never-implemented) for more info.',


intentional that this is in parens?

Yea, but looking at it again I don't think we need the parens. Will fix, thanks for pointing it out. :)

gaearon · 2017-05-24T23:28:00Z

src/isomorphic/React.js

@@ -109,8 +110,11 @@ if (__DEV__) {
      get() {
        lowPriorityWarning(
          didWarnPropTypesDeprecated,
-          'Accessing PropTypes via the main React package is deprecated. Use ' +
-            'the prop-types package from npm instead.',
+          'In React v16.0+, accessing PropTypes via the main React package is deprecated. Use ' +


Isn't this implying it got deprecated in 16 but will be removed later? Usually we say something is deprecated in the current cycle and removed (or moved) in the next cycle. Otherwise it seems like we have not deprecated it yet (but we did).

We could rephrase "In React 16.0+, accessing PropTypes from React package will no longer work. [...]" Then in my opinion it's clearer what will happen and when.

ahhhh yes good catch. sorry I missed that. should say like:

Accessing PropTypes via the main React package is deprecated. In React v16.0, it will be removed completely. Use the …

edit: or dan's wording

Sounds good! Will fix.

**what is the change?:** We rephrased the deprecation messages to clarify that - these APIs are currently deprecated - they will be removed in React v16.0 The previous wording implied that they would be deprecated in v16.0. **why make this change?:** To make the messages easier to understand. **test plan:** Visual inspection

flarnie · 2017-05-25T14:23:09Z

Note to self - cherry-pick this on to master.

gaearon · 2017-05-25T14:46:09Z

src/isomorphic/React.js

-            'and will be removed in the future. Use the ' +
-            'react-addons-dom-factories package instead.',
+            'and will be removed in v16.0+. Use the ' +
+            'react-addons-dom-factories package instead. ' +


I don't think this matches the package name.

https://github.com/facebook/react/tree/master/packages/react-dom-factories

We should probably avoid calling it an "addon" since it was never exposed on React.addons.

Thanks - going to do an overall clean-up of that. The README in that package has the wrong name, and probably other places too.

Please update the gist too when you do it (since it also references the package name).

You mean the fb.me url and the README.md which it points to? Yes.

* Improve deprecation warnings by more info and links **what is the change?:** Updates the warnings for - - `React.createMixin` (was never implemented!) - `React.PropTypes` - `React.createClass` - `React.DOM.*` We never added the warning for `React.createClass` to the 15.5 line, so a follow-up PR will add that, with a link to docs etc. Does *not* update the older warnings for - - Factory.type - 'isMounted' and 'replaceState' - ReactPerf We could do a second pass if we want to improve those three warnings, but for now I don't think they are as hi-pri. Still TODO: - Do an initial release of the [`react-addons-dom-factories`](https://github.com/facebook/react/tree/master/packages/react-dom-factories#react-addons-dom-factories) package on npm, making it 1.0. - Improve the docs for `react-addons-dom-factories` adding documentation and mention the [codemod](https://github.com/reactjs/react-codemod/blob/master/transforms/React-DOM-to-react-dom-factories.js) **why make this change?:** - We want to make updating as easy as possible. Warning messages should increase clarity, and in the past they have caused confusion. **test plan:** `yarn test` and running these messages past some folks who use React. **issue:** facebook#9398 * Rephrase deprecation messages for clarity **what is the change?:** We rephrased the deprecation messages to clarify that - these APIs are currently deprecated - they will be removed in React v16.0 The previous wording implied that they would be deprecated in v16.0. **why make this change?:** To make the messages easier to understand. **test plan:** Visual inspection

flarnie added this to the 15.6 milestone May 24, 2017

flarnie requested review from sophiebits, nhunzaker, gaearon and acdlite May 24, 2017 19:18

facebook-github-bot added the CLA Signed label May 24, 2017

sophiebits approved these changes May 24, 2017

View reviewed changes

gaearon reviewed May 24, 2017

View reviewed changes

flarnie force-pushed the improveDeprecationWarningsAndAddLink branch from 1aa0bad to 404772e Compare May 25, 2017 14:20

flarnie merged commit 3f62cd5 into facebook:15.6-dev May 25, 2017

gaearon reviewed May 25, 2017

View reviewed changes

flarnie mentioned this pull request May 25, 2017

React 15.6 Umbrella #9398

Closed

49 tasks

flarnie deleted the improveDeprecationWarningsAndAddLink branch May 25, 2018 17:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve deprecation warnings by more info and links #9768

Improve deprecation warnings by more info and links #9768

flarnie commented May 24, 2017

sophiebits May 24, 2017

flarnie May 25, 2017

gaearon May 24, 2017

gaearon May 24, 2017

sophiebits May 24, 2017 •

edited

Loading

flarnie May 25, 2017

flarnie commented May 25, 2017

gaearon May 25, 2017

flarnie May 25, 2017

gaearon May 25, 2017

flarnie May 25, 2017

Improve deprecation warnings by more info and links #9768

Improve deprecation warnings by more info and links #9768

Conversation

flarnie commented May 24, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sophiebits May 24, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

flarnie commented May 25, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sophiebits May 24, 2017 •

edited

Loading