Make tagged templates a first-class type#8461
Conversation
Introduce a predefined global `taggedTemplate<'param, 'output>` type (arity 2, mirroring `promise`/`dict` in predef.ml) so that tagged-templateness can live on the type of a value rather than on an external binding site. Add the thin `Stdlib.TaggedTemplate` module aliasing it as `t<'param, 'output>` and exposing `make`, which lifts a plain ReScript tag function `(array<string>, array<'param>) => 'output` into the type by adapting the variadic JS tag-call convention. Part of the first-class tagged-template work (rescript-lang#8415).
When an application carries the parser's `res.taggedTemplate` attribute,
unify the tag's type with a fresh `taggedTemplate<'param, 'output>` and
type the desugared `(array<string>, array<'param>)` arguments against it,
yielding `'output`. This gives clean inference for unannotated tags
(`tag => tag\`...${x}...\``).
If the tag's type is not a `taggedTemplate` (e.g. an old
`(array<string>, array<'a>) => 'o` tag function), raise a dedicated
migration error steering the user to the new binding form or
`TaggedTemplate.make`.
Part of rescript-lang#8415.
Add a `Ptagged_template` primitive (`[tag; strings; values]`) carried from the ml lambda layer through to the JS-IR layer. translcore detects a `res.taggedTemplate` application and emits the primitive, translating the tag as a plain value — so the real backtick literal is emitted at every call site regardless of how the tag was obtained (external, let-binding, function parameter, factory result, cross-module). lam_compile_primitive turns the primitive into the existing `Js_exp_make.tagged_template` node, reusing the established JS dump path. Part of rescript-lang#8415.
Tagged-templateness now lives on the type, so the `@taggedTemplate` external decorator is obsolete. Using it is now a compile error that steers users to bind the external with the `taggedTemplate<...>` type instead. With the decorator gone, the `tagged_template` flag on the `Js_call` FFI spec is always false, so remove the field and the now-unreachable tagged-template branch in lam_compile_external_call. Part of rescript-lang#8415.
Teach the completion engine that applying a value of type `taggedTemplate<'param, 'output>` (via backtick syntax) yields its `'output` type, restoring dot/pipe completions on a tagged-template result. Update the analysis fixture to the new binding form. Map the `taggedTemplate` builtin to an opaque type in gentype (it is a variadic JS function), so genType'd values don't emit a dangling TS reference. Part of rescript-lang#8415.
Rewrite the tagged-template runtime test around the new model: an external bound with the `taggedTemplate` type, a runtime-constructed tag via a factory, a tag passed as a function argument, and a ReScript tag lifted with `TaggedTemplate.make` — asserting real tagged-template syntax is emitted at every call site. Add super_errors fixtures for the two new errors (removed `@taggedTemplate` decorator, and backtick syntax on a non-tag value), catalog the new `Tagged_template_non_tag` variant in ERROR_VARIANTS.md, and add CHANGELOG entries. Part of rescript-lang#8415.
Add runtime assertions that pin down the real behaviour of the feature:
- A `rawTag` binding proves the compiler emits a genuine tagged template:
the tag receives a frozen `TemplateStringsArray` with `.raw`, not a
plain/variadic function call.
- A cross-module case: a `taggedTemplate` value defined in
`Tagged_template_binding` and consumed here with backtick syntax, so the
consumer sees only the type yet still emits a real tagged template.
- A compile-only `tagged_template_global_import` fixture pins the
generated JS for a bare-package import (`@module("postgres")`) returning
a `taggedTemplate`, vs. the existing relative `./file.js` bindings.
Part of rescript-lang#8415.
Cover the ways a tagged template can be used incorrectly:
- super_errors: calling a `taggedTemplate` value as a regular function
(`sql(strings, args)`) — it is not a function type; and interpolating a
value of the wrong `'param` type.
- syntax_tests: unclosed tagged-template backticks (both an unterminated
literal and an unterminated `${...}` interpolation), which recover with
"Did you forget to close this template expression with a backtick?".
Part of rescript-lang#8415.
Drop the references to the issue's "Problem N" labels; each comment now just explains what the test exercises.
The previous `taggedTemplateUnclosedInterpolation` fixture left both the
`${...}` and the backtick unterminated, so it just re-exercised the
"forgot the backtick" path. Replace it with an empty interpolation
(`${}`) inside a properly closed template, which exercises the distinct
"this expression block is empty" interpolation error.
Two error paths reported confusing, leaky messages for tagged templates:
- Calling a tag as a function (`sql(strings, args)`) gave the generic
"this can't be called, it's not a function". It now explains the value
is a tagged-template tag and to use backtick syntax instead.
- Interpolating a value of the wrong `'param` type reported an "array
item" type error (exposing the desugared values array, which the user
never wrote). It now uses a dedicated `TaggedTemplateValue` clash
context: "This interpolated value has type: … But this tag expects
interpolations of type: …", typing each `${...}` directly instead of
routing through the array-literal typer.
Add a runtime test for a tag bound to a bare (non-relative) import specifier, complementing the relative-path bindings. A Node `imports` subpath entry (`#tagged-template-pg`) resolves the bare specifier to a committed mock without installing a package. The mock throws unless it receives a real `TemplateStringsArray`, so the test passing proves the call site emitted real tagged-template syntax against the bare import.
Adding the `TaggedTemplate` stdlib module makes it appear in module completion lists, so the affected analysis snapshots gain the new entry.
Exercise the `taggedTemplate` / `TaggedTemplate.t` -> opaque (`unknown`) gentype translation, which was previously untested. Both the global builtin and the stdlib alias are emitted as `(x:unknown) => unknown`.
A `taggedTemplate<'param, 'output>` is a variadic tag function, so map it to the precise TypeScript signature (strings: TemplateStringsArray, ...values: 'param[]) => 'output rather than the opaque `unknown`. This is also the signature TypeScript requires for a value to be usable with backtick tagged-template syntax. gentype has no rest-argument field, so the spread is encoded in the parameter name (emitted verbatim before the type); the generated output type-checks under `tsc`.
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #8461 +/- ##
==========================================
+ Coverage 60.91% 60.95% +0.03%
==========================================
Files 374 374
Lines 54095 54116 +21
==========================================
+ Hits 32953 32986 +33
+ Misses 21142 21130 -12
🚀 New features to boost your workflow:
|
The testrepo's rescript-bun dependency still used the removed @taggedTemplate decorator on its sh/shExpr shell tags, which now fails to compile. Extend the existing yarn patch to bind them with the first-class taggedTemplate<'param, 'output> type instead.
typecore builds the tagged-template type directly via Predef.path_tagged_template, so the type_tagged_template constructor helper was never called. Remove it (and its .mli signature) rather than leave dead, uncoverable code.
rescript
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
The parser always desugars the interpolated values of a tagged template into an array literal, so the non-array branch was dead code. Replace its silent generic-array fallback with assert false, matching the sibling sargs match in the same block and documenting the invariant instead of masking a potential desugaring bug.
…om coverage - Remove the unused context_to_string function (zero callers repo-wide); its TaggedTemplateValue arm was the only flagged line there. - Mark the debug-only lambda printers and the optimizer fast-path arms (no_side_effects, eq_primitive_approx) with [@coverage off] + a comment. These are reachable only from -drawlambda/-dlambda dumps or optimizer term-equality/purity checks that the test suite never exercises for tagged templates. The shared OR-pattern groups are kept intact so their existing coverage is unaffected.
The no_side_effects tagged-template branch was wrongly marked [@coverage off] with a comment implying it was unreachable. It is in fact reachable by an ordinary top-level tag application with pure arguments. Put Ptagged_template back alongside the other effectful primitives (Pjs_call, etc.); its bisect row is cold for the same reason theirs is (no_side_effects short-circuits on impure args), so it needs no special handling.
tsnobip
left a comment
There was a problem hiding this comment.
amazing work @JonoPrest!!
It'd be cool if @cristianoc could review it given it touches quite many places of the compiler including the type system, but the overall implementation is very elegant!
| let context_to_string = function | ||
| | Some WhileCondition -> "WhileCondition" | ||
| | Some ForLoopCondition -> "ForLoopCondition" | ||
| | Some AssertCondition -> "AssertCondition" | ||
| | Some IfCondition -> "IfCondition" | ||
| | Some (Statement _) -> "Statement" | ||
| | Some (MathOperator _) -> "MathOperator" | ||
| | Some ArrayValue -> "ArrayValue" | ||
| | Some (SetRecordField _) -> "SetRecordField" | ||
| | Some (RecordField _) -> "RecordField" | ||
| | Some MaybeUnwrapOption -> "MaybeUnwrapOption" | ||
| | Some SwitchReturn -> "SwitchReturn" | ||
| | Some TryReturn -> "TryReturn" | ||
| | Some StringConcat -> "StringConcat" | ||
| | Some (FunctionArgument _) -> "FunctionArgument" | ||
| | Some JsxComponent -> "JsxComponent" | ||
| | Some ComparisonOperator -> "ComparisonOperator" | ||
| | Some IfReturn -> "IfReturn" | ||
| | Some TernaryReturn -> "TernaryReturn" | ||
| | Some Await -> "Await" | ||
| | Some BracedIdent -> "BracedIdent" | ||
| | Some LetUnwrapReturn -> "LetUnwrapReturn" | ||
| | None -> "None" |
There was a problem hiding this comment.
I saw there was no coverage on this from the report and then checked it was dead code so I just removed it. I can put it back though.
| | Tconstr (path, _, _) when Path.same path Predef.path_tagged_template -> | ||
| fprintf ppf | ||
| "@[<v>@[<2>This is a tagged-template tag of type@ @{<info>%a@}@]@,\ | ||
| It can't be called like a function. Use it with backtick syntax \ | ||
| instead, e.g. @{<info>tag`SELECT ${id}`@}.@]" | ||
| type_expr typ |
There was a problem hiding this comment.
is there any reason why we can't call it as a function that takes 2 arrays as input, because of the variadic shape?
There was a problem hiding this comment.
Hey Paul,
So I thought about this a bit and ended up just making it illegal. The problem is that to actual compile to a tagged template call, you would need an array literal of strings as first arg myTag(["a", "b"], ...) -> it can't be myTag(injectedStringArray, ...) and then the second arg would need to have the correct length of args based on the number of strings in the string array. This IMO is too finicky for compiler errors etc to create a nice experience.
The other option is to compile to what it was doing before in this case which was to export a wrapper call that spreads in the variadic args. This would be nice for backwards compatibility but I have a few problems with this:
- It creates more complexity and cases for the compiler to be concerned with.
- The consumers of the API and authors of taggedTemplate functions need to handle both ways of it being called. It's quite nice to have compiler guarantee that it's invoked as a template if you're creating an API that sanitises sql injection for example. Which is arguably why people author tagged template libs to begin with.
I am open to allowing this if we are set on backwards compatibility.
There was a problem hiding this comment.
My thinking was that if you need the composability of the function, you can pass around the plain function and lift it into TaggedTemplate.make just before the call site.
Co-authored-by: Paul Tsnobiladzé <paul.tsnobiladze@gmail.com>
cristianoc
left a comment
There was a problem hiding this comment.
So one annotation is gone but res.taggedtemplate is still used right?
Not sure how things were represented before: two different annotations?
Hey @cristianoc,
Previously there was an annotation You could also call any function with type The linked issue explains the problems in detail with the current impl |
Closes #8415.
Replaces the
@taggedTemplatedecorator with a first-classtaggedTemplate<'param, 'output>type (plus a smallTaggedTemplatestdlibmodule). Because tag-ness now lives in the type instead of the FFI annotation,
the compiler emits a real JS tagged-template literal at every call site -
across modules, through first-class values, and for tags built at runtime by a
factory (the
postgres-style case).TaggedTemplate.makelifts a plain(array<string>, array<'param>) => 'outputfunction into the type so ReScript-authored tags work with backtick syntax too.
Breaking: the
@taggedTemplatedecorator is removed. Using this or backticksyntax on a non-
taggedTemplatevalue is now a compile error pointing at thenew binding form, with tag-specific messages for the common misuse cases.