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

Add another test for runtime order of template literals #6092

Merged
merged 4 commits into from Aug 12, 2017

Conversation

Jessidhia
Copy link
Member

@Jessidhia Jessidhia commented Aug 12, 2017

This avoids creating extra .concat calls unless the result could be
potentially observably different from real template evaluation.

The idea is that even if you have multiple objects with @@toPrimitive
in them, they will still be evaluated left-to-right anyway; but any arbitrary
impure code besides a @@toPrimitive implementation would be able
to observe that a previous object's @@toPrimitive was not yet evaluated.

Turns out that you don't even need fancy things such as comma operators
or function calls for the difference to be observable. Adds a test that should
pass but would've broken with my original commit.

This avoids creating extra .concat calls unless the result could be
potentially observably different from real template evaluation.
@mention-bot
Copy link

@Kovensky, thanks for your PR! By analyzing the history of the files in this pull request, we identified @yavorsky, @jridgewell and @existentialism to be potential reviewers.

Diogo Franco (Kovensky) added 2 commits August 12, 2017 13:50
…plit"

This reverts commit 86adc6f.

Signed-off-by: Diogo Franco (Kovensky) <diogomfranco@gmail.com>
More proof that it really is unsafe to merge the .concat calls if the
value is an expression that has any chance of executing impure code 😢
@Jessidhia Jessidhia changed the title Restrict the kinds of expressions that force a .concat call split Add another test for runtime order of template literals Aug 12, 2017
@Jessidhia Jessidhia merged commit 0538c3c into 7.0 Aug 12, 2017
@Jessidhia Jessidhia deleted the spec-template-optimization branch August 12, 2017 05:11
@bakkot
Copy link
Contributor

bakkot commented Aug 12, 2017

For reference, this is a followup to #5791.

@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 6, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 6, 2019
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants