Update DOM warning wording and link #10819

gaearon · 2017-09-25T19:49:31Z

I think we should get it in before 16.

It updates the link from the warning to point to the DOM blog post. We can then change it to docs once they're in place. The link that is in the code now points to the old behavior, and we can't change it because we want 15 to still point to it.

It also updates the wording to make it correct. The wording we have now is more about the old behavior (prop being invalid) rather than new one (value itself being invalid).

One final minor change is that I now use Invalid when I talk about values, and Unknown when I talk about names. Not saying it's the greatest scheme ever but IMO it's better than the arbitrary distinction we have now (almost all say Unknown but one says Invalid for no specific reason).

After discussion, I changed it to consistently using Invalid when talking about misspellings, and Unknown in other cases. I thought about using Misspelled but that isn't very fair to class. Maybe we can add a separate message for class later and point people to an HTML -> JSX compiler?

This only changes strings.

nhunzaker · 2017-09-25T19:51:51Z

src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js

-        'Warning: Invalid props `foo`, `baz` on <div> tag. Either remove these ' +
-          'props from the element, or pass a string or number value to keep ' +
-          'them in the DOM. For details, see https://fb.me/react-unknown-prop' +
+        'Warning: Invalid values for props `foo`, `baz` on <div> tag. Either remove ' +


I really like the change from prop to value

nhunzaker · 2017-09-25T19:53:04Z

src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js

@@ -2313,7 +2313,7 @@ describe('ReactDOMComponent', () => {

        expectDev(console.error.calls.count()).toBe(1);
        expectDev(console.error.calls.argsFor(0)[0]).toContain(
-          'Warning: Invalid DOM property `x-height`. Did you mean `xHeight`',
+          'Warning: Unknown DOM property `x-height`. Did you mean `xHeight`',
        );
      });



Invalid is my fault. I changed it to Invalid because we do know the attribute, and we know that it is being used incorrectly. Having said that, I have no real preference.

This seems okay to me but shouldn't we do this consistently? We don't use Invalid for event handlers.

Yeah. I'd even throw out that something like Unsupported event listener is more appropriate. I don't think the warning is made better by the inconsistency, let's just go with unknown.

nhunzaker

I could go either way on invalid vs unknown, but I'm good with unknown. I really like the change in phrasing too.

gaearon · 2017-09-25T20:01:26Z

Addressed.

@gaearon

* Update changelog for unreleased 16.0 changes (#10730) * First shot at updating changelog for 16.0 **what is the change?:** Added an 'unreleased' section to the changelog with info from #10294 **why make this change?:** To get things set for the 16.0 release. **test plan:** Visual inspection **issue:** #8854 * Fix typos and formatting errors in changelog * Add requestAnimationFrame and remove "New helpful warnings" **what is the change?:** In response to helpful code review - - Add mention of dependency on `requestAnimationFrame` and need to polyfill that as well as `Map` and `Set` - Remove "New helpful warnings" section; it was incomplete, and there are so many new and updated warnings that it might not be reasonable to cover them in the changelog. **why make this change?:** Accuracy **test plan:** Visual inspection **issue:** issue #8854 * Improve wording * Improve wording and fix missing links * Add backticks to file names & code; wording tweak * Break "Major Changes" into "New Feature" and "Breaking Changes" * Add server side render changes to 16.0 changelog * Change gist link from mine to gaearons * Add note about returning fragments * fix misc nits * Misc. formatting/wording fixes to changelog **what is the change?:** Thanks to the kind code review comments of @gaearon and @nhunzaker we have - removed the non-deterministic bold styling on some bullet points - improved wording of the bullet points for portals, attribute whitelist changes, and server rendering changes - Add note about error boundaries including a breaking change to error handling behavior. - punctuation and capitalization fixes **why make this change?:** Clarity and correctness **test plan:** Visual inspection **issue:** #8854 * fix broken link * Fixes #9667: Updated createTextInstance to create the text node on correct document (#10723) * Record sizes * Add a changelog for elements having the same key (#10811) * Add a changelog for elements having the same key * Reword * Markdown fixs on "DOM Attributes in React 16" post (#10816) * Include tag name into the table snapshot (#10818) * Update DOM warning wording and link (#10819) * Update DOM warning wording and link * Consistently use "Invalid" for known misspellings * Update license headers BSD+Patents -> MIT Did find and replace in TextMate. ``` find: (?:( \*)( ))?Copyright (?:$c$ )?(\d{4})\b.+Facebook[\s\S]+(?:this source tree|the same directory)\.$ replace: $1$2Copyright (c) $3-present, Facebook, Inc.\n$1\n$1$2This source code is licensed under the MIT license found in the\n$1$2LICENSE file in the root directory of this source tree. ``` * Change license and remove references to PATENTS Only remaining references: ``` docs/_posts/2014-10-28-react-v0.12.md 51:You can read the full text of the [LICENSE](https://github.com/facebook/react/blob/master/LICENSE) and [`PATENTS`](https://github.com/facebook/react/blob/master/PATENTS) files on GitHub. docs/_posts/2015-04-17-react-native-v0.4.md 20:* **Patent Grant**: Many of you had concerns and questions around the PATENTS file. We pushed [a new version of the grant](https://code.facebook.com/posts/1639473982937255/updating-our-open-source-patent-grant/). src/__mocks__/vendor/third_party/WebComponents.js 8: * subject to an additional IP rights grant found at http://polymer.github.io/PATENTS.txt ``` * Version bumps to use MIT license * Add ReactTestRenderer documentations (#10692) * Add ReactTestRenderer documentations * Add TestRenderer documentations * TestRenderer is not experiment * Add a link for jsdom * Use ES Modules syntax * Twaek * Add a Link component * Use Jest assertions * Move a documentation for createNodeMock to Idea section * Renamed * Tweak * Rename * More explicit * Add a usecase for createNodeMock * Add changelog for 15.6.2 * Add 15.6.2 blog post to master * Add Nate to authors on master * Bump object-assign patch range to match main package.json * Flow should ignore node_modules/create-react-class * Update error codes * Update CHANGELOG for React 16 * v16.0.0 * Doc updates for React 16 + blog post (#10824) * Doc updates for React 16 + blog post * Add link to Sophie's post * Fix React links on the website (#10837) * Fix React links on the website * Fix code editor * Fix code editor, attempt 2 * Doc change for prevContext removal in CDU (#10836) * Doc change for prevContext removal in CDU Ref: #8631 * Minor rewording * Fix note formatting * React.createPortal is not a function (#10843) * Update Portals Documentation (#10840) * Update Portals Documentation Correct some grammar to be more explicit and clear. Update example CodePen to better match code found in documentation. Update code example styles to match other code examples (ie. 'State and Lifecycle', 'Handling Events'). * Clean up comment to be accurate to example There was a small comment overlooked when reviewing the documentation. This fixes it to be accurate to the example as well as grammatically correct. * Update portals.md * More fixes * Update name of property initializer proposal (#10812) The proposal for property initializers is called [Public Class Fields](https://tc39.github.io/proposal-class-public-fields/) now (part of the combined [Class Fields](https://github.com/tc39/proposal-class-fields) proposal). * Fix portal link (#10845) * Update docs for React 16 (#10846) * Minor doc edit * Rename urls

Update DOM warning wording and link

9d3306f

facebook-github-bot added the CLA Signed label Sep 25, 2017

gaearon requested review from sophiebits and nhunzaker September 25, 2017 19:49

gaearon added this to the 16.0 milestone Sep 25, 2017

nhunzaker reviewed Sep 25, 2017

View reviewed changes

nhunzaker approved these changes Sep 25, 2017

View reviewed changes

Consistently use "Invalid" for known misspellings

4c686ca

aweary approved these changes Sep 25, 2017

View reviewed changes

gaearon merged commit b24d23d into facebook:master Sep 25, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update DOM warning wording and link #10819

Update DOM warning wording and link #10819

gaearon commented Sep 25, 2017 •

edited

Loading

nhunzaker Sep 25, 2017

nhunzaker Sep 25, 2017

gaearon Sep 25, 2017

nhunzaker Sep 25, 2017

nhunzaker left a comment

gaearon commented Sep 25, 2017

Update DOM warning wording and link #10819

Update DOM warning wording and link #10819

Conversation

gaearon commented Sep 25, 2017 • edited Loading

nhunzaker Sep 25, 2017

Choose a reason for hiding this comment

nhunzaker Sep 25, 2017

Choose a reason for hiding this comment

gaearon Sep 25, 2017

Choose a reason for hiding this comment

nhunzaker Sep 25, 2017

Choose a reason for hiding this comment

nhunzaker left a comment

Choose a reason for hiding this comment

gaearon commented Sep 25, 2017

gaearon commented Sep 25, 2017 •

edited

Loading