WebReflection / domtagger

The hyperHTML's template literal parser
ISC License
43 stars 3 forks source link

Dynamic SVG path breaks render in Edge #18

Closed WebReflection closed 5 years ago

WebReflection commented 5 years ago

This bug https://github.com/WebReflection/lighterhtml/issues/65 belongs to this repository.

vdzk commented 5 years ago

I managed to create a workaround that fixes render in Edge and avoids errors in Chrome and Safari.

const {render, svg} = lighterhtml
const d = 'M150 0 L75 200 L225 200 Z'
const path = svg([`<path d="${d}" />`])
render(document.body, () => svg`
  <svg height="210" width="400">
    ${path}
  </svg>
`)

Please note double quotes around ${d} which are necessary but not sufficient for Edge. Does this hack have any side effects beside the fact that the path node is recreated at every render?

WebReflection commented 5 years ago

Does this hack have any side effects?

Yes.

The library uses template literals features detection to ensure transpilers (TypeScript and old Babel) and broken browsers don't end up with the slow path because of badly, broken, transpilation or quirk old browsers implementation.

That library itself would make your whole code bail out of standard fast lane for every template literal transform.

As summary, you are better off with this workaround instead, but the node will be trashed, and re-created, each time, making performance on each update the worst possible for the library (which is not too bad, yet not optimal)

const {render, svg} = lighterhtml;
const d = 'M150 0 L75 200 L225 200 Z';
const path = Object`<path d="${d}"/>`;
render(document.body, () => svg`
  <svg height="210" width="400">${svg(path, d)}</svg>
`);

In this case the Object will simply return the template literal as it is, and you can explicitly pass holes to the svg(template, ...values) function, so that you won't ever trigger the bail out from standard code/transpilers.

P.S. best of all, in that case you'll never have a new <path> node each time, because the template literal will always be the same.

WebReflection commented 5 years ago

Actually ... that recreates exact scenario 😅 ... so ... as proper workaround:

const {render, svg} = lighterhtml;
const wm = new WeakMap;

const d = 'M150 0 L75 200 L225 200 Z';
const fake = Object`<path d=""/>`;
const path = wm.get(fake) || wm.set(fake, svg`<path d=""/>`.firstChild).get(fake);
path.setAttribute('d', d);
render(document.body, () => svg`
  <svg height="210" width="400">${path}</svg>
`);

but I suggest you waiting for me to fix the issue in Edge

WebReflection commented 5 years ago

OK, I've found the culprit of the issue, and I've described everything in here: https://github.com/WebReflection/lighterhtml/issues/65#issuecomment-534640281

I need to update every project I have based on domtagger, as this is something domtagger cannot solve.

I hope the proposed solution works good enough for your needs.