ungap / template-literal

A feature detected fix for non unique template literals
ISC License
7 stars 2 forks source link

Avoid TS + Babel fallback #10

Closed WebReflection closed 4 years ago

WebReflection commented 4 years ago

Coming from this issue https://github.com/WebReflection/lighterhtml/issues/92 it'd be great to solve once for all possible TS + Babel transpilation issues.

yuretz commented 4 years ago

Well, it seems I can reproduce the original issue on Firefox too. No transpilation involved.

yuretz commented 4 years ago

After testing/debugging it a little bit more, I no longer believe it's a problem in this ungap module. The problem seems to be with how lighterhtml deals with merging/updating nodes produced by cached template objects that look like html`${something}`, so maybe WebReflection/lighterhtml#92 should be reopened?

WebReflection commented 4 years ago

I need to have a deeper look at your example ... as I don't know what's the expected result, can you please tell me steps to reproduce the issue and, eventually, tell me if there's any browser that doesn't fail?

WebReflection commented 4 years ago

this is extremely odd ... but I wonder why you are using empty templates instead of just:

const one = (name = 'one') => box(() => html`one`);

const two = (name = 'two', value = 'opt') => box(() => html`
  <label class="block px-3 mb-3">
    <input type="radio" class="form-radio" name=${name} value=${value}>
    <span>two</span>
  </label>
`);

I am not saying this is not a bug, but as I've never had such issue, I wonder what's the use case for empty templates in here.

WebReflection commented 4 years ago

P.S. just FYI, with uhtml I have no issue whatsoever ... this is getting even weirder ...

WebReflection commented 4 years ago

Found it.

WebReflection commented 4 years ago

it is this module, precisely the following code:

var foreverCache = function (tl) {
  for (var key = '.', i = 0; i < tl.length; i++)
    key += tl[i].length + '.' + tl[i];
  return forever[key] || (forever[key] = tl);
};

Why is this an issue? two empty templates results into exact same code, hence the dom-differ believes the template is the previous one, and tries to apply changes in that part of the node that was previously removed.

Oh gosh ... I'm so happy I've found it, and also I have no idea how to solve this, as there's no way to understand if a template was coming from a different function.

This is a hard-to-solve problem, and I start thinking if it's even worth it solving it, as opposite of removing completely the userAgent sniffing part.

What a bummer

WebReflection commented 4 years ago

this is the issue in a nutshell with TypeScript targeting ES5 (IE 11) https://bit.ly/2XYvAmY

while this is Babel 7 (correct behavior) https://bit.ly/3dZc5QY

I think I'll get rid of the utility, as it causes more damages than it solves.

This might break though, so it's a major.

yuretz commented 4 years ago

OK, so the mystery is solved. Nice! However I don't understand why both Firefox and Safari were forced to use foreverCache?

To answer your question about how it happened that there were empty templates in my code, well, I had a list-like component that was returning an array of Hole instances and it was violating a generic component type definition that stated that a return type should be Hole or HTMLElement, and instead of updating the type definition I decided to wrap the whole array in html`...`. So, lazyness, I guess 😅

WebReflection commented 4 years ago

It'd be great if you could help improving the definition file, as I don't use TS and I'm not too familiar with it.

Older Firefox and Safari had issues with the uniqueness of template literals ... but as there's no way to fix this via JS, I think I'll stick a big note in every README warning people to use Babel 7 to transpile their code, even just the template literals bit, if they are targeting older browsers, as Babel 7 produces a reliable, specs compliant, behavior, TS targeting ES5 doesn't.