[fixed] bug where the label assertion can return a false failure #48

reactjs · Jun 8, 2015 · 8c6a7ce · 8c6a7ce
1 parent bd7cc9a
commit 8c6a7ce
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 11 deletions.
diff --git a/lib/__tests__/index-test.js b/lib/__tests__/index-test.js
@@ -333,7 +333,7 @@ describe('labels', () => {
     });
   });
 
-  it('does not warn when a child is a component with text and and an image without alt', (done) => {
+  it('does not warn when there is an image without alt text with a sibling text node', (done) => {
     var Foo = React.createClass({
       render: function() {
         return (
@@ -349,7 +349,7 @@ describe('labels', () => {
     });
   });
 
-  it('warns when a child is a component without text content', () => {
+  it('warns when a child is a component without text content', (done) => {
     var Bar = React.createClass({
       render: () => {
         return (
@@ -359,7 +359,7 @@ describe('labels', () => {
     });
 
     expectWarning(assertions.render.NO_LABEL.msg, () => {
-      React.render(<div role="button"><Bar/></div>, fixture);
+      React.render(<div role="button"><Bar/></div>, fixture, done);
     });
   });
 
@@ -387,7 +387,7 @@ describe('labels', () => {
     });
   });
 
-  it('warns if no child components have label text', () => {
+  it('warns if no child components have label text', (done) => {
     var Bar = React.createClass({
       render: () => {
         return (
@@ -405,12 +405,12 @@ describe('labels', () => {
     });
 
     expectWarning(assertions.render.NO_LABEL.msg, () => {
-      React.render(<div role="button"><Bar/><Foo/></div>, fixture);
+      React.render(<div role="button"><Bar/><Foo/></div>, fixture, done);
     });
   });
 
 
-  it('does not error when the component has a componentDidMount callback', () => {
+  it('does not error when the component has a componentDidMount callback', (done) => {
     var Bar = React.createClass({
       _privateProp: 'bar',
 
@@ -425,7 +425,7 @@ describe('labels', () => {
     });
 
     expectWarning(assertions.render.NO_LABEL.msg, () => {
-      React.render(<div role="button"><Bar/></div>, fixture);
+      React.render(<div role="button"><Bar/></div>, fixture, done);
     });
   });
 

diff --git a/lib/assertions.js b/lib/assertions.js
@@ -93,6 +93,11 @@ var hasChildTextNode = (props, children, failureCB) => {
       after(child.type.prototype, 'componentDidMount', function() {
         assertLabel(React.findDOMNode(this), context, failureCB);
       });
+
+      // Return true because the label check is now going to be async
+      // (due to the componentDidMount listener) and we want to avoid
+      // pre-maturely calling the failure callback.
+      hasText = true;
     }
   });
   return hasText;
@@ -180,10 +185,12 @@ exports.render = {
 
       var failed = !(
         (isInteractive(tagName, props) || props.role) &&
-        (props['aria-label'] ||
-        props['aria-labelled-by'] ||
-        (tagName === 'img' && props.alt) ||
-        hasChildTextNode(props, children, failureCB))
+        (
+          props['aria-label'] ||
+          props['aria-labelled-by'] ||
+          (tagName === 'img' && props.alt) ||
+          hasChildTextNode(props, children, failureCB)
+        )
       );
 
       if (failed)