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

fix: add number verification for children #429

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ipluser
Copy link

@ipluser ipluser commented Mar 31, 2017

e.g.

import { h, create } from 'virtual-dom'

const vtree = h('div', [
  h('p', 1),  // the children is number
  h('p', [1]),  // the children is an array that contains number
])

const $rootNode = create(vtree)
document.body.appendChild($rootNode)

`
  <div>
    <p></p>  // not text '1'
    <p>1</p>
  </div>
`

OMG, why not render the first p's text '1'?

next, see the source code:

function h(tagName, properties, children) {
  var childNodes = [];
  var tag, props, key, namespace;
   
  // children verifycation
  if (!children && isChildren(properties)) {  
    children = properties;
    props = {};
  }
    
  // ...
  
  // add childs
  if (children !== undefined && children !== null) {
    addChild(children, childNodes, tag, props);
  }

  return new VNode(tag, props, childNodes, key, namespace);
}

function isChild(x) {
  return isVNode(x) || isVText(x) || isWidget(x) || isVThunk(x);
}

function isChildren(x) {
  // not number verifycation
  return typeof x === 'string' || isArray(x) || isChild(x);
}

function addChild(c, childNodes, tag, props) {
  if (typeof c === 'string') {
    childNodes.push(new VText(c));
  } else if (typeof c === 'number') {  // turns number into VText
    childNodes.push(new VText(String(c)));
  } else if (isChild(c)) {
    childNodes.push(c);
  } else if (isArray(c)) {
    for (var i = 0; i < c.length; i++) {
        addChild(c[i], childNodes, tag, props);  // recursion array's value
    }
  } // ...
}

I think the h('p', 1) and h('p', [1]) must be the same representation:

  • the children is number
  • the children is an array that contains number

@ipluser ipluser changed the title refactor: add number verification for children fix: add number verification for children Mar 31, 2017
@coveralls
Copy link

coveralls commented Mar 31, 2017

Coverage Status

Coverage remained the same at 97.531% when pulling 889118f on ipluser:master into 947ecf9 on Matt-Esch:master.

@coveralls
Copy link

coveralls commented Mar 31, 2017

Coverage Status

Coverage remained the same at 97.531% when pulling 8fd0560 on ipluser:master into 947ecf9 on Matt-Esch:master.

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.

None yet

2 participants