socketsupply / tonic

A Low Profile Component Framework – Stable, minimal, easy to audit, zero-dependencies and build-tool-free.
http://tonicframework.dev
MIT License
867 stars 24 forks source link

Numbers and booleans aren’t replaced correctly #106

Open gryzzly opened 1 year ago

gryzzly commented 1 year ago

Describe the bug

When passing a boolean or number, I am seeing the variable type in the rendered output, i.e.

1__number
false__boolean

Steps To Reproduce See https://jsfiddle.net/fev3zko2/ with the following code:

<my-component></my-component>
import Tonic from 'https://cdn.jsdelivr.net/npm/@socketsupply/tonic@15.1.1/+esm'

const numberValue = 1;
const booleanValue = true;
const stringValue = 'lorem ipsum';

class MyComponent extends Tonic {
    render() {
    return this.html`<ul>
        <li>Number ${numberValue}</li>
      <li>Boolean ${booleanValue}</li>
      <li>String ${stringValue}</li>
    </ul>`;
  }
}

Tonic.add(MyComponent);

This seems to happen in every browser that I tested with.

gryzzly commented 1 year ago

I’ve tried to look at the source code, but I am not sure how is it supposed to work:

First, in connectedCallback the HTML attributes of the <web-component foo="bar"></web-component> are iterated, and these are collected into this.props. However, the way they are tested expects placeholder values (like 1__float, true__boolean). Why would value of an attribute be a placeholder value at this point? We have just connected the web component, where would a replaced value come from?

    for (const { name: _name, value } of this.attributes) {
      const name = cc(_name)
      const p = this.props[name] = value

      if (/__\w+__\w+__/.test(p)) {
        const { 1: root } = p.split('__')
        this.props[name] = Tonic._data[root][p]
      } else if (/\d+__float/.test(p)) {
        this.props[name] = parseFloat(p, 10)
      } else if (p === 'null__null') {
        this.props[name] = null
      } else if (/\w+__boolean/.test(p)) {
        this.props[name] = p.includes('true')
      } else if (/placehold:\w+:\w+__/.test(p)) {
        const { 1: root } = p.split(':')
        this.props[name] = Tonic._children[root][p][0]
      }
    }

After this, the values passed to html are replaced with placeholders:

html (strings, ...values) {
    const refs = o => {
      if (o && o.__children__) return this._placehold(o)
      if (o && o.isTonicTemplate) return o.rawText
      switch (Object.prototype.toString.call(o)) {
        case '[object HTMLCollection]':
        case '[object NodeList]': return this._placehold([...o])
        case '[object Array]': {
          if (o.every(x => x.isTonicTemplate && !x.unsafe)) {
            return new TonicTemplate(o.join('\n'), null, false)
          }
          return this._prop(o)
        }
        case '[object Object]':
        case '[object Function]':
        case '[object Set]':
        case '[object Map]':
        case '[object WeakMap]': return this._prop(o)
        case '[object NamedNodeMap]':
          return this._prop(Tonic._normalizeAttrs(o))
        case '[object Number]': return `${o}__float`
        case '[object String]': return Tonic.escape(o)
        case '[object Boolean]': return `${o}__boolean`
        case '[object Null]': return `${o}__null`
        case '[object HTMLElement]':
          return this._placehold([o])
      }
      if (
        typeof o === 'object' && o && o.nodeType === 1 &&
        typeof o.cloneNode === 'function'
      ) {
        return this._placehold([o])
      }
      return o
    }

    const out = []
    for (let i = 0; i < strings.length - 1; i++) {
      out.push(strings[i], refs(values[i]))
    }

But there seems to be nothing to replace these placeholders now. They are directly inserted into DOM at this point.

Am I doing something wrong?

heapwolf commented 1 year ago

What you're seeing is a placeholder that is passing the data and its type information (1__number), this is correct. What it sounds like you are trying to do is insert the value into the html instead of pass it as a reference. To insert the value just cast it down to a string, for example ...<p>The numeric value is ${String(myVariable)}</p>.

import Tonic from 'https://cdn.jsdelivr.net/npm/@socketsupply/tonic@15.1.1/+esm'

const numberValue = 1;
const booleanValue = true;
const stringValue = 'lorem ipsum';

class MyComponent extends Tonic {
    render() {
    return this.html`<ul>
        <li>Number ${String(numberValue)}</li>
      <li>Boolean ${String(booleanValue)}</li>
      <li>String ${stringValue}</li>
    </ul>`;
  }
}

Tonic.add(MyComponent);
gryzzly commented 1 year ago

Thank you! I’d be happy to contribute docs if you give me some guidance!

gryzzly commented 1 year ago

Feel free to close 👍

heapwolf commented 1 year ago

Thank you! I’d be happy to contribute docs if you give me some guidance!

I think in the getting started guide there should be a mention that says something like "anything that isn't a string type will be considered a value that you are trying to pass as a reference."