Skip to content
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

Template strings transpile incorrectly #2033

Closed
ljharb opened this issue Jul 18, 2015 · 3 comments
Closed

Template strings transpile incorrectly #2033

ljharb opened this issue Jul 18, 2015 · 3 comments
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Spec Compliance 👓 A type of pull request used for our changelog categories

Comments

@ljharb
Copy link
Member

ljharb commented Jul 18, 2015

The following:

`${foo}`

transpiles to:

"" + foo;

However, concatenating to an empty string actually calls valueOf, not toString, and as such is not correct. See the following JS:

var x = { toString: function () { return NaN; }, valueOf: function () { return Infinity; }};
[x.toString(), x + '', String(x)]
/* note that only the third item is both a string, and the toString value */

It should transpile to:

String(foo)
@sebmck
Copy link
Contributor

sebmck commented Jul 18, 2015

Duplicate of #1065 and also see the optional es6.spec.templateLiterals transformer.

@sebmck sebmck closed this as completed Jul 18, 2015
@sebmck sebmck added duplicate PR: Spec Compliance 👓 A type of pull request used for our changelog categories labels Jul 18, 2015
@ljharb
Copy link
Member Author

ljharb commented Jul 18, 2015

Ah, thanks, i didn't see the other issue. I'm glad the optional transformer is there, but imo that should be default, since the default case is not going to be using template strings in a tight loop - I'd rather see incorrectness+performance be opt-in than the way it is right now.

@ljharb
Copy link
Member Author

ljharb commented Jul 18, 2015

In other words, correctness is more important than performance, and for the few who care, they'll be able to figure out how to turn on any relevant options to increase performance at the cost of known spec violations.

@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Jul 13, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Jul 13, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Spec Compliance 👓 A type of pull request used for our changelog categories
Projects
None yet
Development

No branches or pull requests

2 participants