-
Notifications
You must be signed in to change notification settings - Fork 46.8k
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
[Fiber] Add unit tests for ReactDOMFiber #8016
[Fiber] Add unit tests for ReactDOMFiber #8016
Conversation
|
||
beforeEach(() => { | ||
// to supress a warning that ReactDOMFiber is an experimental renderer. | ||
spyOn(console, 'error'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has an unfortunate downside of hiding all (even unintentional) warnings.
An alternative to this would be to import ReactDOM
but wrap the whole test suite into if (ReactDOMFeatureFlags.useFiber)
. I would probably prefer that with existing test infrastructure.
Alternatively we can suppress the warning when process.env.NODE_ENV === 'test'
.
@sebmarkbage You have an opinion on this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea of just importing ReactDOM
and wrapping it all in a if (ReactDOMFeatureFlags.useFiber)
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gaearon @sebmarkbage Thanks! I've updated this with using ReactDOMFeatureFlags.useFiber
.
20967c3
to
3c4aac4
Compare
}); | ||
|
||
it('should render a component returning strings directly from render', () => { | ||
if (ReactDOMFeatureFlags.useFiber) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add this if
outside the it
? That way this test isn't counted into the total.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sebmarkbage Thanks! I've updated it. But In case of ReactDOMFeatureFlags.useFiber
is false, this test is failed because of Your test suite must contain at least one test.
.
So I added some tests that can pass regardless the fiber flag.
Does this make sense?
3c4aac4
to
8686573
Compare
That makes sense! Thanks. |
Thanks! |
This is an another PR of #8001, which adds unit tests for
ReactTopLevelText
inReactDOMFiber
.#8001 (comment)