Skip to content

Conversation

@edvinerikson
Copy link
Contributor

@edvinerikson edvinerikson commented Jan 25, 2017

For future reference, this fixes mistakes made in #8857 and #8858.

@edvinerikson
Copy link
Contributor Author

I can't really understand why it fails

@gaearon
Copy link
Collaborator

gaearon commented Jan 25, 2017

The CI shows that a previously passing test now fails in Fiber mode:

--- a/scripts/fiber/tests-passing-except-dev.txt
+++ b/scripts/fiber/tests-passing-except-dev.txt
@@ -116,3 +116,4 @@ src/renderers/shared/shared/__tests__/ReactMultiChild-test.js
 
 src/renderers/shared/shared/__tests__/ReactStatelessComponent-test.js
 * should warn for childContextTypes on a functional component
+* deduplicates ref warnings based on element or owner
diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt
index 050dd41..189f341 100644
--- a/scripts/fiber/tests-passing.txt
+++ b/scripts/fiber/tests-passing.txt
@@ -1560,7 +1560,6 @@ src/renderers/shared/shared/__tests__/ReactStatelessComponent-test.js
 * should throw on string refs in pure functions
 * should warn when given a string ref
 * should warn when given a function ref
-* deduplicates ref warnings based on element or owner
 * should provide a null ref

Read how to run tests in Fiber mode: #7925 (comment).

@gaearon
Copy link
Collaborator

gaearon commented Jan 25, 2017

It's likely failing because two different classes are Unknown in that test, and it gets deduplicated.

@gaearon
Copy link
Collaborator

gaearon commented Jan 25, 2017

Okay, so it turned out we have a bunch of call sites that explicitly check for != null result and have special logic when the name is not available (e.g. don’t print an extra useless message like Check the render method of Unknown component).

So null was useful after all.

@edvinerikson
Copy link
Contributor Author

Yeah I noticed that as well. I will revert to null

@gaearon
Copy link
Collaborator

gaearon commented Jan 25, 2017

Sorry for the churn 😅

@edvinerikson
Copy link
Contributor Author

No worries :D

@gaearon
Copy link
Collaborator

gaearon commented Jan 25, 2017

I don't think this will pass Flow, would it? It’s effectively ?string now.

@edvinerikson
Copy link
Contributor Author

oops.. missed that one..

@edvinerikson
Copy link
Contributor Author

success 🎉

@gaearon gaearon changed the title Component -> Unknown Fix Flow issue and revert to showing "Unknown" in warnings for anonymous components Jan 25, 2017
@gaearon gaearon merged commit 0c7f21a into facebook:master Jan 25, 2017
@edvinerikson edvinerikson deleted the rename-component branch January 25, 2017 14:51
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.

3 participants