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

Does not handle children that are numbers #31

Open
SilentCicero opened this issue Mar 23, 2017 · 5 comments
Open

Does not handle children that are numbers #31

SilentCicero opened this issue Mar 23, 2017 · 5 comments
Labels

Comments

@SilentCicero
Copy link

Does not nicely handle vnode instances which are numbers, only strings. Maybe if type number => render to string.

@wavesoft
Copy link
Owner

wavesoft commented Mar 26, 2017

Ouch, that's indeed a tricky one. I will try to find a good solution for this, but right now it's not easy, since I am relying on string's properties to identify it.

@wavesoft wavesoft added the bug label Mar 26, 2017
@jhnns
Copy link

jhnns commented Dec 20, 2018

Imo that's not a big limitation. I think it's fair to assume that numbers need to be casted to strings by components. The same problem also exists for null and false which are valid render values in React. Both the null and the false case can be expressed as "" which I think is fine.

@mindplay-dk
Copy link

mindplay-dk commented Oct 6, 2019

For the record, we're talking about these lines here, right?

I'm wondering, can we simply reverse the test and the condition?

So instead of vnode.replace, how about e.g. !!vnode.a?

This should match anything that's not a vnode object, including strings, numbers and booleans, right?

(if that works, also maybe swap the terminals of the ternary expression and save one of the ! operators.)

wavesoft added a commit that referenced this issue Oct 21, 2019
wavesoft added a commit that referenced this issue Oct 27, 2019
wavesoft added a commit that referenced this issue Oct 27, 2019
wavesoft added a commit that referenced this issue Oct 28, 2019
wavesoft added a commit that referenced this issue Oct 30, 2019
@mindplay-dk
Copy link

I think we can close this?

@mindplay-dk
Copy link

...although it's still on a devel branch that might should be merged back to master?

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

No branches or pull requests

4 participants