Skip to content
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

Add null rendering test #369

Closed
wants to merge 1 commit into from

Conversation

rstacruz
Copy link
Collaborator

@rstacruz rstacruz commented Jan 9, 2016

Here's a failing test case.

@rstacruz
Copy link
Collaborator Author

rstacruz commented Jan 9, 2016

okaayyy.. i could've sworn that failed on my machine...

@rstacruz
Copy link
Collaborator Author

rstacruz commented Jan 9, 2016

okay, I see now: it seems CI will always pass, even if tests fail (!). Have a look at the CI logs and you should see the failure.

@rstacruz
Copy link
Collaborator Author

rstacruz commented Jan 9, 2016

Relevant: tape-testing/tape-run#27

@anthonyshort
Copy link
Owner

I'm not entirely sure if it's worth adding this feature when a user could just as easily add a <noscript /> or <div /> instead. What do you think?

@rstacruz
Copy link
Collaborator Author

If undefined is handled, then I'd expect null to be too. This is a very common pattern after all:

return <div>
  { someCondition && <Navigation /> }
  { someCondition && <Content /> }
</div>
/* if someCondition is `null`, then we're screwed */

@anthonyshort
Copy link
Owner

These tests were added in another branch last week :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants