Skip to content

^10.0.0-alpha.1 template literals output "regression" #5521

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

Closed
DZakh opened this issue Jul 6, 2022 · 6 comments · Fixed by #6090
Closed

^10.0.0-alpha.1 template literals output "regression" #5521

DZakh opened this issue Jul 6, 2022 · 6 comments · Fixed by #6090
Milestone

Comments

@DZakh
Copy link
Member

DZakh commented Jul 6, 2022

The compiler started adding empty strings around values in template literals. Technically doesn't break anything, but adds a redundant operation.

Example:

Js.log(`${1->Js.Int.toString}${2->Js.Int.toString}`)

Output:

console.log("" + (1).toString() + "" + (2).toString() + "");

Before:

console.log((1).toString() + (2).toString());

Context: DZakh/sury#1 (comment), #5514 (comment)

@cristianoc
Copy link
Collaborator

@DZakh see here: rescript-lang/syntax#602

@cristianoc
Copy link
Collaborator

That attempt at the syntax level is no good. Some cleanup is needed first. Then it should be doable in the compiler back-end.

@cristianoc
Copy link
Collaborator

cristianoc commented Jul 8, 2022

It's actually not clear how to do it in the compiler back-end without invasive changes.
The code generated for normal (default "js") and "j" interpolation is the same: appending strings together.
In the back-end, one cannot distinguish them.
And only for "js" it is safe to remove "" + .
So we won't try to do this optimisation for this version of the compiler.

@DZakh
Copy link
Member Author

DZakh commented Jul 8, 2022

What about the trailing empty string + ""?

@cristianoc
Copy link
Collaborator

Same

@cristianoc cristianoc added this to the v10.1 milestone Jul 9, 2022
@cristianoc
Copy link
Collaborator

Putting this under milestone 10.1.

If anyone is interested in contributing a PR meanwhile, that's great.
That PR should be considered an optimisation, which should not fire for j`stuff` template expressions.

@cristianoc cristianoc removed this from the v10.1 milestone Sep 7, 2022
@cristianoc cristianoc added this to the v11.0 milestone Mar 21, 2023
cristianoc added a commit that referenced this issue Mar 21, 2023
Now that `j` interpolation is gone, string append can be optimized in the back-end.

Fixes #5521
cristianoc added a commit that referenced this issue Mar 21, 2023
Now that `j` interpolation is gone, string append can be optimized in the back-end.

Fixes #5521
cristianoc added a commit that referenced this issue Mar 21, 2023
Now that `j` interpolation is gone, string append can be optimized in the back-end.

Fixes #5521
cristianoc added a commit that referenced this issue Mar 21, 2023
Now that `j` interpolation is gone, string append can be optimized in the back-end.

Fixes #5521
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants