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

Discussion: pretty-format with React element of type undefined renders Unknown in snapshot #3823

Closed
nathanforce opened this issue Jun 14, 2017 · 6 comments · Fixed by #4360
Closed

Comments

@nathanforce
Copy link

Do you want to request a feature or report a bug?
feature/discussion

What is the current behavior?
When creating a snapshot with an undefined component, the snapshot will use <Unknown>.

(https://github.com/facebook/jest/blob/master/packages/pretty-format/src/plugins/ReactElement.js#L84).

I am wondering if this is an intentional implementation, as it leads to easily missed mistakes when using static subcomponents a la Semantic UI.

As an example, if you were to make a typo as below (Message.Haeder), your snapshot would still render and the mistake could be easily missed.

<Message icon>
  <Icon name='circle notched' loading />
  <Message.Content>
    <Message.Haeder>
      Just one second
    </Message.Haeder>
    We're fetching that content for you.
  </Message.Content>
</Message>

What is the expected behavior?

Would it be better to error when trying to render a React element of type undefined or is that outside the scope of pretty-format?

@cpojer
Copy link
Member

cpojer commented Aug 24, 2017

We'll keep using this for now.

@cpojer cpojer closed this as completed Aug 24, 2017
@nathanforce
Copy link
Author

@cpojer Could you provide the reasoning, just for my curiosity?

@cpojer
Copy link
Member

cpojer commented Aug 24, 2017

Components can be nameless, and we fill in an unknown name, which is similar to what React also does.

@nathanforce
Copy link
Author

I understand that for valid, renderable components, but can components be undefined? Seems you could get the best of both worlds by adding an explicit check for undefined/invalid nodes.

@cpojer
Copy link
Member

cpojer commented Aug 24, 2017

cc @pedrottimark I'll defer to you.

@pedrottimark
Copy link
Contributor

@nathanforce Thank you for your patience until we could think how to sort it out in #4360

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

Successfully merging a pull request may close this issue.

4 participants