-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Template strings are cached incorrectly #27460
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
Comments
The problem here is that we do the right thing in modules, but not in the global scope because we don't want to have potentially conflicting variables. So we could just come up with absurdly long variable names that describe the string contents and work with that. |
Close to 2 years, and still no fixes, but people keep filing issues to my libraries: My workaround is also mostly reliable, but it's based on "signing" each template literal, assuming two templates literals with the same content comes from the same scope/callback. This plays well until you have people doing, for a reason or another, the following: const componentA = content => html`${content}`;
const componentB = content => html`${content}`;
render(where, html`${componentA()}${componentB()}`); All three empty template literals produces the same template signature, so there's no way to reasonably cache and assign a unique identifier to each of them. A better playground has been created: it's based on the following code: function tag(template, ...args) {
set.add(template);
const out = [template[0]];
for (let i = 1, { length } = template; i < length; i++)
out.push(args[i - 1], template[i]);
return out.join('');
}
const set = new Set;
const a = () => tag`${1}`;
const b = () => tag`${2}`;
console.assert(
a() === a() && b() === b(),
'transpilation error'
);
console.assert(
set.size === 2,
`expected 2 unique templates, got ${set.size} instead`
); This issue is easily breaking in the wild otherwise perfectly valid code, and as there's no workaround to solve this in user-land, except telling people to not use TypeScript to transpile, I hope it'll get a higher priority. The current measure I'm considering in here, is to remove any normalization, which will penalize TypeScript users performances as there will be tons of unnecessary DOM trashes in code transpiled in the wild, including CodePens, code in any html page, etc etc. Best Regards. |
This doesn't seem like an important scenario for modern JS development - folks can either target ES6 which is supported basically everywhere, or use an alternate transpiler with stricter adherence semantics if this is important for their scenario. |
@RyanCavanaugh my last comment was from 4 years ago ... sure thing I wouldn't care about this today but thanks for reminding me this never got fixed or received any attention it deserved, in the name of transpiling to Web standards, to date. Yet I agree nobody should care about this anymore these days, thanks! |
TypeScript Version: 3.2.0-dev.20180929
Search Terms:
Code
Expected behavior:
After compiling and running the code in a browser console, the body's content should be "true".
The code works as expected when compiled with Babel, or run verbatim in a browser console.
Actual behavior:
The body's content becomes "false". This breaks libraries such as hyperHTML (note: now it contains a workaround that works for me at least WebReflection/hyperHTML@6d3d879)
WebReflection/hyperHTML#270
Playground Link: https://www.typescriptlang.org/play/#src=var%20invokes%20%3D%20%5B%5D%3B%0D%0Afunction%20test(template%2C%20_)%20%7B%0D%0A%20%20invokes.push(template)%3B%20%0D%0A%7D%0D%0A%0D%0Afunction%20update(value)%20%7B%0D%0A%20%20test%60some%20%24%7Bvalue%7D!%60%3B%0D%0A%7D%0D%0A%0D%0Aupdate(1)%3B%0D%0Aupdate(2)%3B%0D%0A%0D%0Adocument.body.textContent%20%3D%20''%20%2B%20(invokes%5B0%5D%20%3D%3D%3D%20invokes%5B1%5D)%3B%20%0D%0A
Related Issues:
The text was updated successfully, but these errors were encountered: