Tagged templates #6250

tsnobip · 2023-05-09T09:50:50Z

The goal of this PR is to support tagged template literals in Rescript, primarily to benefit with externals from JS tools like sql`some sql query` or gql`some graphql query`.

@module("./tagged_template_lib.js") @variadic
external sql: (array<string>, array<string>) => string = "sql"
let table = "users"
let id = "5"
let query = sql`SELECT * FROM ${table} WHERE id = ${id}`

should generate

var Tagged_template_libJs = require("./tagged_template_lib.js");
var table = "users";
var id = "5";
var query = Tagged_template_libJs.sql`SELECT * FROM ${table} WHERE id = ${id}`;

The tag function should have the following signature in rescript:

let myTag: (array<string>, array<'a>) => 'b

where:

Array.length(stringArray) == Array.length(parameterArray) + 1

These two expressions should be equivalent:

let query = sql`SELECT * FROM ${table} WHERE id = ${id}`
let query = sql([ "SELECT * FROM ", " WHERE id = ", ""], [table, id])

This PR is originally based on kevinbarabash#2.

I'm trying to make it compile, remaining to fix:

issue with @as(json`{}`). in UncurriedExternals.res
@local not working in lam_pass_remove_alias.ml
wrong inlining of the interpolated string
all todos

cristianoc · 2023-05-09T11:36:17Z

what about js? Is it any different than `` now?

same

…

jscomp/core/lam_pass_remove_alias.ml

tsnobip · 2023-05-09T14:05:38Z

issue with @as(json`{}`). in UncurriedExternals.res

@cristianoc did we change something recently when it comes to the representation of json``?

cristianoc · 2023-05-10T09:41:04Z

jscomp/syntax/src/res_core.ml

+  in
+
+  match prefix with
+  | "js" | "j" | "json" -> genInterpolatedString ()


Can you explain where the need to support json is coming from.
Not sure if it's something you want to support, or if it's here just to fix some test?

The need is not really to support json but to avoid considering `json`something` as a tagged template.

But now that js and j string interpolations have been simplified, we can likely refactor this function to something simpler.

My goal was to first make the original PR work, then start polishing it.

Sounds good. The reason for asking is I would like to eventually remove that json thing entirely from externals too.

OK now I see again. I'm unsure why json should e a special tagged template v.s. say my_json. Is there a reason for that?

cristianoc · 2023-05-10T09:42:13Z

@tsnobip would you add a description at the top of this PR? It would help set the context on what this is doing.

jscomp/core/js_dump.ml

jscomp/test/tagged_template_test.js

cristianoc · 2023-05-10T23:14:04Z

@tsnobip sorry before getting into the question about the implementations, some questions about the description above.

let query = sql`SELECT * FROM ${table} WHERE id = ${id}`

should generate

var query = sql`SELECT * FROM ${table} WHERE id = ${id}`;

Not sure what that means. Could you give a self-contained example where table and id are variables in the source?
Then describe the desired code generated?
I would expect some inlining but perhaps that's not the goal, an if it isn't then there are a few follow-on questions about variable names that are not going to be preserved by the compiler (the "id" on the ReScript side won't be called "id" on the JS side sometimes).

tsnobip · 2023-05-11T07:05:43Z

@cristianoc I updated the example so it's more complete:

@module("./tagged_template_lib.js") @variadic
external sql: (array<string>, array<string>) => string = "sql"
let table = "users"
let id = "5"
let query = sql`SELECT * FROM ${table} WHERE id = ${id}`

should generate

var Tagged_template_libJs = require("./tagged_template_lib.js");
var table = "users";
var id = "5";
var query = Tagged_template_libJs.sql`SELECT * FROM ${table} WHERE id = ${id}`;

So now, if we consider a dummy implementation of the sql function that just wrap the content of the variable between quotes, the result in this case would be:

"SELECT * FROM 'users' WHERE id = '5'"

but with the inlining, the compiler right now generates:

var query = Tagged_template_libJs.sql`SELECT * FROM users WHERE id = 5`;

which means the sql function would not have the same input and therefore would not generate the same output.

You can have a look at jscomp/test/tagged_template_test.res to see a whole (for now failing) test.

cristianoc · 2023-05-11T08:15:47Z

@cristianoc I updated the example so it's more complete:
@module("./tagged_template_lib.js") @variadic
external sql: (array<string>, array<string>) => string = "sql"
let table = "users"
let id = "5"
let query = sql`SELECT * FROM ${table} WHERE id = ${id}`
should generate
var Tagged_template_libJs = require("./tagged_template_lib.js");
var table = "users";
var id = "5";
var query = Tagged_template_libJs.sql`SELECT * FROM ${table} WHERE id = ${id}`;
So now, if we consider a dummy implementation of the sql function that just wrap the content of the variable between quotes, the result in this case would be:
"SELECT * FROM 'users' WHERE id = '5'"
but with the inlining, the compiler right now generates:
var query = Tagged_template_libJs.sql`SELECT * FROM users WHERE id = 5`;
which means the sql function would not have the same input and therefore would not generate the same output.

You can have a look at jscomp/test/tagged_template_test.res to see a whole (for now failing) test.

There's a problem even before looking at the implementation strategy:

let foo = (id) => {
  Js.log(id)
  let id = id ++ "_"
  Js.log(id)
}

gives:

function foo(id) {
  console.log(id);
  var id$1 = id + "_";
  console.log(id$1);
}

so one cannot rely on id being id.
My understanding of the summary is that it's relying on this. And maybe I'm not understanding the summary. In which case we can update the summary and make it more explicit.

cristianoc · 2023-05-11T08:17:41Z

In fact, I'm probably making too many assumptions.
Is the idea that inlining should not happen? Perhaps inlining is fine and I'm reading too much in the specific example?

cristianoc · 2023-05-11T08:18:37Z

Another way of phrasing the question is: in the proposal, is there an assumption that tagged templates are preserved in the output, or can the compiler do anything as long at it implements the semantics of tagged templates?

tsnobip · 2023-05-11T09:29:50Z

so one cannot rely on id being id.
My understanding of the summary is that it's relying on this. And maybe I'm not understanding the summary. In which case we can update the summary and make it more explicit.

Indeed, we don't rely on the fact that id should keep its name, it can be any name as long as its value is what it should be.

cristianoc

Added quick review.

I think we can proceed with getting something that works. Then revisit a few points.

The one with json I would like to understand better.
The rest is just implementation details.

jscomp/core/lam_pass_remove_alias.ml

cristianoc · 2023-05-11T09:33:30Z

jscomp/core/lam_pass_remove_alias.ml

@@ -256,6 +257,18 @@ let simplify_alias (meta : Lam_stats.t) (lam : Lam.t) : Lam.t =
                            (Lam_beta_reduce.propagate_beta_reduce_with_map meta
                               param_map params body ap_args))
                | _ -> normal ()
+                in
+                let result = (match result with
+                | Lprim {primitive; args; loc} -> (match primitive with


formatting looks a little weird

hmm for some reason autoformat is broken in my setup, I'll try to fix it.

There's no auto format. Long story.
For now manual format

jscomp/ml/lambda.ml

jscomp/ml/translcore.ml

cristianoc · 2023-05-11T09:36:19Z

jscomp/ml/translcore.ml

@@ -985,14 +1005,14 @@ and transl_cases_try cases =
  in
  List.map transl_case_try cases

-and transl_apply ?(inlined = Default_inline) ?(uncurried_partial_application=None) lam sargs loc =
+and transl_apply ?(inlined = Default_inline) ?(is_tagged_template=false) ?(uncurried_partial_application=None) lam sargs loc =


Again, invasive, but let's revisit later.

jscomp/syntax/src/res_core.ml

cristianoc · 2023-05-11T09:37:58Z

jscomp/syntax/src/res_core.ml

@@ -2251,6 +2264,64 @@ and parseBinaryExpr ?(context = OrdinaryExpr) ?a p prec =
 (* ) *)

 and parseTemplateExpr ?(prefix = "js") p =
+  let partPrefix =
+    match prefix with
+    | "js" | "j" | "json" -> Some prefix


This I'd also like to revisit.
Not sure why we need to consider json. I'll need a reminder of why some issue with existing code and json suddenly comes up when extending templates. Seems counter-intuitive that a pure extension brings new issues to existing code/examples/tests.

yeah something must be broken, I did this to make tests pass, but I agree it's ugly and there's no reason why we couldn't use json`` tagged templates, or js and j now that they don't make sense.

This would be the relevant part:

let parse_processed = function | None -> Some External_arg_spec.DNone | Some "json" -> Some DNoQuotes | Some "*j" -> Some DStarJ

basically, the delimiter determines how the string is emitted: whether in quotes, etc.
And this is going to interfere.

cristianoc · 2023-05-11T09:39:30Z

jscomp/syntax/src/res_core.ml

+  in
+
+  match prefix with
+  | "js" | "j" | "json" -> genInterpolatedString ()


OK now I see again. I'm unsure why json should e a special tagged template v.s. say my_json. Is there a reason for that?

jscomp/test/tagged_template_test.js

tsnobip · 2023-05-11T10:22:51Z

Added quick review.

I think we can proceed with getting something that works. Then revisit a few points.

The one with json I would like to understand better. The rest is just implementation details.

I agree with your plan!

cristianoc · 2023-05-13T02:00:37Z

My suggestion is to:

work out the binary-operator-based translation
remove all the code changes beyond parsing: so no changes to type checker, lambda, back-end, etc

The result should be type safe by design, and zero cost in the simple cases.

cristianoc · 2023-05-13T02:03:34Z

Then perhaps unify with the existing template mechanism, where the operator is string concatenation, the state is string, the conversion function for strings is the identity, and for values it is also the identity.

cristianoc · 2023-05-13T04:19:30Z

ask chatgpt a bunch of questions...

Apologies for the confusion earlier. In the revised code, we're using a simplified version of how tagged templates work in JavaScript, and applying it to ReScript. Here's a summary:

The revised code breaks down the tagged template into its constituent parts and manually processes each one.

First, we define stringHandler and valueHandler functions, which will handle the string and value parts of the template respectively. In this case, stringHandler simply returns the string as-is, while valueHandler wraps the value in single quotes.
We define a combiner function that takes two states and concatenates them. This function will be used to merge the string parts with the value parts.
We declare table and id as the values to be interpolated into the SQL query.
The stringParts and valueParts arrays contain the string and value parts of the template respectively. This is similar to how tagged templates work in JavaScript: the string parts and interpolated values are passed to the tag function as separate arguments.
Finally, we manually construct the SQL query by calling combiner on each pair of string and value parts, and concatenating the results. This mimics how the tag function would process the template in JavaScript.

Simplified code by inlining the construction functions would be as follows:

let table = "users"
let id = "5"
let query = ("SELECT * FROM " ++ "'" ++ table ++ "'") ++ 
            (" WHERE id = " ++ "'" ++ id ++ "'")

In terms of correctness, this implementation correctly mimics the behavior of tagged templates in JavaScript. It handles the string and value parts separately, and constructs the final string in the correct order. As for encapsulation, the stringHandler, valueHandler, and combiner functions encapsulate the logic of handling strings and values, and combining them, respectively. This makes the code more modular and easier to understand and modify.

However, this implementation is more verbose and manual than using tagged templates in JavaScript, which handle the splitting and combining of string and value parts automatically. As of my knowledge cutoff in September 2021, ReScript did not support tagged templates, but the pull request you mentioned earlier would add this feature.

cristianoc · 2023-05-13T04:20:41Z

// Tag handler function definitions
let stringHandler = str => str
let valueHandler = value => "'" ++ value ++ "'"
let combiner = (state1, state2) => state1 ++ state2

// Definition of sql as a triple of handler functions
let sql = (stringHandler, valueHandler, combiner)

tsnobip · 2023-05-13T07:38:46Z

@cristianoc all those proposals are interesting but then we lose 0 cost bindings to JS tagged templates which is the biggest perk of this feature, shouldn't we focus on this first? What do you think @zth?

cristianoc · 2023-05-13T08:20:38Z

@cristianoc all those proposals are interesting but then we lose 0 cost bindings to JS tagged templates which is the biggest perk of this feature, shouldn't we focus on this first? What do you think @zth?

I am OK with 0 cost bindings to JS tagged unions.
But that is the opposite answer to my first design question.
If that's what we want to do, then of course inlining is a deal breaker etc.

tsnobip · 2023-05-13T15:25:18Z

@cristianoc all those proposals are interesting but then we lose 0 cost bindings to JS tagged templates which is the biggest perk of this feature, shouldn't we focus on this first? What do you think @zth?

I am OK with 0 cost bindings to JS tagged unions. But that is the opposite answer to my first design question. If that's what we want to do, then of course inlining is a deal breaker etc.

yeah we have to make a decision here, we could ask on the forum what would be the preferred use case.

cristianoc · 2023-05-14T02:42:01Z

Definitely. The community should also indicate that this feature is needed.

zth · 2023-05-14T07:20:59Z

Ok, so let's try and sum this up and the various paths we can take.

This was not made clear enough from our side (me + @tsnobip who's been talking about this) from the start, but the primary reason for building native support for tagged template literal functionality is to integrate with the broader JS ecosystem. Rolling our own solution to the same problem that tagged template literals "solve" is also quite interesting, but it wouldn't be solving the same problem, which is integration with the broader ecosystem.

Integrating with the broader ecosystem means two things (to me):

Emitting actual tagged template literal syntax in JS: let stuff = someTag`hello ${name}` . This is of course negotiable, but emitting the raw function call instead of this means we loose compability with a bunch of tooling that relies on that exact syntax.
Having typings work the same way, meaning being able to control the expected value one can put into the template strings, as well as the return type from the tag.

So, to sum up my stance, if this feature is to be useful, it should (in falling priority):

Exactly look like and work like the JS equivalent does.
Or alternatively, but slightly less useful, look like tagged template literals in ReScript (most important because this is what the dev will interact with) and compile to direct function calls to the tag function. We loose integration with some (badly designed but still widely used) tooling, but we retain the ergonomics of using tagged template literals in ReScript.

As for whether this is something the community wants, here's a few issues and forum posts related to tagged template literals:
#4070, #4294, https://forum.rescript-lang.org/t/rfc-support-for-tagged-template-literals/3744, https://forum.rescript-lang.org/t/binding-to-tagged-template-function/1093, https://forum.rescript-lang.org/t/proposal-typesafe-tagged-template-literals/1204, https://forum.rescript-lang.org/t/type-safe-database-access-with-rescript/2376, https://forum.rescript-lang.org/t/code-first-graphql-schema-generation-on-backend-like-nexusjs/2974

Here's a few examples of popular JS libs relying on tagged template literals for ergonomic (subjective of course) APIs:

Outside of this I think it could open up some pretty interesting possibilities for building libs leveraging tagged template literals in pure ReScript as well.

Now, with all of that, my personal view is that this feature would be valuable and would plug a hole that would allow certain use cases much more naturally. But, it's probably no where near as important as for example async/await was, in terms of what the community needs/wants. So, I think it comes down to how complex this functionality will get. If it doesn't complicate things too much I think it's worth it, and I think it would serve a good purpose. But if it needs moderate to highly complex changes to the compiler then it's probably not.

These questions come to mind from my end after following this PR and discussion:

Inlining, when does it actually happen? Does it happen in scenarios where it's considered breaking? Considered breaking = the end result returned from the tag function isn't the same as it would be with the same code in regular JS
How much complexity does this functionality add? How far can we go in reducing the complexity of the feature?

@cristianoc @tsnobip what are your thoughts on this?

zth · 2023-05-14T08:02:26Z

After offline discussions we've decided to pause working on this for now. We will likely revisit at some point in the future.

jfrolich · 2023-05-16T09:47:35Z

After offline discussions we've decided to pause working on this for now. We will likely revisit at some point in the future.

Any context why? Is it because it's very hard to prevent inlining?

zth · 2023-06-13T08:10:21Z

Sorry for dropping the ball on this, my fault.

In order to move this forward, we need to more clearly state:

What exact real world use cases this solves, and why they're important. So rather than saying "things like", we should enumerate exact use cases and why they're only solved fully via this new mechanism.
Make it clearer how this is a recurrent need and not just something peripheral that very few will use.

Again, on me for not being clearer from the start.

tsnobip · 2023-07-19T14:21:38Z

ok, so for my use case I use rescript to write GraphQL servers backed by a Postgres DB using postgraphile.

This makes me query the DB quite a lot, and hence, write SQL queries. You have basically two ways to do it,
you either use parameters like this:

query(
  pgClient,
  ~statement="select * from app_private.create_media($1, $2, $3, $4, $5)",
  ~params=[uuid(productId), string(path), string(filename), float(size), string(sha256)],
)

but it's very brittle when you start having a significant number of arguments and start modifying them, so you could instead use tagged templates like this:

query(
  pgClient,
  sql`select * from app_private.create_media(${uuid(productId)}, ${string(path)}, ${string(filename)}, ${float(size)}, ${string(sha256)})`
)

This solution is way more robust but is not doable right now in rescript.

I also write GraphQL schema language using a gql tagged template function, but using tagged templates manually is so complex and error prone I just make it a single string and don't use parameters, copy pasting blocks instead of using parameters, which is of course not ideal.

tsnobip · 2023-07-19T14:28:29Z

might be better to use the forum to list those use cases, I've started doing it in the RFC.

tsnobip · 2024-01-25T11:52:12Z

@cristianoc @zth I simplified the implementation a lot, we now have to add a @tagged_template annotation to the external, that's it! I don't know why I didn't think of it before!

Let me know what you think

cristianoc · 2024-01-25T12:37:54Z

@cristianoc @zth I simplified the implementation a lot, we now have to add a @tagged_template annotation to the external, that's it! I don't know why I didn't think of it before!

Let me know what you think

Looks great!

zth · 2024-01-25T13:14:41Z

@tsnobip ready to merge?

tsnobip · 2024-01-25T13:23:18Z

@tsnobip ready to merge?

@zth I've just amended the commit to allow for both @taggedTemplate and @tagged_template annotations. But I think it's ready to merge now!

zth · 2024-01-25T13:25:52Z

Why allow both though? Pick the most idiomatic sounding one unless there's strong reasons to have both.

tsnobip · 2024-01-25T13:28:42Z

Why allow both though? Pick the most idiomatic sounding one unless there's strong reasons to have both.

haha mostly to avoid bike-shedding, most "historical" annotations use snake_case (@set_index), most newer annotations use camlCase (@genType) which is more idiomatic in modern ReScript I guess, should I just pick @taggedTemplate then @zth?

zth · 2024-01-25T13:30:35Z

Yes, definately. 😀

tsnobip · 2024-01-25T13:37:27Z

OK we're all good then, I've enabled auto-merge (squash and merge)

zth · 2024-01-25T13:44:41Z

Perfect! Great work!

tsnobip commented May 9, 2023

View reviewed changes

jscomp/core/lam_pass_remove_alias.ml Outdated Show resolved Hide resolved

cristianoc reviewed May 10, 2023

View reviewed changes

tsnobip commented May 10, 2023

View reviewed changes

jscomp/core/js_dump.ml Outdated Show resolved Hide resolved

tsnobip commented May 10, 2023

View reviewed changes

jscomp/test/tagged_template_test.js Outdated Show resolved Hide resolved

cristianoc reviewed May 11, 2023

View reviewed changes

tsnobip force-pushed the tagged_templates branch from c1f4f12 to 56b8642 Compare January 15, 2024 14:49

tsnobip and others added 6 commits January 25, 2024 10:15

get rid of duplicate filter_map

ae7c8b6

implement expression_desc for Tagged_template

dc2aef6

remove useless comment

83e36ee

remove useless comment

f7a72dc

document special template literal prefixes

0f23f76

Set version to 11.1.0

d086f5b

tsnobip force-pushed the tagged_templates branch from 1c6ecd3 to c1d2a03 Compare January 25, 2024 09:17

tsnobip changed the base branch from master to 11.0_release January 25, 2024 09:18

add tagged templates to changelog

d7f48c7

tsnobip force-pushed the tagged_templates branch 4 times, most recently from 589206d to 8bd1dfd Compare January 25, 2024 11:48

tsnobip force-pushed the tagged_templates branch from 8bd1dfd to dc9cbd0 Compare January 25, 2024 13:21

use @taggedTemplate annotation in ffi instead

949af10

tsnobip force-pushed the tagged_templates branch from dc9cbd0 to 949af10 Compare January 25, 2024 13:31

tsnobip enabled auto-merge (squash) January 25, 2024 13:36

tsnobip merged commit 6fa3289 into 11.0_release Jan 25, 2024

zth deleted the tagged_templates branch January 25, 2024 13:51

tsnobip added a commit to tsnobip/rescript-lang.org that referenced this pull request Jan 25, 2024

document tagged templates from rescript-lang/rescript#6250

afa3e0c

tsnobip mentioned this pull request Jan 25, 2024

Document tagged templates rescript-lang/rescript-lang.org#803

Merged

Tagged templates #6250

Tagged templates #6250

Conversation

tsnobip commented May 9, 2023 • edited Loading

cristianoc commented May 9, 2023 via email

tsnobip commented May 9, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cristianoc commented May 10, 2023

cristianoc commented May 10, 2023

tsnobip commented May 11, 2023 • edited Loading

cristianoc commented May 11, 2023

cristianoc commented May 11, 2023

cristianoc commented May 11, 2023

tsnobip commented May 11, 2023

cristianoc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tsnobip commented May 11, 2023

cristianoc commented May 13, 2023

cristianoc commented May 13, 2023 • edited Loading

cristianoc commented May 13, 2023

cristianoc commented May 13, 2023

tsnobip commented May 13, 2023

cristianoc commented May 13, 2023

tsnobip commented May 13, 2023

cristianoc commented May 14, 2023

zth commented May 14, 2023

zth commented May 14, 2023

jfrolich commented May 16, 2023

zth commented Jun 13, 2023

tsnobip commented Jul 19, 2023

tsnobip commented Jul 19, 2023

tsnobip commented Jan 25, 2024

cristianoc commented Jan 25, 2024

zth commented Jan 25, 2024

tsnobip commented Jan 25, 2024

zth commented Jan 25, 2024

tsnobip commented Jan 25, 2024

zth commented Jan 25, 2024

tsnobip commented Jan 25, 2024

zth commented Jan 25, 2024

tsnobip commented May 9, 2023 •

edited

Loading

tsnobip commented May 11, 2023 •

edited

Loading

cristianoc commented May 13, 2023 •

edited

Loading