dy / xhtm

XHTM − eXtended Hyperscript Tagged Markup
MIT License
25 stars 2 forks source link

Additional comma added by xhtm #9

Closed aral closed 1 year ago

aral commented 1 year ago

xhtm bound to vhtml adds an additional comma during string interpolation within attribute values.

To reproduce

import htm from 'htm'
import xhtm from 'xhtm'
import vhtml from 'vhtml'

const htmlHtm = htm.bind(vhtml)
const htmlXhtm = xhtm.bind(vhtml)

const titles = ['one', 'two', 'three']

const ComponentHtm = ({title, index}) => htmlHtm`
  <li src='https://my.site/section/${index+1}'>
    Section ${index+1}: ${title}
  </li>
`
const ComponentXhtm = ({title, index}) => htmlXhtm`
  <li src='https://my.site/section/${index+1}'>
    Section ${index+1}: ${title}
  </li>
`

const htmlFromHtm = htmlHtm`
  <ul>
    ${titles.map((title, index) => htmlHtm`<${ComponentHtm} title=${title} index=${index} />`)}
  </ul>

`
const htmlFromXhtm = htmlXhtm`
  <ul>
    ${titles.map((title, index) => htmlXhtm`<${ComponentXhtm} title=${title} index=${index} />`)}
  </ul>
`

console.info('HTML from htm\n\n', htmlFromHtm)
console.info('\nHTML from xhtm\n\n', htmlFromXhtm)

Expected output

Both results should match:

 <ul><li src="https://my.site/section/1">Section 1: one</li><li src="https://my.site/section/2">Section 2: two</li><li src="https://my.site/section/3">Section 3: three</li></ul>

Actual output

HTML from htm

 <ul><li src="https://my.site/section/1">Section 1: one</li><li src="https://my.site/section/2">Section 2: two</li><li src="https://my.site/section/3">Section 3: three</li></ul>

HTML from xhtm

 <ul> <li src="https://my.site/section/,1"> Section 1: one </li><li src="https://my.site/section/,2"> Section 2: two </li><li src="https://my.site/section/,3"> Section 3: three </li> </ul>

Note the commas before the indices in the src attributes.

(Minor: Also note that xhtm is adding some additional spaces inside the the <li> tags and between the <ul> and <li> tags but this is ignored by browsers and does not affect the content viewed in the browser.)

aral commented 1 year ago

So now I’m confused.

I added the following test to test/index.js and it’s passing and it’s now failing as it should:

(I originally – and mistakenly – had resultHtm using componentXhtm instead of componentHtm. Which likely means it’s a difference in the component rendering(?))

import htmOriginal from 'htm'
const htmOriginalBound = htmOriginal.bind(h)

t('component', t => {
    const titles = ['one', 'two', 'three']
    const componentXhtm = ({title, index}) => html`<li src='https://my.site/section/${index+1}'>Section ${index+1}: ${title}</li>`
    const resultXhtm = html`<ul>${titles.map((title, index) => html`<${componentXhtm} title=${title} index=${index} />`)}</ul>`
    const componentHtm = ({title, index}) => htmlmOriginalBound`<li src='https://my.site/section/${index+1}'>Section ${index+1}: ${title}</li>`
    const resultHtm = htmOriginalBound`<ul>${titles.map((title, index) => htmOriginalBound`<${componentHtm} title=${title} index=${index} />`)}</ul>`
    t.deepEqual(resultXhtm, resultHtm, 'The output should be the same')
})

Update: Ah, I’m tired. I spotted a mistake. Going to fix it and see what happens…

Update 2: Fixed the code and now the test fails. I’m about to head out to dinner with Laura. @dy, if you get a chance, let me know if you’d like me to add this test and take a look tomorrow (or happy to let you take a look if you have time/might know what the problem is. It’ll likely take me a little longer but happy to dig in).

dy commented 1 year ago

Fixed the issue - please try xhtm@1.5.5

aral commented 1 year ago

Wow, thank you so much, Dmitry. I can’t wait to try it out tomorrow :)

aral commented 1 year ago

@dy 1.5.5 seems to have broken components. I’m going to take a proper look at it later but I spotted one piece of code at least that seems to have an obvious error in it (unless I’m missing something) – although fixing that doesn’t help with the component issue:

https://github.com/dy/xhtm/commit/1e139e229b2f22a7601f35c970f93bc0481cf10c#r102054115

dy commented 1 year ago

Oh, ok. I need a test case then. I was following existing tests.

aral commented 1 year ago

@dy I’m looking into it now. There are a few places where it seems to have broken the Kitten examples. Will prepare a test case or cases.

The error I’m seeing, by the way, is TypeError: value.join is not a function while running Array.Array.isArray.value.toString in /htm.js (line 66, column 47) when it tries to render a component. With that particular component, it seems to be trying to call join() on a string.

aral commented 1 year ago

I narrowed it down and opened a separate issue: https://github.com/dy/xhtm/issues/10 :)

dy commented 1 year ago

Yes, thanks! Fixed