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

Component attempting to render unexpected value renders <undefined></undefined> #671

Closed
dharFr opened this issue Apr 25, 2017 · 3 comments
Closed

Comments

@dharFr
Copy link
Contributor

dharFr commented Apr 25, 2017

Components attempting to render something that is neither a component, or a null/undefined value, results in an <undefined></undefined> node rendered to the DOM.

e.g. :

  • const Zero = () => (0) => renders <undefined></undefined>
  • const Number = () => (42) => renders <undefined></undefined>
  • const False = () => (false) => renders <undefined></undefined>

Here is a test case to reproduce.

Not sure how it is supposed to behave, but rendering <undefined></undefined> does feel appropriate to me and might lead to unexpected behaviors.

@dharFr
Copy link
Contributor Author

dharFr commented Apr 25, 2017

Just tested out of curiosity, and React seems to throw an error in such situations.

@developit
Copy link
Member

Ah yep, this makes sense. In 7 or 8 (don't remember), we moved type handling into h(), but the root of a component is not passed through h() if it's a literal value. Good call.

Seems like the solution is just to extend this condition a bit:
https://github.com/developit/preact/blob/master/src/vdom/diff.js#L69

Something like:

if (vnode==null || vnode===false || vnode===true) vnode = '';

if (typeof vnode==='string' || typeof vnode==='number') {
  // ...

@dharFr
Copy link
Contributor Author

dharFr commented Apr 25, 2017

So the idea would be to avoid throwing an error by silently ignoring booleans + implicitly converting numbers, right?

Doesn't seem that difficult. I'll try to send a PR.

dharFr added a commit to dharFr/preact that referenced this issue Apr 25, 2017
dharFr added a commit to dharFr/preact that referenced this issue Apr 25, 2017
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

No branches or pull requests

2 participants