From 39249d31f9673c212a035a377eefa62368afac43 Mon Sep 17 00:00:00 2001 From: Pau Sanchez Date: Sun, 26 Jul 2026 10:57:01 +0200 Subject: [PATCH 01/12] feat!: implement liquidjs templating system MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Templates are parsed once and rendered per recipient, so a basic template can render at 140k renders/s whereas a richer one with sevearl conditionals and loops renders at ~18k renders/s. 1M-contact campaign spends under one minute of CPU time for rendering, which is negligible against 1M SES calls. The engine is sandboxed: {% include %}, {% render %} and {% layout %} are rejected because they resolve partials off the file system, and parse/render/memory are capped so a runaway loop can't stall a worker. Rendering stays lenient, a template that fails to parse falls back to plain {{variable}} substitution rather than failing the send. Syntax errors are surfaced at write time instead: POST/PATCH of templates and campaigns, and POST /v1/send with an inline subject/body, now return 400 with the line and column. Adding multiple tests to assess the proper behaviour and to keep some performance guarantees (for both memory & CPU). Backwards compatible with existing template system for most scenarios. See template-language.mdx Notes for existing templates for more info, but basically: 0/false/NaN are now rendered while before they produced empty string. Also {{ name ?? 'hi' }} produced 'hi' with quotes, which seems like a bug, now emits hi with no quotes. Body copy containing balanced {% %} markup is now executed rather than printed literally — wrap it in {% raw %} to keep it as text. --- apps/api/src/controllers/Actions.ts | 5 + apps/api/src/controllers/Campaigns.ts | 5 + apps/api/src/controllers/Templates.ts | 5 + apps/api/src/services/CampaignService.ts | 19 +- apps/api/src/services/EmailService.ts | 8 +- .../__tests__/CampaignService.test.ts | 74 ++++ .../__tests__/templateValidation.test.ts | 64 +++ apps/api/src/utils/templateValidation.ts | 32 ++ apps/wiki/content/docs/concepts/templates.mdx | 41 +- .../content/docs/guides/custom-fields.mdx | 12 +- apps/wiki/content/docs/guides/meta.json | 1 + .../content/docs/guides/template-language.mdx | 190 +++++++++ packages/shared/package.json | 1 + .../shared/src/__tests__/template.test.ts | 402 ++++++++++++++++++ packages/shared/src/index.ts | 2 +- packages/shared/src/template/engine.ts | 92 ++++ packages/shared/src/template/index.ts | 198 +++++++++ .../src/{template.ts => template/legacy.ts} | 27 +- packages/shared/src/template/preprocess.ts | 231 ++++++++++ .../template-rendering.perf.test.ts | 248 +++++++++++ yarn.lock | 20 + 21 files changed, 1648 insertions(+), 29 deletions(-) create mode 100644 apps/api/src/utils/__tests__/templateValidation.test.ts create mode 100644 apps/api/src/utils/templateValidation.ts create mode 100644 apps/wiki/content/docs/guides/template-language.mdx create mode 100644 packages/shared/src/__tests__/template.test.ts create mode 100644 packages/shared/src/template/engine.ts create mode 100644 packages/shared/src/template/index.ts rename packages/shared/src/{template.ts => template/legacy.ts} (51%) create mode 100644 packages/shared/src/template/preprocess.ts create mode 100644 test/performance/template-rendering.perf.test.ts diff --git a/apps/api/src/controllers/Actions.ts b/apps/api/src/controllers/Actions.ts index 1266e807f..ff280192e 100644 --- a/apps/api/src/controllers/Actions.ts +++ b/apps/api/src/controllers/Actions.ts @@ -11,6 +11,7 @@ import {EmailVerificationService} from '../services/EmailVerificationService.js' import {EventService} from '../services/EventService.js'; import {NotFound, ValidationError} from '../exceptions/index.js'; import {CatchAsync} from '../utils/asyncHandler.js'; +import {assertValidTemplateSyntax} from '../utils/templateValidation.js'; import {DASHBOARD_URI} from '../app/constants.js'; /** @@ -188,6 +189,10 @@ export class Actions { const {to, subject, body, subscribed, name, from, reply, headers, data, template, attachments} = ActionSchemas.send.parse(req.body); + // Inline subject/body are templates too. Stored templates are checked when they're + // saved, but these arrive with the request and have never been validated. + assertValidTemplateSyntax({subject, body}); + // Normalize recipients to array and parse email/name type Recipient = {email: string; name?: string}; const recipients: Recipient[] = (Array.isArray(to) ? to : [to]).map(recipient => { diff --git a/apps/api/src/controllers/Campaigns.ts b/apps/api/src/controllers/Campaigns.ts index b776ebe04..f740b8439 100644 --- a/apps/api/src/controllers/Campaigns.ts +++ b/apps/api/src/controllers/Campaigns.ts @@ -9,6 +9,7 @@ import {CampaignService} from '../services/CampaignService.js'; import {DomainService} from '../services/DomainService.js'; import {CatchAsync} from '../utils/asyncHandler.js'; import {parseListSort} from '../utils/listSort.js'; +import {assertValidTemplateSyntax} from '../utils/templateValidation.js'; @Controller('campaigns') export class Campaigns { @@ -32,6 +33,8 @@ export class Campaigns { throw new HttpException(400, 'Audience condition is required for FILTERED audience type'); } + assertValidTemplateSyntax({subject, body}); + // Verify domain ownership and verification await DomainService.verifyEmailDomain(from, auth.projectId); @@ -164,6 +167,8 @@ export class Campaigns { throw new HttpException(400, 'Audience condition is required for FILTERED audience type'); } + assertValidTemplateSyntax({subject, body}); + // Verify domain ownership and verification if 'from' is being updated if (from) { await DomainService.verifyEmailDomain(from, auth.projectId); diff --git a/apps/api/src/controllers/Templates.ts b/apps/api/src/controllers/Templates.ts index 20d83a0ea..da2c989c8 100644 --- a/apps/api/src/controllers/Templates.ts +++ b/apps/api/src/controllers/Templates.ts @@ -7,6 +7,7 @@ import {DomainService} from '../services/DomainService.js'; import {TemplateService} from '../services/TemplateService.js'; import {CatchAsync} from '../utils/asyncHandler.js'; import {parseListSort} from '../utils/listSort.js'; +import {assertValidTemplateSyntax} from '../utils/templateValidation.js'; @Controller('templates') export class Templates { @@ -84,6 +85,8 @@ export class Templates { return res.status(400).json({error: 'From address is required'}); } + assertValidTemplateSyntax({subject, body}); + // Verify domain ownership and verification await DomainService.verifyEmailDomain(from, auth.projectId!); @@ -117,6 +120,8 @@ export class Templates { return res.status(400).json({error: 'Template ID is required'}); } + assertValidTemplateSyntax({subject, body}); + // Verify domain ownership and verification if 'from' is being updated if (from) { await DomainService.verifyEmailDomain(from, auth.projectId!); diff --git a/apps/api/src/services/CampaignService.ts b/apps/api/src/services/CampaignService.ts index deb80c891..31591c771 100644 --- a/apps/api/src/services/CampaignService.ts +++ b/apps/api/src/services/CampaignService.ts @@ -1,5 +1,6 @@ import type {Campaign, Contact, Prisma} from '@plunk/db'; import {CampaignAudienceType, CampaignStatus, EmailSourceType, EmailStatus, TemplateType} from '@plunk/db'; +import {compileTemplate} from '@plunk/shared'; import type {CreateCampaignData, FilterCondition, PaginatedResponse, UpdateCampaignData} from '@plunk/types'; import {fromPrismaJson, toPrismaJson} from '@plunk/types'; import signale from 'signale'; @@ -537,6 +538,11 @@ export class CampaignService { // Get batch of recipients using cursor-based pagination const {contacts, nextCursor, hasMore} = await this.getRecipientsCursor(campaign.projectId, campaign, limit, cursor); + // Parse the Liquid templates once per batch rather than once per recipient. The + // subject and body are identical for every contact, only the variables differ. + const subjectTemplate = compileTemplate(campaign.subject); + const bodyTemplate = compileTemplate(campaign.body); + // Queue emails for each contact for (const contact of contacts) { try { @@ -553,17 +559,8 @@ export class CampaignService { manageUrl: `${DASHBOARD_URI}/manage/${contact.id}`, }; - const renderedSubject = EmailService.format({ - subject: campaign.subject, - body: '', - data: variables, - }).subject; - - const renderedBody = EmailService.format({ - subject: '', - body: campaign.body, - data: variables, - }).body; + const renderedSubject = subjectTemplate.render(variables); + const renderedBody = bodyTemplate.render(variables); await EmailService.sendCampaignEmail({ projectId: campaign.projectId, diff --git a/apps/api/src/services/EmailService.ts b/apps/api/src/services/EmailService.ts index 51ae7efa6..a6cc50670 100644 --- a/apps/api/src/services/EmailService.ts +++ b/apps/api/src/services/EmailService.ts @@ -642,8 +642,12 @@ export class EmailService { } /** - * Format email template by replacing variables in subject and body - * Uses shared template rendering from @plunk/shared + * Render a single email's subject and body. + * + * Uses the shared Liquid renderer from @plunk/shared, which caches parsed templates, + * so the per-email call sites here don't re-parse the same body for every recipient. + * When rendering a known template for many contacts in one pass, prefer + * `compileTemplate` to hoist the parse out of the loop entirely (see CampaignService). */ public static format({subject, body, data}: {subject: string; body: string; data: Record}): { subject: string; diff --git a/apps/api/src/services/__tests__/CampaignService.test.ts b/apps/api/src/services/__tests__/CampaignService.test.ts index e20e23ef4..32a09b512 100644 --- a/apps/api/src/services/__tests__/CampaignService.test.ts +++ b/apps/api/src/services/__tests__/CampaignService.test.ts @@ -665,4 +665,78 @@ describe('CampaignService', () => { expect(scheduledCampaign.totalRecipients).toBe(10); }); }); + + // ======================================== + // TEMPLATE RENDERING (processBatch) + // ======================================== + describe('processBatch template rendering', () => { + /** Create a SENDING campaign plus contacts, then run the first batch. */ + async function sendBatch({subject, body}: {subject: string; body: string}, contacts: Record[]) { + for (const data of contacts) { + await factories.createContact({projectId, subscribed: true, data}); + } + + const campaign = await factories.createCampaign({ + projectId, + subject, + body, + status: CampaignStatus.SENDING, + audienceType: CampaignAudienceType.ALL, + }); + + await CampaignService.processBatch(campaign.id, 1, 0, 500); + + return prisma.email.findMany({ + where: {campaignId: campaign.id}, + include: {contact: true}, + }); + } + + it('renders each contact through the Liquid template', async () => { + const emails = await sendBatch( + { + subject: '{% if locale == "es" %}Tu oferta{% else %}Your offer{% endif %}', + body: '

Hi {{firstName ?? there}}, you are on {{plan | upcase}}.

', + }, + [ + {firstName: 'Ada', plan: 'pro', locale: 'en'}, + {firstName: 'Bruno', plan: 'free', locale: 'es'}, + {plan: 'free', locale: 'en'}, + ], + ); + + expect(emails).toHaveLength(3); + + const byFirstName = new Map( + emails.map(email => [(email.contact.data as Record)?.firstName ?? null, email]), + ); + + expect(byFirstName.get('Ada')?.subject).toBe('Your offer'); + expect(byFirstName.get('Ada')?.body).toBe('

Hi Ada, you are on PRO.

'); + + expect(byFirstName.get('Bruno')?.subject).toBe('Tu oferta'); + expect(byFirstName.get('Bruno')?.body).toBe('

Hi Bruno, you are on FREE.

'); + + // Contact without a firstName falls back + expect(byFirstName.get(null)?.body).toBe('

Hi there, you are on FREE.

'); + }); + + it('renders loops over contact data', async () => { + const emails = await sendBatch( + { + subject: 'Your cart', + body: '
    {% for item in cart %}
  • {{item.name}}
    • {% endfor %}
', + }, + [{cart: [{name: 'Starter'}, {name: 'Team'}]}], + ); + + expect(emails[0]?.body).toBe('
  • Starter
  • Team
'); + }); + + it('still injects the per-recipient unsubscribe URL', async () => { + const emails = await sendBatch({subject: 'Hi', body: 'Unsubscribe'}, [{}]); + + expect(emails[0]?.body).toContain(`/unsubscribe/${emails[0]?.contactId}`); + }); + }); }); diff --git a/apps/api/src/utils/__tests__/templateValidation.test.ts b/apps/api/src/utils/__tests__/templateValidation.test.ts new file mode 100644 index 000000000..b20336405 --- /dev/null +++ b/apps/api/src/utils/__tests__/templateValidation.test.ts @@ -0,0 +1,64 @@ +import {describe, expect, it} from 'vitest'; + +import {HttpException} from '../../exceptions/index.js'; +import {assertValidTemplateSyntax} from '../templateValidation.js'; + +/** + * Rendering is intentionally forgiving — a broken template still sends, falling back to + * plain placeholder substitution. That makes write time the only place a syntax error + * can be reported, so this guard is what stops a broken `{% if %}` reaching an audience. + */ +describe('assertValidTemplateSyntax', () => { + it('accepts legacy placeholder syntax', () => { + expect(() => + assertValidTemplateSyntax({subject: 'Hi {{firstName ?? there}}', body: '

{{data.plan}}

'}), + ).not.toThrow(); + }); + + it('accepts Liquid conditionals, loops and filters', () => { + expect(() => + assertValidTemplateSyntax({ + subject: '{% if locale == "es" %}Hola{% else %}Hi{% endif %}', + body: '
    {% for item in cart %}
  • {{item.name | upcase}}
    • {% endfor %}
', + }), + ).not.toThrow(); + }); + + it('ignores fields that are absent or empty', () => { + expect(() => assertValidTemplateSyntax({subject: undefined, body: ''})).not.toThrow(); + expect(() => assertValidTemplateSyntax({body: null})).not.toThrow(); + }); + + it('rejects an unclosed tag with a 400 naming the field and position', () => { + let thrown: unknown; + try { + assertValidTemplateSyntax({body: '

ok

\n{% if plan == "pro" %}Pro'}); + } catch (error) { + thrown = error; + } + + expect(thrown).toBeInstanceOf(HttpException); + const exception = thrown as HttpException; + expect(exception.code).toBe(400); + expect(exception.message).toContain('body'); + expect(exception.message).toContain('line 2'); + expect(exception.details).toMatchObject({field: 'body', line: 2}); + }); + + it('rejects an unclosed placeholder', () => { + expect(() => assertValidTemplateSyntax({subject: 'Hi {{firstName'})).toThrow(HttpException); + }); + + it('rejects a filter typo that rendering would silently drop', () => { + expect(() => assertValidTemplateSyntax({body: '{{firstName | upcse}}'})).toThrow(/upcse/); + }); + + it('rejects the file-system tags', () => { + expect(() => assertValidTemplateSyntax({body: "{% render 'secrets' %}"})).toThrow(/not available/); + expect(() => assertValidTemplateSyntax({body: "{% include 'secrets' %}"})).toThrow(/not available/); + }); + + it('reports the first offending field', () => { + expect(() => assertValidTemplateSyntax({subject: '{% if %}', body: 'fine'})).toThrow(/subject/); + }); +}); diff --git a/apps/api/src/utils/templateValidation.ts b/apps/api/src/utils/templateValidation.ts new file mode 100644 index 000000000..21b7c775c --- /dev/null +++ b/apps/api/src/utils/templateValidation.ts @@ -0,0 +1,32 @@ +import {validateTemplate} from '@plunk/shared'; + +import {ErrorCode, HttpException} from '../exceptions/index.js'; + +/** + * Reject template markup the Liquid engine cannot parse. + * + * Rendering is deliberately lenient — a template that fails to parse still sends, + * falling back to plain `{{variable}}` substitution — so authoring time is the only + * place a syntax error can be surfaced. Failing the write is what stops a broken + * `{% if %}` from reaching a whole audience. + */ +export function assertValidTemplateSyntax(fields: Record): void { + for (const [field, source] of Object.entries(fields)) { + if (typeof source !== 'string' || source.length === 0) { + continue; + } + + const result = validateTemplate(source); + + if (!result.valid) { + const position = result.line !== undefined ? ` (line ${result.line}, column ${result.column})` : ''; + + throw new HttpException( + 400, + `Invalid template syntax in ${field}${position}: ${result.error}`, + ErrorCode.VALIDATION_ERROR, + {field, line: result.line, column: result.column}, + ); + } + } +} diff --git a/apps/wiki/content/docs/concepts/templates.mdx b/apps/wiki/content/docs/concepts/templates.mdx index 427cab261..c09d19906 100644 --- a/apps/wiki/content/docs/concepts/templates.mdx +++ b/apps/wiki/content/docs/concepts/templates.mdx @@ -38,7 +38,17 @@ Templates can be created using the built-in editor or by uploading your own HTML ### Personalization -You can use contact data to personalize your templates by using the handlebars syntax `{{ key }}`, where `key` is the contact data key. +You can use contact data to personalize your templates by using the handlebars syntax `{{ key }}`, where `key` is the contact data key. Subjects and bodies are rendered with [Liquid](https://liquidjs.com/), so besides variables like `{{ key }}` you also get conditionals, loops and filters. See [Template language](/guides/template-language) for the full reference. + +```html +

Hi {{firstName ?? 'there'}},

+ +{% if locale == 'es' %} +

Tu plan {{plan | upcase}} se renueva pronto.

+{% else %} +

Your {{plan | upcase}} plan renews soon.

+{% endif %} +``` #### Always Available Variables @@ -60,7 +70,31 @@ You can provide fallback values for variables that might not be set: Hello {{firstName ?? 'there'}}! ``` -If `firstName` is not set, this will render as "Hello there!" +If `firstName` is not set, this will render as "Hello there!" Liquid's `default` filter does the same thing and can fall back to another variable rather than a literal: + +``` +Hello {{firstName | default: nickname}}! +``` + +#### Conditionals, loops and filters + +Anything Liquid supports works in a template — branch on a custom field, loop over an array, or transform a value with a filter: + +```html +{% case plan %} + {% when 'pro' %}

Here's 20% off your renewal.

+ {% when 'free' %}

Upgrade and save 20%.

+ {% else %}

Thanks for being with us.

+{% endcase %} + +
    + {% for item in cartItems %} +
  • {{item.name}} — {{item.price | times: quantity | round: 2}}
    • + {% endfor %} +
+``` + +This lets a single campaign cover combinations that would otherwise need one campaign per segment — e.g. three languages × two offers in one send instead of six. See [Template language](/guides/template-language). #### Special Fields @@ -93,6 +127,9 @@ There are three types of templates in Plunk. Each type is treated at the same pr ## What's next + + Conditionals, loops and filters with Liquid. + Send templated emails directly through `/v1/send`. diff --git a/apps/wiki/content/docs/guides/custom-fields.mdx b/apps/wiki/content/docs/guides/custom-fields.mdx index c996863c9..d7fdccb12 100644 --- a/apps/wiki/content/docs/guides/custom-fields.mdx +++ b/apps/wiki/content/docs/guides/custom-fields.mdx @@ -75,7 +75,7 @@ Plus these core contact fields, which exist outside `data`: `email`, `subscribed ### In templates -Reference any field by name as a Handlebars variable: +Reference any field by name as a template variable: ```html

Hi {{firstName ?? "there"}},

@@ -84,6 +84,16 @@ Reference any field by name as a Handlebars variable: The `?? fallback` syntax is Plunk-specific and lets you provide a default when the field is missing or null. Nested data (`data.profile.tier`) is accessible the same way: `{{profile.tier}}`. +Templates are rendered with Liquid, so custom fields can also drive conditionals, loops and filters: + +```html +{% if plan == 'pro' and lifetimeValue > 500 %} +

Your VIP renewal discount is ready.

+{% endif %} +``` + +Field names built from CSV headers often contain spaces (a "First Name" column becomes `first name`). Reference those directly — `{{first name}}` — or with a bracket lookup when combining with filters: `{{data["first name"] | capitalize}}`. See [Template language](/guides/template-language). + ### In segments Reference custom fields with the `data.` prefix in segment filters: diff --git a/apps/wiki/content/docs/guides/meta.json b/apps/wiki/content/docs/guides/meta.json index b15f1c28f..947bdf419 100644 --- a/apps/wiki/content/docs/guides/meta.json +++ b/apps/wiki/content/docs/guides/meta.json @@ -8,6 +8,7 @@ "idempotency", "localization", "webhooks", + "template-language", "importing-contacts", "custom-fields", "segment-filters", diff --git a/apps/wiki/content/docs/guides/template-language.mdx b/apps/wiki/content/docs/guides/template-language.mdx new file mode 100644 index 000000000..f55d2ac35 --- /dev/null +++ b/apps/wiki/content/docs/guides/template-language.mdx @@ -0,0 +1,190 @@ +--- +title: Template language +description: Use conditionals, loops and filters in subjects and bodies with Liquid +icon: Braces +--- + +Plunk renders email subjects and bodies with [Liquid](https://liquidjs.com/), the template language used by Shopify, Zendesk and Netlify. Every `{{variable}}` placeholder Plunk has always supported keeps working exactly as before — Liquid adds conditionals, loops and filters on top. + +The main thing this buys you is fewer campaigns. Instead of one campaign per segment, a single campaign can cover the whole combinatorial space: three languages × two offers is one send with branching copy rather than six sends to compare. + +## Variables + +Reference any contact field by name. Custom fields under `data` are available both with and without the prefix: + +```html +

Hi {{firstName}}, you are on the {{plan}} plan.

+

Same thing: {{data.firstName}} / {{data.plan}}

+``` + +These are always available regardless of contact data: + +| Variable | Description | +| -------------------- | -------------------------------------- | +| `{{id}}` | The contact's unique identifier | +| `{{email}}` | The contact's email address | +| `{{locale}}` | The contact's preferred locale | +| `{{subscribed}}` | Subscription status | +| `{{unsubscribeUrl}}` | URL to the unsubscribe page | +| `{{subscribeUrl}}` | URL to the subscribe/resubscribe page | +| `{{manageUrl}}` | URL to the preferences management page | + +A variable that isn't set renders as an empty string — it never errors and never leaks the placeholder into the email. + +### Fields whose names contain spaces + +CSV imports build field names from your column headers, so a "First Name" column becomes the field `first name`. Reference it directly, or with a bracket lookup if you want to combine it with filters: + +```html +

Hi {{first name}}

+

Hi {{data["first name"] | capitalize}}

+``` + +## Fallbacks + +`?? fallback` is Plunk-specific shorthand and takes a literal: + +```html +

Hi {{firstName ?? there}}, welcome back.

+``` + +Liquid's `default` filter is the general form and can fall back to another variable: + +```html +

Hi {{firstName | default: nickname}}

+``` + +Both treat a missing, `null`, `false` or empty value as "not set". + +## Conditionals + +`if` / `elsif` / `else`, `unless`, and `case` / `when` all work. This is the multilanguage pattern: + +```html +{% if locale == 'es' %} +

Hola {{firstName ?? cliente}}

+{% elsif locale == 'fr' %} +

Bonjour {{firstName ?? client}}

+{% else %} +

Hi {{firstName ?? there}}

+{% endif %} +``` + +And the segment-specific-offer pattern, without splitting the campaign: + +```html +{% case plan %} + {% when 'pro' %}

Here's 20% off your renewal.

+ {% when 'free' %}

Upgrade now and save 20%.

+ {% else %}

Thanks for being with us.

+{% endcase %} +``` + +Comparison (`==`, `!=`, `>`, `<`, `>=`, `<=`), boolean (`and`, `or`), `contains`, and `blank` / `empty` are all available: + +```html +{% if lifetimeValue > 500 and plan != 'free' %} +

You qualify for our VIP tier.

+{% endif %} +``` + + + Plunk uses JavaScript truthiness rather than Liquid's stricter rules, because imported + contact data very often has a column present but blank. An empty string is falsy, so + `{% if firstName %}` is false for a contact whose `firstName` is `""`. + + +## Loops + +Iterate arrays stored on a contact or passed to `/v1/send`: + +```html +
    + {% for item in cartItems %} +
  • {{forloop.index}}. {{item.name}} — {{item.price}}
    • + {% endfor %} +
+``` + +`forloop.index`, `.index0`, `.first`, `.last` and `.length` are available inside the loop, and `limit` / `offset` / `reversed` work on the `for` tag itself. + +Outputting an array directly — `{{items}}` — wraps each entry in an `
  • ` element. This predates Liquid support and is kept for compatibility; prefer an explicit `{% for %}` so you control the markup. + +## Filters + +Filters transform a value with `|`. The [full LiquidJS filter list](https://liquidjs.com/filters/overview.html) applies; these come up most: + +| Filter | Example | Result | +| ----------------------------------------- | --------------------------------------- | ------------- | +| `upcase` / `downcase` | `{{plan \| upcase}}` | `PRO` | +| `capitalize` | `{{firstName \| capitalize}}` | `Ada` | +| `default` | `{{firstName \| default: "there"}}` | `there` | +| `date` | `{{signupDate \| date: "%B %-d, %Y"}}` | `May 6, 2026` | +| `plus` / `minus` / `times` / `divided_by` | `{{price \| times: 0.8 \| round: 2}}` | `40` | +| `join` | `{{tags \| join: ", "}}` | `a, b` | +| `truncate` | `{{bio \| truncate: 40}}` | truncated | + +For `date` to work, store the value as a full ISO 8601 string (see [Custom fields](/guides/custom-fields)). + +## Computed values + +`assign` and `capture` let you derive a value once and reuse it — useful when an offer depends on something other than a raw field: + +```html +{% assign discount = lifetimeValue | divided_by: 20 %} +{% if discount > 25 %}{% assign discount = 25 %}{% endif %} + +

    Here's {{discount}}% off, calculated from your account history.

    +``` + +## Escaping template syntax + +To show `{{ }}` literally in an email, wrap it in `raw`: + +```html +{% raw %}Use {{firstName}} to personalise your own emails.{% endraw %} +``` + +## What isn't available + +`{% include %}`, `{% render %}` and `{% layout %}` are rejected. + +## Errors + +Saving a template or campaign whose syntax Liquid can't parse returns `400` with the line and column of the problem, so mistakes surface while you're editing. + +Rendering itself is deliberately forgiving — nothing about a template can fail a send: + +- An unknown variable renders empty. +- An unknown filter is skipped and the value passes through unchanged. +- If a stored template somehow can't be parsed at send time, Plunk falls back to plain `{{variable}}` substitution rather than dropping the email. + +## Notes for existing templates + +Templates written before Liquid keep rendering the same output in almost every case — plain placeholders, nested paths, `?? fallback`, missing variables and bare arrays are all unchanged. These are the cases that do render differently: + +**Fixes to previously wrong output** + +- A quoted fallback no longer leaks its quotes. `{{name ?? 'there'}}` used to render `'there'` — quotes included — and now renders `there`, which is what this guide always described. +- Falsy values render instead of disappearing. `0`, `false` and `NaN` used to come out as an empty string; `{{count}}` for a contact with `count: 0` now renders `0`, and `{{subscribed}}` renders `false`. Use `{% unless subscribed %}` to branch on it. +- A fallback containing `??` inside quotes is no longer truncated at the first `??`. +- Placeholders spanning multiple lines, and array indexes like `{{items.0}}`, now resolve; both used to render as literal text or empty. + +**Behaviour to check before upgrading** + +- Balanced tag markup in body copy is now executed. Prose like `Write {% if x %}…{% endif %} to branch` used to print verbatim and now evaluates to nothing. Wrap it in `{% raw %}` to keep it as text. Unbalanced markup — `Use {% if x %} in docs` — still fails to parse and survives untouched. +- A custom field whose name contains `|`, quotes or other operator characters is now parsed as an expression. Field names made of letters, digits, `_`, `-`, `.` and spaces are unaffected. + +## What's next + + + + Store the data your templates branch on. + + + How templates fit into campaigns, workflows and `/v1/send`. + + + Set a `locale` per contact to drive multilanguage templates. + + diff --git a/packages/shared/package.json b/packages/shared/package.json index 0979293c9..0ca9a745c 100644 --- a/packages/shared/package.json +++ b/packages/shared/package.json @@ -15,6 +15,7 @@ }, "dependencies": { "@plunk/types": "*", + "liquidjs": "^10.27.2", "zod": "^3.23.8" }, "exports": { diff --git a/packages/shared/src/__tests__/template.test.ts b/packages/shared/src/__tests__/template.test.ts new file mode 100644 index 000000000..8a5581337 --- /dev/null +++ b/packages/shared/src/__tests__/template.test.ts @@ -0,0 +1,402 @@ +import {beforeEach, describe, expect, it, vi} from 'vitest'; + +import {clearTemplateCache, compileTemplate, renderTemplate, validateTemplate} from '../template/index.js'; + +/** + * The template engine is Liquid (see issue #426), replacing a regex-based + * `{{variable}}` substitution. Two things matter equally here: that the Liquid + * features campaigns need actually work, and that every template written against the + * old syntax keeps rendering byte for byte. + */ +describe('renderTemplate', () => { + beforeEach(() => { + clearTemplateCache(); + }); + + // ======================================== + // BACKWARDS COMPATIBILITY WITH {{variable}} + // ======================================== + describe('legacy placeholder syntax', () => { + it('substitutes a top-level variable', () => { + expect(renderTemplate('Hello {{name}}!', {name: 'World'})).toBe('Hello World!'); + }); + + it('substitutes a nested path', () => { + expect(renderTemplate('Hello {{data.firstName}}!', {data: {firstName: 'Ada'}})).toBe('Hello Ada!'); + }); + + it('resolves a key nested under data without the prefix', () => { + expect(renderTemplate('Hello {{firstName}}!', {data: {firstName: 'Ada'}})).toBe('Hello Ada!'); + }); + + it('prefers a top-level key over the same key under data', () => { + expect(renderTemplate('{{plan}}', {plan: 'pro', data: {plan: 'free'}})).toBe('pro'); + }); + + it('renders an unquoted ?? fallback when the value is missing', () => { + expect(renderTemplate('Hello {{name ?? Guest}}!', {})).toBe('Hello Guest!'); + }); + + it('renders a quoted ?? fallback when the value is missing', () => { + expect(renderTemplate("Hello {{firstName ?? 'there'}}!", {})).toBe('Hello there!'); + }); + + it('ignores the ?? fallback when the value is present', () => { + expect(renderTemplate('Hello {{firstName ?? there}}!', {firstName: 'Ada'})).toBe('Hello Ada!'); + }); + + it('treats the ?? fallback as a literal, not a variable reference', () => { + expect(renderTemplate('{{plan ?? free}}', {free: 'SHOULD NOT APPEAR'})).toBe('free'); + }); + + it('uses the first of several ?? fallbacks, as the old renderer did', () => { + // Fallbacks are literals, so the first one is always non-empty and wins. + expect(renderTemplate('{{a ?? b ?? c}}', {})).toBe('b'); + }); + + it('keeps a fallback containing spaces intact', () => { + expect(renderTemplate('{{title ?? Dear customer}}', {})).toBe('Dear customer'); + }); + + it('renders a missing variable as an empty string', () => { + expect(renderTemplate('Hello {{missing}}!', {})).toBe('Hello !'); + }); + + it('renders a missing nested path as an empty string', () => { + expect(renderTemplate('[{{a.b.c}}]', {a: {}})).toBe('[]'); + }); + + it('renders a bare array as HTML list items', () => { + expect(renderTemplate('{{items}}', {items: ['one', 'two']})).toBe('
  • one
    • \n
  • two
    • '); + }); + + it('resolves keys containing spaces, which CSV imports produce from headers', () => { + expect(renderTemplate('Hi {{first name}}!', {'first name': 'Ada'})).toBe('Hi Ada!'); + }); + + it('resolves a nested key containing spaces', () => { + expect(renderTemplate('{{profile.first name}}', {profile: {'first name': 'Ada'}})).toBe('Ada'); + }); + + it('renders an empty placeholder as an empty string', () => { + expect(renderTemplate('[{{}}][{{ }}]', {})).toBe('[][]'); + }); + + it('renders numbers and objects the way the old renderer did', () => { + expect(renderTemplate('{{count}}', {count: 42})).toBe('42'); + expect(renderTemplate('[{{o}}]', {o: {a: 1}})).toBe('[[object Object]]'); + }); + + it('leaves unrelated braces alone', () => { + expect(renderTemplate('', {})).toBe(''); + }); + + it('leaves a double brace it cannot resolve alone, as before', () => { + expect(renderTemplate('', {})).toBe(''); + }); + }); + + // ======================================== + // DELIBERATE DIVERGENCES FROM THE OLD RENDERER + // ======================================== + describe('intentional differences from the previous renderer', () => { + beforeEach(() => { + vi.spyOn(console, 'warn').mockImplementation(() => undefined); + }); + + it('strips the quotes from a quoted ?? fallback', () => { + // The old renderer split on `??` and returned the raw text, so the quotes ended up + // in the email: `Hi {{name ?? 'there'}}` rendered as `Hi 'there'`. The documented + // behaviour has always been `Hi there`, which is what Liquid's `default:` gives. + expect(renderTemplate("Hi {{name ?? 'there'}}", {})).toBe('Hi there'); + expect(renderTemplate('Hi {{name ?? "there"}}', {})).toBe('Hi there'); + }); + + it('renders falsy values instead of blanking them', () => { + // The old lookup chained with `||`, so any falsy value fell through to the + // fallback and rendered empty. `{{subscribed}}` is documented as rendering + // true/false, which only works now. + expect(renderTemplate('[{{count}}]', {count: 0})).toBe('[0]'); + expect(renderTemplate('[{{subscribed}}]', {subscribed: false})).toBe('[false]'); + expect(renderTemplate('[{{a.b}}]', {a: {b: 0}})).toBe('[0]'); + }); + + it('no longer truncates a ?? fallback at a ?? inside quotes', () => { + expect(renderTemplate('{{name ?? "a ?? b"}}', {})).toBe('a ?? b'); + }); + + it('resolves placeholders that span lines and array indexes', () => { + // Both rendered as literal text / empty before. + expect(renderTemplate('{{\n name \n}}', {name: 'Ada'})).toBe('Ada'); + expect(renderTemplate('[{{items.0}}]', {items: ['a', 'b']})).toBe('[a]'); + }); + + it('evaluates balanced tag markup that used to be literal body copy', () => { + // The main upgrade risk: prose that happens to contain balanced Liquid markup is + // now executed rather than printed. Unbalanced markup still fails to parse and + // falls back, so it survives unchanged. + expect(renderTemplate('Docs: {% if x %}shown{% endif %} end', {})).toBe('Docs: end'); + expect(renderTemplate('Use {% if x %} in docs', {})).toBe('Use {% if x %} in docs'); + }); + }); + + // ======================================== + // LIQUID FEATURES REQUESTED IN THE TICKET + // ======================================== + describe('conditionals', () => { + it('branches on a contact field, the multilanguage case', () => { + const template = `{% if locale == 'es' %}Hola{% elsif locale == 'fr' %}Bonjour{% else %}Hello{% endif %}`; + + expect(renderTemplate(template, {locale: 'es'})).toBe('Hola'); + expect(renderTemplate(template, {locale: 'fr'})).toBe('Bonjour'); + expect(renderTemplate(template, {locale: 'de'})).toBe('Hello'); + expect(renderTemplate(template, {})).toBe('Hello'); + }); + + it('supports case/when', () => { + const template = `{% case plan %}{% when 'pro' %}20% off{% when 'free' %}Upgrade{% else %}Thanks{% endcase %}`; + + expect(renderTemplate(template, {plan: 'pro'})).toBe('20% off'); + expect(renderTemplate(template, {plan: 'free'})).toBe('Upgrade'); + expect(renderTemplate(template, {plan: 'enterprise'})).toBe('Thanks'); + }); + + it('supports comparison and boolean operators', () => { + expect(renderTemplate('{% if ltv > 100 and plan == "pro" %}VIP{% endif %}', {ltv: 240, plan: 'pro'})).toBe('VIP'); + expect(renderTemplate('{% if ltv > 100 and plan == "pro" %}VIP{% endif %}', {ltv: 40, plan: 'pro'})).toBe(''); + expect(renderTemplate('{% unless subscribed %}Resubscribe{% endunless %}', {subscribed: false})).toBe( + 'Resubscribe', + ); + }); + + it('treats a blank custom field as falsy', () => { + // Contact data comes from CSV imports where "column present but empty" is the + // norm, so `jsTruthy` is enabled rather than Liquid's Shopify-compatible default. + expect(renderTemplate('{% if firstName %}Hi {{firstName}}{% else %}Hi there{% endif %}', {firstName: ''})).toBe( + 'Hi there', + ); + }); + }); + + describe('loops', () => { + it('iterates an array of primitives', () => { + const template = '{% for item in items %}
  • {{item}}
    • {% endfor %}'; + + expect(renderTemplate(template, {items: ['a', 'b']})).toBe('
  • a
  • b
    • '); + }); + + it('iterates an array of objects with forloop metadata', () => { + const template = '{% for p in products %}{{forloop.index}}. {{p.name}} — {{p.price}}\n{% endfor %}'; + + expect(renderTemplate(template, {products: [{name: 'Pro', price: 10}, {name: 'Team', price: 20}]})).toBe( + '1. Pro — 10\n2. Team — 20\n', + ); + }); + + it('renders nothing for an empty or missing collection', () => { + expect(renderTemplate('{% for i in items %}x{% endfor %}', {items: []})).toBe(''); + expect(renderTemplate('{% for i in items %}x{% endfor %}', {})).toBe(''); + }); + }); + + describe('filters', () => { + it('applies string and number filters', () => { + expect(renderTemplate('{{name | upcase}}', {name: 'ada'})).toBe('ADA'); + expect(renderTemplate('{{tags | join: ", "}}', {tags: ['a', 'b']})).toBe('a, b'); + expect(renderTemplate('{{price | times: 0.8 | round: 2}}', {price: 50})).toBe('40'); + }); + + it('applies the default filter, the modern form of ??', () => { + expect(renderTemplate('{{firstName | default: "there"}}', {})).toBe('there'); + }); + + it('formats dates', () => { + expect(renderTemplate('{{signupDate | date: "%Y-%m"}}', {signupDate: '2026-05-06T12:00:00Z'})).toBe('2026-05'); + }); + + it('skips an unknown filter rather than failing the send', () => { + expect(renderTemplate('{{name | upcse}}', {name: 'ada'})).toBe('ada'); + }); + }); + + describe('assignment', () => { + it('supports assign for derived values, the pricing case', () => { + const template = + '{% assign discount = ltv | divided_by: 10 %}{% if discount > 20 %}20{% else %}{{discount}}{% endif %}% off'; + + expect(renderTemplate(template, {ltv: 150})).toBe('15% off'); + expect(renderTemplate(template, {ltv: 900})).toBe('20% off'); + }); + + it('supports capture', () => { + expect(renderTemplate('{% capture greeting %}Hi {{name}}{% endcapture %}{{greeting}}!', {name: 'Ada'})).toBe( + 'Hi Ada!', + ); + }); + }); + + describe('whitespace control and comments', () => { + it('trims with the dash markers', () => { + expect(renderTemplate('a{%- if true -%} b {%- endif -%}c', {})).toBe('abc'); + expect(renderTemplate('[{{- name -}}]', {name: 'x'})).toBe('[x]'); + }); + + it('drops comment blocks', () => { + expect(renderTemplate('a{% comment %}note{% endcomment %}b', {})).toBe('ab'); + }); + + it('emits raw blocks verbatim', () => { + expect(renderTemplate('{% raw %}{{ not a variable }}{% endraw %}', {'not a variable': 'x'})).toBe( + '{{ not a variable }}', + ); + }); + }); + + // ======================================== + // MARKUP TYPED IN THE RICH-TEXT EDITOR + // ======================================== + describe('HTML-escaped markup', () => { + it('decodes comparison operators escaped by the editor', () => { + expect(renderTemplate('{% if age > 18 %}adult{% endif %}', {age: 21})).toBe('adult'); + expect(renderTemplate('{% if age <= 18 %}minor{% endif %}', {age: 12})).toBe('minor'); + }); + + it('decodes escaped quotes and non-breaking spaces inside delimiters', () => { + expect(renderTemplate('{% if plan == "pro" %}Pro{% endif %}', {plan: 'pro'})).toBe('Pro'); + expect(renderTemplate('{% if plan == 'pro' %}Pro{% endif %}', {plan: 'pro'})).toBe('Pro'); + }); + + it('does not decode entities outside delimiters', () => { + expect(renderTemplate('Tom & Jerry {{name}}', {name: 'x'})).toBe('Tom & Jerry x'); + }); + }); + + // ======================================== + // SANDBOXING + // ======================================== + describe('sandboxing', () => { + beforeEach(() => { + vi.spyOn(console, 'warn').mockImplementation(() => undefined); + }); + + it.each(['include', 'render', 'layout'])('refuses the {%% %s %%} tag, which reads from disk', tag => { + const template = `{% ${tag} 'package.json' %}`; + const result = validateTemplate(template); + + expect(result.valid).toBe(false); + expect(result.error).toContain(`{% ${tag} %} tag is not available`); + // The file's contents must never reach the rendered email. + expect(renderTemplate(template, {})).not.toContain('"dependencies"'); + }); + + it('does not expose prototype members of contact data', () => { + expect(renderTemplate('[{{profile.constructor}}][{{profile.__proto__}}]', {profile: {}})).toBe('[][]'); + }); + + it('aborts a runaway loop instead of blocking the worker', () => { + const result = renderTemplate('{% for i in (1..100000000) %}x{% endfor %}', {}); + + // Falls back to legacy substitution, which leaves the tag as literal text. + expect(result).toContain('{% for i in (1..100000000) %}'); + expect(console.warn).toHaveBeenCalled(); + }); + }); + + // ======================================== + // ERROR HANDLING + // ======================================== + describe('malformed templates', () => { + beforeEach(() => { + vi.spyOn(console, 'warn').mockImplementation(() => undefined); + }); + + it('never throws', () => { + expect(() => renderTemplate('{% if %}{% endfor %}', {})).not.toThrow(); + expect(() => renderTemplate('{{ | | }}', {})).not.toThrow(); + }); + + it('falls back to placeholder substitution when parsing fails', () => { + // An unterminated `{{` used to be left as literal text; it still is, and the + // well-formed placeholder next to it still resolves. + expect(renderTemplate('{{name}} and {{ unclosed', {name: 'Ada'})).toBe('Ada and {{ unclosed'); + }); + + it('logs once per template rather than once per recipient', () => { + const template = '{% if %}'; + + for (let i = 0; i < 100; i += 1) { + renderTemplate(template, {}); + } + + expect(console.warn).toHaveBeenCalledTimes(1); + }); + }); +}); + +describe('compileTemplate', () => { + beforeEach(() => { + clearTemplateCache(); + }); + + it('renders one parsed template against many recipients', () => { + const compiled = compileTemplate('Hi {{name}}, you are on {{plan}}.'); + + expect(compiled.valid).toBe(true); + expect(compiled.render({name: 'Ada', plan: 'pro'})).toBe('Hi Ada, you are on pro.'); + expect(compiled.render({name: 'Bob', plan: 'free'})).toBe('Hi Bob, you are on free.'); + }); + + it('does not leak state between renders', () => { + const compiled = compileTemplate('{% assign total = price | plus: 10 %}{{total}}'); + + expect(compiled.render({price: 1})).toBe('11'); + expect(compiled.render({price: 2})).toBe('12'); + }); + + it('exposes the original source and reports an invalid template', () => { + vi.spyOn(console, 'warn').mockImplementation(() => undefined); + const compiled = compileTemplate('{% if %}'); + + expect(compiled.source).toBe('{% if %}'); + expect(compiled.valid).toBe(false); + expect(compiled.render({})).toBe('{% if %}'); + }); +}); + +describe('validateTemplate', () => { + beforeEach(() => { + clearTemplateCache(); + }); + + it('accepts legacy and Liquid syntax', () => { + expect(validateTemplate('Hi {{firstName ?? there}}!')).toEqual({valid: true}); + expect(validateTemplate('{% if locale == "es" %}Hola{% endif %}')).toEqual({valid: true}); + expect(validateTemplate('{{first name}}')).toEqual({valid: true}); + expect(validateTemplate('')).toEqual({valid: true}); + }); + + it('reports an unclosed tag with its position', () => { + const result = validateTemplate('line one\n{% if plan == "pro" %}Pro'); + + expect(result.valid).toBe(false); + expect(result.error).toBeTruthy(); + expect(result.error).not.toMatch(/line:\d+/); + expect(result.line).toBe(2); + expect(result.column).toBeGreaterThan(0); + }); + + it('reports an unclosed placeholder', () => { + expect(validateTemplate('Hi {{ name').valid).toBe(false); + }); + + it('reports an unknown filter, which rendering would silently skip', () => { + const result = validateTemplate('{{name | upcse}}'); + + expect(result.valid).toBe(false); + expect(result.error).toContain('upcse'); + }); + + it('reports an unknown tag', () => { + expect(validateTemplate('{% loop %}x{% endloop %}').valid).toBe(false); + }); +}); diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index 530387abb..acee767df 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -1,5 +1,5 @@ export * from './schemas/index.js'; export * from './operators.js'; -export * from './template.js'; +export * from './template/index.js'; export * from './i18n/index.js'; export * from './unsubscribe.js'; diff --git a/packages/shared/src/template/engine.ts b/packages/shared/src/template/engine.ts new file mode 100644 index 000000000..d07ac28b7 --- /dev/null +++ b/packages/shared/src/template/engine.ts @@ -0,0 +1,92 @@ +import {Liquid} from 'liquidjs'; + +/** + * Tags that resolve a partial by name through the file system. Template bodies are + * authored by users, so leaving these enabled turns every template into an arbitrary + * file read on the worker's disk (`{% render '../../.env' %}`). We both remove the + * file system (`templates: {}` shadows the `root`/`partials`/`layouts` lookups) and + * replace the tags, so authors get a clear message instead of a confusing ENOENT. + */ +const BLOCKED_TAGS = ['include', 'render', 'layout'] as const; + +/** + * DoS ceilings. Templates are user input and are rendered once per recipient, so a + * single pathological template must not be able to stall a worker. The limits are far + * above anything a real email needs — a 60 KB template renders in tens of + * microseconds, so a 1 s render budget is roughly four orders of magnitude of slack. + */ +export const TEMPLATE_PARSE_LIMIT = 2_000_000; +export const TEMPLATE_RENDER_LIMIT_MS = 1_000; +export const TEMPLATE_MEMORY_LIMIT = 10_000_000; + +/** + * Key under which the render scope references itself. Liquid has no syntax for + * looking up a top-level key that contains a space, and CSV imports routinely produce + * them (a "First Name" header becomes the key `first name`). Templates referencing + * those keys are rewritten to `__plunk["first name"]` during preprocessing. + */ +export const SCOPE_ALIAS = '__plunk'; + +/** + * Preserves the pre-Liquid behaviour of outputting a bare array as an HTML list. + * + * Liquid joins arrays without a separator, which would silently mangle templates that + * relied on `{{ items }}` rendering `
  • ` elements. Registered as Liquid's + * `outputEscape` hook, which appends itself to every output's filter chain — so + * `{{ items | raw }}` opts out and `{% for %}` is unaffected. + */ +function renderArrayAsListItems(value: unknown): string { + if (!Array.isArray(value)) { + // Not necessarily a string: Liquid stringifies whatever we hand back, and + // deferring to it keeps Drops, dates and numbers formatted exactly as usual. + return value as string; + } + + return value.map(item => `
  • ${item ?? ''}
    • `).join('\n'); +} + +function createEngine({strictFilters}: {strictFilters: boolean}): Liquid { + const engine = new Liquid({ + // No file system access whatsoever — see BLOCKED_TAGS. + templates: {}, + relativeReference: false, + // JavaScript truthiness. Contact data arrives from CSV imports and API payloads + // where "key present but blank" is the norm, and Liquid's Shopify-compatible + // truthiness would make `{% if firstName %}` true for an empty string. + jsTruthy: true, + // Contact data is unsanitised user input; never walk its prototype chain. + ownPropertyOnly: true, + // An unknown variable renders as an empty string, matching the previous + // regex-based renderer. Templates are written against per-contact custom fields + // that legitimately differ from contact to contact, so this can't be strict. + strictVariables: false, + strictFilters, + parseLimit: TEMPLATE_PARSE_LIMIT, + renderLimit: TEMPLATE_RENDER_LIMIT_MS, + memoryLimit: TEMPLATE_MEMORY_LIMIT, + outputEscape: renderArrayAsListItems, + }); + + for (const name of BLOCKED_TAGS) { + engine.registerTag(name, { + parse() { + throw new Error(`The {% ${name} %} tag is not available in Plunk templates`); + }, + render() { + return ''; + }, + }); + } + + return engine; +} + +/** Engine used for every send. Lenient: unknown filters and variables are skipped. */ +export const renderEngine = createEngine({strictFilters: false}); + +/** + * Engine used by `validateTemplate` at authoring time. Identical except that unknown + * filters raise, so a typo like `{{ name | upcse }}` is reported while the author is + * still editing rather than silently dropping the value from every email. + */ +export const validationEngine = createEngine({strictFilters: true}); diff --git a/packages/shared/src/template/index.ts b/packages/shared/src/template/index.ts new file mode 100644 index 000000000..19a09c628 --- /dev/null +++ b/packages/shared/src/template/index.ts @@ -0,0 +1,198 @@ +import {LiquidError, type Template} from 'liquidjs'; + +import {renderEngine, SCOPE_ALIAS, validationEngine} from './engine.js'; +import {renderLegacyTemplate} from './legacy.js'; +import {preprocessTemplate} from './preprocess.js'; + +/** A template parsed once and ready to be rendered against many recipients. */ +export interface CompiledTemplate { + /** The original, unpreprocessed template string. */ + readonly source: string; + /** + * Whether the source parsed as Liquid. A template that failed to parse still renders + * — through the legacy placeholder renderer — so sends never break on a bad template. + */ + readonly valid: boolean; + render(variables: Record): string; +} + +export interface TemplateValidationResult { + valid: boolean; + /** Human-readable description of the first syntax error, if any. */ + error?: string; + line?: number; + column?: number; +} + +/** + * Parsed templates are cached so the existing per-email call sites (which pass the same + * campaign body once per recipient) pay the parse cost once per worker rather than once + * per send. Bounded on both entry count and template size to keep worker memory flat. + */ +const MAX_CACHE_ENTRIES = 32; +const MAX_CACHEABLE_LENGTH = 200_000; + +/** Cap on distinct templates we remember having warned about. */ +const MAX_REPORTED_SOURCES = 64; + +type ParseResult = {templates: Template[]; error?: undefined} | {templates?: undefined; error: Error}; + +const parseCache = new Map(); + +/** Bounded set of template sources already reported, so a bad template logs once. */ +const reportedSources = new Set(); + +function reportOnce(source: string, stage: string, error: unknown): void { + if (reportedSources.has(source)) { + return; + } + + if (reportedSources.size >= MAX_REPORTED_SOURCES) { + reportedSources.clear(); + } + reportedSources.add(source); + + const message = error instanceof Error ? error.message : String(error); + // console rather than signale: @plunk/shared also runs in the browser (editor preview). + console.warn(`[TEMPLATE] Failed to ${stage} template, falling back to plain variable substitution: ${message}`); +} + +function parse(source: string): ParseResult { + try { + return {templates: renderEngine.parse(preprocessTemplate(source))}; + } catch (error) { + reportOnce(source, 'parse', error); + return {error: error instanceof Error ? error : new Error(String(error))}; + } +} + +function parseCached(source: string): ParseResult { + const cached = parseCache.get(source); + if (cached) { + // Re-insert to mark as most recently used. + parseCache.delete(source); + parseCache.set(source, cached); + return cached; + } + + const result = parse(source); + + if (source.length <= MAX_CACHEABLE_LENGTH) { + if (parseCache.size >= MAX_CACHE_ENTRIES) { + const oldest = parseCache.keys().next().value; + if (oldest !== undefined) { + parseCache.delete(oldest); + } + } + parseCache.set(source, result); + } + + return result; +} + +function isPlainObject(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +/** + * Flatten the render scope. + * + * Callers pass contact fields both spread at the top level and nested under `data`. + * Merging keeps both `{{firstName}}` and `{{data.firstName}}` working, with top-level + * keys winning — the same precedence the old renderer used. `SCOPE_ALIAS` lets + * preprocessed lookups reach keys whose names contain spaces. + */ +function buildScope(variables: Record): Record { + const nested = variables.data; + const scope: Record = isPlainObject(nested) ? {...nested, ...variables} : {...variables}; + scope[SCOPE_ALIAS] = scope; + return scope; +} + +/** + * Parse a template once so it can be rendered for many recipients. + * + * Never throws: a template that fails to parse falls back to plain `{{variable}}` + * substitution, and `valid` reports which path it took. Use `validateTemplate` to + * surface syntax errors to the author. + */ +export function compileTemplate(source: string): CompiledTemplate { + const parsed = parseCached(source); + + if (parsed.error) { + return { + source, + valid: false, + render: variables => renderLegacyTemplate(source, variables), + }; + } + + return { + source, + valid: true, + render: variables => { + try { + return String(renderEngine.renderSync(parsed.templates, buildScope(variables))); + } catch (error) { + // A render error means a runtime limit was hit (e.g. an unbounded loop). + reportOnce(source, 'render', error); + return renderLegacyTemplate(source, variables); + } + }, + }; +} + +/** + * Render an email template. + * + * Templates are Liquid, so conditionals, loops and filters are available on top of the + * `{{variable}}` and `{{variable ?? fallback}}` syntax Plunk has always supported: + * + * renderTemplate('Hello {{name}}!', {name: 'World'}) -> 'Hello World!' + * renderTemplate('Hello {{data.name}}!', {data: {name: 'World'}}) -> 'Hello World!' + * renderTemplate('Hello {{name ?? Guest}}!', {}) -> 'Hello Guest!' + * renderTemplate('{% if plan == "pro" %}Pro{% endif %}', {plan: 'pro'}) -> 'Pro' + * + * Never throws. For a template rendered against many recipients, prefer + * `compileTemplate` to hoist the parse out of the loop. + */ +export function renderTemplate(template: string, variables: Record): string { + return compileTemplate(template).render(variables); +} + +/** + * Check a template for syntax errors, for use at authoring time. + * + * Stricter than rendering: unknown filters are reported here but skipped at send time, + * so saving a template can flag a typo without ever blocking a send. + * + * Positions come from the preprocessed source. Preprocessing never adds or removes + * newlines, so `line` always matches the author's template; `column` can be off by the + * length of a rewrite earlier on the same line. + */ +export function validateTemplate(source: string): TemplateValidationResult { + try { + validationEngine.parse(preprocessTemplate(source)); + return {valid: true}; + } catch (error) { + if (LiquidError.is(error)) { + const [line, column] = error.token.getPosition(); + return { + valid: false, + // Liquid appends ", line:N, col:M" to the message; the position is returned + // separately so callers can format it themselves. + error: error.message.replace(/, line:\d+, col:\d+$/, ''), + line, + column, + }; + } + + return {valid: false, error: error instanceof Error ? error.message : String(error)}; + } +} + +/** Test seam: drop parsed templates and warning state. */ +export function clearTemplateCache(): void { + parseCache.clear(); + reportedSources.clear(); +} diff --git a/packages/shared/src/template.ts b/packages/shared/src/template/legacy.ts similarity index 51% rename from packages/shared/src/template.ts rename to packages/shared/src/template/legacy.ts index b1198b24e..528e9cc75 100644 --- a/packages/shared/src/template.ts +++ b/packages/shared/src/template/legacy.ts @@ -1,22 +1,25 @@ /** - * Render email template by replacing variables - * Supports {{variable}} and {{variable ?? defaultValue}} syntax - * Also supports nested access like {{data.firstName}} + * The pre-Liquid renderer, kept as a safety net. * - * Example: - * renderTemplate('Hello {{name}}!', { name: 'World' }) -> 'Hello World!' - * renderTemplate('Hello {{data.name}}!', { data: { name: 'World' } }) -> 'Hello World!' - * renderTemplate('Hello {{name ?? Guest}}!', {}) -> 'Hello Guest!' + * Templates are authored by users and stored indefinitely, so a template that Liquid + * refuses to parse must not take a campaign down with it. When parsing fails, + * `renderTemplate` falls back to this function: plain `{{variable}}` and + * `{{variable ?? default}}` placeholders still resolve, and anything Liquid-specific is + * left as literal text. `validateTemplate` is what surfaces the actual error to the + * author. + * + * Supports `{{variable}}`, `{{variable ?? defaultValue}}` and nested access + * (`{{data.firstName}}`). */ -export function renderTemplate(template: string, variables: Record): string { - return template.replace(/\{\{(.*?)\}\}/g, (match, key) => { +export function renderLegacyTemplate(template: string, variables: Record): string { + return template.replace(/\{\{(.*?)\}\}/g, (_match, key) => { const [mainKey, defaultValue] = key.split('??').map((s: string) => s.trim()); // Handle nested property access (e.g., data.firstName) const getValue = (obj: Record, path: string): unknown => { - return path.split('.').reduce((current: Record | unknown, key) => { + return path.split('.').reduce((current: Record | unknown, segment) => { if (current && typeof current === 'object' && !Array.isArray(current)) { - return (current as Record)[key]; + return (current as Record)[segment]; } return undefined; }, obj); @@ -35,4 +38,4 @@ export function renderTemplate(template: string, variables: Record 18 %}` + * reaches the API as `{% if age > 18 %}` and fails to tokenize. Inside the + * delimiters the content is code rather than prose, so decoding is always the intent. + */ +const ENTITIES: Record = { + '<': '<', + '>': '>', + '"': '"', + '"': '"', + ''': "'", + ''': "'", + '&': '&', + ' ': ' ', + ' ': ' ', +}; + +// A single pass, so `&gt;` correctly decodes to `>` rather than to `>`. +const ENTITY_PATTERN = /&(?:lt|gt|quot|apos|amp|nbsp|#34|#39|#160);/g; + +/** + * A plain variable path and nothing else — no filters, operators, quotes or brackets. + * Used to detect the legacy `{{first name}}` form, which Liquid cannot express. + */ +const BARE_PATH = /^[A-Za-z_][\w .-]*$/; + +/** `{% raw %}` content must survive verbatim, so preprocessing skips over it. */ +const RAW_TAG = /^\s*raw\s*$/; +const END_RAW_TAG = /\{%-?\s*endraw\s*-?%\}/; + +function decodeEntities(source: string): string { + return source.includes('&') ? source.replace(ENTITY_PATTERN, entity => ENTITIES[entity] ?? entity) : source; +} + +/** + * Peel off Liquid's whitespace-control markers so the expression in between can be + * rewritten without losing them: `{{- name -}}` -> `-`, `name`, `-`. + */ +function splitTrimMarkers(inner: string): {prefix: string; body: string; suffix: string} { + let prefix = ''; + let suffix = ''; + let body = inner; + + if (body.startsWith('-') || body.startsWith('+')) { + prefix = body.slice(0, 1); + body = body.slice(1); + } + + if (body.endsWith('-') || body.endsWith('+')) { + suffix = body.slice(-1); + body = body.slice(0, -1); + } + + return {prefix, body, suffix}; +} + +/** Split an expression on top-level `??`, ignoring occurrences inside string literals. */ +function splitDefaults(expression: string): string[] { + if (!expression.includes('??')) { + return [expression]; + } + + const parts: string[] = []; + let start = 0; + let quote: string | undefined; + + for (let index = 0; index < expression.length; index += 1) { + const char = expression[index]; + + if (quote) { + if (char === quote) { + quote = undefined; + } + continue; + } + + if (char === '"' || char === "'") { + quote = char; + continue; + } + + if (char === '?' && expression[index + 1] === '?') { + parts.push(expression.slice(start, index)); + index += 1; + start = index + 1; + } + } + + parts.push(expression.slice(start)); + return parts; +} + +/** + * Quote a legacy `?? fallback` operand. The old renderer always treated the fallback + * as a literal string, so it stays quoted rather than being resolved as a variable — + * `{{plan ?? free}}` keeps rendering "free" and not the (missing) `free` variable. + * Authors who want a variable fallback can use Liquid's `| default:` directly. + */ +function toLiquidLiteral(raw: string): string { + const value = raw.trim(); + + const alreadyQuoted = + value.length >= 2 && + ((value.startsWith('"') && value.endsWith('"')) || (value.startsWith("'") && value.endsWith("'"))); + if (alreadyQuoted) { + return value; + } + + if (!value.includes('"')) { + return `"${value}"`; + } + if (!value.includes("'")) { + return `'${value}'`; + } + + // LiquidJS string literals have no escape sequences, so a fallback containing both + // quote styles keeps its double quotes as an entity. + return `"${value.replace(/"/g, '"')}"`; +} + +/** + * Route a key containing spaces through the scope alias. `{{first name}}` becomes + * `{{ __plunk["first name"] }}` and `{{profile.first name}}` becomes + * `{{ __plunk["profile"]["first name"] }}`, mirroring how the old renderer resolved + * dotted paths segment by segment. + */ +function rewriteSpacedPath(expression: string): string { + const path = expression.trim(); + + if (!path.includes(' ') || !BARE_PATH.test(path)) { + return expression; + } + + const segments = path + .split('.') + .map(segment => segment.trim()) + .filter(segment => segment.length > 0); + + if (segments.length === 0) { + return expression; + } + + return SCOPE_ALIAS + segments.map(segment => `[${JSON.stringify(segment)}]`).join(''); +} + +function rewriteOutput(inner: string): string { + const decoded = decodeEntities(inner); + const {prefix, body, suffix} = splitTrimMarkers(decoded); + + // The old renderer replaced `{{}}` with an empty string; Liquid rejects it outright. + if (body.trim().length === 0) { + return ''; + } + + const [head = '', ...fallbacks] = splitDefaults(body); + const expression = rewriteSpacedPath(head).trim(); + + if (decoded === inner && fallbacks.length === 0 && expression === head.trim()) { + return `${OUTPUT_OPEN}${inner}${OUTPUT_CLOSE}`; + } + + const defaults = fallbacks.map(fallback => ` | default: ${toLiquidLiteral(fallback)}`).join(''); + return `${OUTPUT_OPEN}${prefix} ${expression}${defaults} ${suffix}${OUTPUT_CLOSE}`; +} + +/** + * Bridge Plunk's historic template syntax to Liquid, and undo the escaping the + * rich-text editor applies to markup typed inside `{{ }}` / `{% %}`. + * + * Runs once per template at parse time, never per recipient. + */ +export function preprocessTemplate(source: string): string { + if (!source.includes(OUTPUT_OPEN) && !source.includes(TAG_OPEN)) { + return source; + } + + let result = ''; + let index = 0; + + while (index < source.length) { + const outputAt = source.indexOf(OUTPUT_OPEN, index); + const tagAt = source.indexOf(TAG_OPEN, index); + + if (outputAt === -1 && tagAt === -1) { + break; + } + + const isOutput = tagAt === -1 || (outputAt !== -1 && outputAt < tagAt); + const openAt = isOutput ? outputAt : tagAt; + const closeAt = source.indexOf(isOutput ? OUTPUT_CLOSE : TAG_CLOSE, openAt + 2); + + if (closeAt === -1) { + // Unterminated delimiter: leave the remainder untouched and let Liquid report it. + break; + } + + result += source.slice(index, openAt); + const inner = source.slice(openAt + 2, closeAt); + + if (isOutput) { + result += rewriteOutput(inner); + index = closeAt + 2; + continue; + } + + const {body} = splitTrimMarkers(inner); + + if (RAW_TAG.test(body)) { + // Copy `{% raw %}...{% endraw %}` through byte for byte. + const rest = source.slice(closeAt + 2); + const endRaw = END_RAW_TAG.exec(rest); + const rawEnd = endRaw ? closeAt + 2 + endRaw.index + endRaw[0].length : source.length; + result += source.slice(openAt, rawEnd); + index = rawEnd; + continue; + } + + result += `${TAG_OPEN}${decodeEntities(inner)}${TAG_CLOSE}`; + index = closeAt + 2; + } + + return result + source.slice(index); +} diff --git a/test/performance/template-rendering.perf.test.ts b/test/performance/template-rendering.perf.test.ts new file mode 100644 index 000000000..e30a726ac --- /dev/null +++ b/test/performance/template-rendering.perf.test.ts @@ -0,0 +1,248 @@ +import {beforeEach, describe, expect, it} from 'vitest'; + +import {clearTemplateCache, compileTemplate, renderTemplate} from '../../packages/shared/src/template/index'; + +/** + * Performance Tests: Liquid template rendering + * + * Templates are rendered once per recipient, so template rendering sits directly on + * the campaign send path — a campaign to 1M contacts renders 2M templates (subject + + * body). Issue #426 flagged CPU cost as the main risk of adopting a real templating + * language, so these tests pin the throughput floor. + * + * Measured on a developer machine (logged by every run, so regressions show as a trend + * rather than only as a failure): + * + * | Scenario | Result | + * | ---------------------------------------------------- | --------------- | + * | Small template, parse hoisted (issue #426 benchmark) | ~142k renders/s | + * | Realistic campaign body (loop + filters + branches) | ~18k renders/s | + * | Same body via renderTemplate (parse from cache) | ~0.05 ms/render | + * | Parsing a 100 KB template | ~15 ms (once) | + * | Full 500-recipient batch, subject + body | ~30 ms | + * + * At ~0.05 ms/render a 1M-contact campaign spends under a minute of CPU on rendering, + * which is negligible next to 1M SES calls. The assertions below are set well below the + * measured values so they hold on a loaded CI box. + * + * Performance Targets: + * - Rendering must never dominate the per-email cost (network + SES call): < 1ms/email + * - Parsing must be hoisted or cached, never repeated per recipient + * - Memory must stay flat regardless of how many distinct templates are rendered + */ +describe('Performance: Template Rendering at Scale', () => { + /** A realistic marketing email: conditionals, a loop, filters and fallbacks. */ + const CAMPAIGN_TEMPLATE = ` + + +
    + {% if locale == 'es' %} +

    Hola {{firstName ?? cliente}}

    + {% elsif locale == 'fr' %} +

    Bonjour {{firstName ?? client}}

    + {% else %} +

    Hi {{firstName ?? there}}

    + {% endif %} + +

    You have been on the {{plan | upcase}} plan since {{signupDate | date: "%B %Y"}}.

    + + {% assign discount = ltv | divided_by: 20 %} + {% if discount > 25 %}{% assign discount = 25 %}{% endif %} +

    Here is {{discount}}% off your next renewal.

    + +
      + {% for product in products %} +
    • {{forloop.index}}. {{product.name}} — \${{product.price | times: 1.0 | round: 2}}
      • + {% endfor %} +
    + + Unsubscribe +
    + +`; + + const contactVariables = (index: number) => ({ + id: `contact-${index}`, + email: `contact${index}@example.com`, + firstName: index % 3 === 0 ? '' : `Contact ${index}`, + locale: ['en', 'es', 'fr'][index % 3], + plan: index % 2 === 0 ? 'pro' : 'free', + ltv: index % 1000, + signupDate: '2026-05-06T12:00:00Z', + products: [ + {name: 'Starter', price: 9.99}, + {name: 'Team', price: 29.99}, + ], + unsubscribeUrl: `https://example.com/unsubscribe/${index}`, + }); + + beforeEach(() => { + clearTemplateCache(); + }); + + // ======================================== + // THROUGHPUT + // ======================================== + describe('Render throughput', () => { + it('reproduces the issue #426 benchmark at over 50k renders/second', () => { + // Same shape as the benchmark posted on the issue: a small template with one + // variable, one filter and one conditional, parsed once outside the loop. Kept + // separate from the realistic template below so the published number stays + // directly comparable. + const ITERATIONS = 50_000; + const compiled = compileTemplate( + `

    Hello {{ recipient.name }},

    +

    This is render number {{ i }}.

    + {% assign remainder = i | modulo: 2 %} + {% if remainder == 0 %}

    {{ i }} is even.

    {% else %}

    {{ i }} is odd.

    {% endif %}`, + ); + + const startTime = performance.now(); + for (let i = 0; i < ITERATIONS; i += 1) { + compiled.render({i, recipient: {name: `My Name ${i}`}}); + } + const duration = performance.now() - startTime; + + const rendersPerSecond = (ITERATIONS / duration) * 1000; + console.log(`[PERF] issue #426 benchmark: ${Math.round(rendersPerSecond).toLocaleString()} renders/s`); + + expect(rendersPerSecond).toBeGreaterThan(50_000); + }, 60000); + + it('renders a realistic campaign body at over 5k renders/second', () => { + const ITERATIONS = 20_000; + const compiled = compileTemplate(CAMPAIGN_TEMPLATE); + expect(compiled.valid).toBe(true); + + const startTime = performance.now(); + let characters = 0; + for (let i = 0; i < ITERATIONS; i += 1) { + characters += compiled.render(contactVariables(i)).length; + } + const duration = performance.now() - startTime; + + const rendersPerSecond = (ITERATIONS / duration) * 1000; + console.log( + `[PERF] compiled render: ${Math.round(rendersPerSecond).toLocaleString()} renders/s ` + + `(${(duration / ITERATIONS).toFixed(4)} ms/render, ${characters.toLocaleString()} chars)`, + ); + + expect(characters).toBeGreaterThan(0); + expect(rendersPerSecond).toBeGreaterThan(5_000); + }, 60000); + + it('keeps renderTemplate within 1ms/email by caching the parse', () => { + // This is the path every individual send takes (EmailService.format), where the + // caller has no compiled template to hand — the parse must come from the cache. + const ITERATIONS = 10_000; + + const startTime = performance.now(); + for (let i = 0; i < ITERATIONS; i += 1) { + renderTemplate(CAMPAIGN_TEMPLATE, contactVariables(i)); + } + const duration = performance.now() - startTime; + + const msPerRender = duration / ITERATIONS; + console.log(`[PERF] renderTemplate: ${msPerRender.toFixed(4)} ms/render`); + + expect(msPerRender).toBeLessThan(1); + }, 60000); + + it('parses a 100KB template in under 100ms', () => { + // Parsing happens once per template, so it only needs to be cheap enough not to + // stall the first email of a batch. + const large = CAMPAIGN_TEMPLATE.repeat(Math.ceil(100_000 / CAMPAIGN_TEMPLATE.length)); + + const startTime = performance.now(); + const compiled = compileTemplate(large); + const duration = performance.now() - startTime; + + console.log(`[PERF] parse ${(large.length / 1024).toFixed(0)}KB template: ${duration.toFixed(2)} ms`); + + expect(compiled.valid).toBe(true); + expect(duration).toBeLessThan(100); + }, 60000); + }); + + // ======================================== + // CAMPAIGN BATCH BUDGET + // ======================================== + describe('Campaign batch', () => { + it('renders a 500-recipient batch (subject + body) in under 500ms', () => { + // CampaignService.processBatch handles BATCH_SIZE = 500 contacts per job and + // renders both the subject and the body for each one. + const BATCH_SIZE = 500; + const subject = compileTemplate('{% if locale == "es" %}Tu oferta{% else %}Your offer{% endif %}, {{firstName ?? there}}'); + const body = compileTemplate(CAMPAIGN_TEMPLATE); + + const startTime = performance.now(); + for (let i = 0; i < BATCH_SIZE; i += 1) { + const variables = contactVariables(i); + subject.render(variables); + body.render(variables); + } + const duration = performance.now() - startTime; + + console.log(`[PERF] 500-recipient batch: ${duration.toFixed(2)} ms`); + + expect(duration).toBeLessThan(500); + }, 60000); + + it('produces per-recipient output rather than a shared render', () => { + const compiled = compileTemplate(CAMPAIGN_TEMPLATE); + const rendered = new Set(); + + for (let i = 0; i < 100; i += 1) { + rendered.add(compiled.render(contactVariables(i))); + } + + // 100 contacts vary by name, locale, plan and ltv — none should collide. + expect(rendered.size).toBe(100); + }); + }); + + // ======================================== + // MEMORY + // ======================================== + describe('Memory', () => { + it('keeps the parse cache bounded across many distinct templates', () => { + // A worker sees templates from every project it serves. The cache is capped, so + // cycling through far more templates than it holds must not accumulate ASTs. + const TEMPLATE_COUNT = 500; + const templates = Array.from( + {length: TEMPLATE_COUNT}, + (_, i) => `${CAMPAIGN_TEMPLATE}{{firstName ?? there}}`, + ); + + const initialMemory = process.memoryUsage().heapUsed; + + for (const template of templates) { + renderTemplate(template, contactVariables(1)); + } + + const memoryIncrease = (process.memoryUsage().heapUsed - initialMemory) / 1024 / 1024; + console.log(`[PERF] ${TEMPLATE_COUNT} distinct templates: +${memoryIncrease.toFixed(1)} MB heap`); + + expect(memoryIncrease).toBeLessThan(100); + }, 60000); + + it('does not grow the heap while rendering one template repeatedly', () => { + const compiled = compileTemplate(CAMPAIGN_TEMPLATE); + + // Warm up so lazily-allocated internals are not counted as growth. + for (let i = 0; i < 1_000; i += 1) { + compiled.render(contactVariables(i)); + } + + const initialMemory = process.memoryUsage().heapUsed; + for (let i = 0; i < 50_000; i += 1) { + compiled.render(contactVariables(i)); + } + const memoryIncrease = (process.memoryUsage().heapUsed - initialMemory) / 1024 / 1024; + + console.log(`[PERF] 50k renders: +${memoryIncrease.toFixed(1)} MB heap`); + + expect(memoryIncrease).toBeLessThan(100); + }, 60000); + }); +}); diff --git a/yarn.lock b/yarn.lock index 27fade442..e0c6fcbd5 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3154,6 +3154,7 @@ __metadata: "@plunk/types": "npm:*" "@plunk/typescript-config": "npm:*" "@types/node": "npm:^24.10.0" + liquidjs: "npm:^10.27.2" typescript: "npm:^5.7.2" zod: "npm:^3.23.8" languageName: unknown @@ -8888,6 +8889,13 @@ __metadata: languageName: node linkType: hard +"commander@npm:^10.0.0": + version: 10.0.1 + resolution: "commander@npm:10.0.1" + checksum: 10c0/53f33d8927758a911094adadda4b2cbac111a5b377d8706700587650fd8f45b0bbe336de4b5c3fe47fd61f420a3d9bd452b6e0e6e5600a7e74d7bf0174f6efe3 + languageName: node + linkType: hard + "commander@npm:^12.1.0": version: 12.1.0 resolution: "commander@npm:12.1.0" @@ -13439,6 +13447,18 @@ __metadata: languageName: node linkType: hard +"liquidjs@npm:^10.27.2": + version: 10.27.2 + resolution: "liquidjs@npm:10.27.2" + dependencies: + commander: "npm:^10.0.0" + bin: + liquid: bin/liquid.js + liquidjs: bin/liquid.js + checksum: 10c0/aac43b9b0296914de6031cc6957682a6391949ca5c4a3b914b762352c0f2ac8849bc42d85b82ae25c8168945fc9b45b378ca3654b375cde37b44497c43a6f6e8 + languageName: node + linkType: hard + "load-json-file@npm:^4.0.0": version: 4.0.0 resolution: "load-json-file@npm:4.0.0" From 19ac14cca261d590a4616b75a00fd69caf243f96 Mon Sep 17 00:00:00 2001 From: Pau Sanchez Date: Sun, 26 Jul 2026 13:00:27 +0200 Subject: [PATCH 02/12] fix: stub remaining SES calls in DomainService tests verifyDomain and getDomainVerificationAttributes were already mocked, but deleteIdentity and disableFeedbackForwarding were not, so the suite issued live AWS SES requests on every run. DomainService catches and logs failures from both, so the calls stayed invisible in test output while still going over the network. With real SES credentials in .env they would have executed DeleteIdentity and SetIdentityFeedbackForwardingEnabled against a live AWS account. DomainService.test.ts drops from ~40s to 23s. --- apps/api/src/services/__tests__/DomainService.test.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/apps/api/src/services/__tests__/DomainService.test.ts b/apps/api/src/services/__tests__/DomainService.test.ts index 1d441fda3..5b9601554 100644 --- a/apps/api/src/services/__tests__/DomainService.test.ts +++ b/apps/api/src/services/__tests__/DomainService.test.ts @@ -18,6 +18,11 @@ describe('DomainService', () => { status: 'Success', tokens: ['token1', 'token2', 'token3'], }); + // DomainService swallows failures from these two, so an unmocked call is invisible + // in the results while still hitting AWS on every run — and for anyone whose .env + // holds real credentials it would mutate a live SES account. + vi.spyOn(SESService, 'deleteIdentity').mockResolvedValue(undefined); + vi.spyOn(SESService, 'disableFeedbackForwarding').mockResolvedValue(undefined); }); // ======================================== From a3de94d78954129ced8b673fb327712caba318c2 Mon Sep 17 00:00:00 2001 From: Pau Sanchez Date: Sun, 9 Aug 2026 21:15:24 +0200 Subject: [PATCH 03/12] fix: improve performance tests to work on CI and fix test config - vitest 4 deprecated forks - exposed garbage collector for better memory assertions - performance tests scaled down for CI machines - setup.ts to include default AWS SES so tests work if .env misses them --- .../template-rendering.perf.test.ts | 131 +++++++++++++----- test/setup.ts | 23 ++- vitest.config.ts | 20 +-- 3 files changed, 130 insertions(+), 44 deletions(-) diff --git a/test/performance/template-rendering.perf.test.ts b/test/performance/template-rendering.perf.test.ts index e30a726ac..8a57e3d96 100644 --- a/test/performance/template-rendering.perf.test.ts +++ b/test/performance/template-rendering.perf.test.ts @@ -7,23 +7,25 @@ import {clearTemplateCache, compileTemplate, renderTemplate} from '../../package * * Templates are rendered once per recipient, so template rendering sits directly on * the campaign send path — a campaign to 1M contacts renders 2M templates (subject + - * body). Issue #426 flagged CPU cost as the main risk of adopting a real templating - * language, so these tests pin the throughput floor. + * body). CPU cost is therefore the main risk of using a real templating language + * instead of string interpolation, and these tests pin the throughput floor so that + * risk cannot quietly materialise later. * * Measured on a developer machine (logged by every run, so regressions show as a trend * rather than only as a failure): * - * | Scenario | Result | - * | ---------------------------------------------------- | --------------- | - * | Small template, parse hoisted (issue #426 benchmark) | ~142k renders/s | - * | Realistic campaign body (loop + filters + branches) | ~18k renders/s | - * | Same body via renderTemplate (parse from cache) | ~0.05 ms/render | - * | Parsing a 100 KB template | ~15 ms (once) | - * | Full 500-recipient batch, subject + body | ~30 ms | + * | Scenario | Result | + * | --------------------------------------------------- | --------------- | + * | Small template, parse hoisted | ~142k renders/s | + * | Realistic campaign body (loop + filters + branches) | ~18k renders/s | + * | Same body via renderTemplate (parse from cache) | ~0.05 ms/render | + * | Parsing a 100 KB template | ~15 ms (once) | + * | Full 500-recipient batch, subject + body | ~30 ms | + * | Heap retained by 3000 distinct templates | ~6 MB | + * | Heap retained by 50k renders of one template | ~0 MB | * * At ~0.05 ms/render a 1M-contact campaign spends under a minute of CPU on rendering, - * which is negligible next to 1M SES calls. The assertions below are set well below the - * measured values so they hold on a loaded CI box. + * which is negligible next to 1M SES calls. * * Performance Targets: * - Rendering must never dominate the per-email cost (network + SES call): < 1ms/email @@ -31,6 +33,44 @@ import {clearTemplateCache, compileTemplate, renderTemplate} from '../../package * - Memory must stay flat regardless of how many distinct templates are rendered */ describe('Performance: Template Rendering at Scale', () => { + /** + * The raw throughput floors are calibrated on a developer machine, where a shared CI + * runner measures 3.5–4.5× slower on identical code: it has a fraction of the cores, + * and vitest schedules this file alongside the DB-heavy suites, so the benchmark loop + * only ever gets part of one of them. Left unscaled, the floors fail on CI for reasons + * that have nothing to do with the code under test. + * + * Lowering them for everyone would fix that but stop them catching anything on real + * hardware, so scale by environment instead. A developer still has to hit the full + * number; on CI the same assertions become a catastrophic-regression backstop, which + * is all they can honestly be there — losing the parse cache or reintroducing a + * per-render compile costs far more than 6×. + * + * The margin is wide: a CI run that failed the unscaled floors still measured 41.7k + * and 4.2k renders/s, roughly 5× above the scaled ones. + */ + const CI_SLOWDOWN = process.env.CI ? 6 : 1; + + /** + * heapUsed counts garbage the collector has not reached yet, and rendering produces + * a lot of it — sampling it raw measures GC timing rather than retained memory. The + * 50k-render loop below reads +26 MB on a developer machine and +53 MB on CI while + * actually retaining nothing. Forcing a collection first makes the number mean + * "still reachable", which is the only version of it that can catch a leak. + * + * global.gc comes from `execArgv: ['--expose-gc']` in vitest.config.ts. + */ + const forceGc = (globalThis as {gc?: () => void}).gc; + + function retainedHeapMB(): number { + if (!forceGc) { + throw new Error('These assertions need global.gc — run through vitest.config.ts, which sets --expose-gc.'); + } + // Twice: the first pass can leave objects queued for the following cycle. + forceGc(); + forceGc(); + return process.memoryUsage().heapUsed / 1024 / 1024; + } /** A realistic marketing email: conditionals, a loop, filters and fallbacks. */ const CAMPAIGN_TEMPLATE = ` @@ -84,11 +124,12 @@ describe('Performance: Template Rendering at Scale', () => { // THROUGHPUT // ======================================== describe('Render throughput', () => { - it('reproduces the issue #426 benchmark at over 50k renders/second', () => { - // Same shape as the benchmark posted on the issue: a small template with one - // variable, one filter and one conditional, parsed once outside the loop. Kept - // separate from the realistic template below so the published number stays - // directly comparable. + it('renders a minimal template with the parse hoisted at the expected throughput', () => { + // A deliberately small template — one variable, one filter, one conditional — + // parsed once outside the loop, so this measures render cost with parse cost + // removed. Kept separate from the realistic template below so the headline + // number stays comparable across changes rather than moving whenever the + // example campaign body is edited. const ITERATIONS = 50_000; const compiled = compileTemplate( `

    Hello {{ recipient.name }},

    @@ -104,12 +145,16 @@ describe('Performance: Template Rendering at Scale', () => { const duration = performance.now() - startTime; const rendersPerSecond = (ITERATIONS / duration) * 1000; - console.log(`[PERF] issue #426 benchmark: ${Math.round(rendersPerSecond).toLocaleString()} renders/s`); + const floor = 50_000 / CI_SLOWDOWN; + console.log( + `[PERF] minimal template, parse hoisted: ${Math.round(rendersPerSecond).toLocaleString()} renders/s ` + + `(floor ${Math.round(floor).toLocaleString()})`, + ); - expect(rendersPerSecond).toBeGreaterThan(50_000); + expect(rendersPerSecond).toBeGreaterThan(floor); }, 60000); - it('renders a realistic campaign body at over 5k renders/second', () => { + it('renders a realistic campaign body at the expected throughput', () => { const ITERATIONS = 20_000; const compiled = compileTemplate(CAMPAIGN_TEMPLATE); expect(compiled.valid).toBe(true); @@ -122,18 +167,22 @@ describe('Performance: Template Rendering at Scale', () => { const duration = performance.now() - startTime; const rendersPerSecond = (ITERATIONS / duration) * 1000; + const floor = 5_000 / CI_SLOWDOWN; console.log( `[PERF] compiled render: ${Math.round(rendersPerSecond).toLocaleString()} renders/s ` + - `(${(duration / ITERATIONS).toFixed(4)} ms/render, ${characters.toLocaleString()} chars)`, + `(${(duration / ITERATIONS).toFixed(4)} ms/render, ${characters.toLocaleString()} chars, ` + + `floor ${Math.round(floor).toLocaleString()})`, ); expect(characters).toBeGreaterThan(0); - expect(rendersPerSecond).toBeGreaterThan(5_000); + expect(rendersPerSecond).toBeGreaterThan(floor); }, 60000); it('keeps renderTemplate within 1ms/email by caching the parse', () => { // This is the path every individual send takes (EmailService.format), where the // caller has no compiled template to hand — the parse must come from the cache. + // Deliberately not scaled by CI_SLOWDOWN: 1ms/email is a product budget rather + // than a machine measurement, and the loaded runner still comes in at ~0.23ms. const ITERATIONS = 10_000; const startTime = performance.now(); @@ -148,19 +197,22 @@ describe('Performance: Template Rendering at Scale', () => { expect(msPerRender).toBeLessThan(1); }, 60000); - it('parses a 100KB template in under 100ms', () => { + it('parses a 100KB template without stalling the first email of a batch', () => { // Parsing happens once per template, so it only needs to be cheap enough not to - // stall the first email of a batch. + // stall the first email of a batch. Pure CPU, so it scales with the runner. const large = CAMPAIGN_TEMPLATE.repeat(Math.ceil(100_000 / CAMPAIGN_TEMPLATE.length)); + const budget = 100 * CI_SLOWDOWN; const startTime = performance.now(); const compiled = compileTemplate(large); const duration = performance.now() - startTime; - console.log(`[PERF] parse ${(large.length / 1024).toFixed(0)}KB template: ${duration.toFixed(2)} ms`); + console.log( + `[PERF] parse ${(large.length / 1024).toFixed(0)}KB template: ${duration.toFixed(2)} ms (budget ${budget} ms)`, + ); expect(compiled.valid).toBe(true); - expect(duration).toBeLessThan(100); + expect(duration).toBeLessThan(budget); }, 60000); }); @@ -170,7 +222,8 @@ describe('Performance: Template Rendering at Scale', () => { describe('Campaign batch', () => { it('renders a 500-recipient batch (subject + body) in under 500ms', () => { // CampaignService.processBatch handles BATCH_SIZE = 500 contacts per job and - // renders both the subject and the body for each one. + // renders both the subject and the body for each one. Like the 1ms/email budget + // this is a product target, so it is not scaled — the loaded runner spends ~140ms. const BATCH_SIZE = 500; const subject = compileTemplate('{% if locale == "es" %}Tu oferta{% else %}Your offer{% endif %}, {{firstName ?? there}}'); const body = compileTemplate(CAMPAIGN_TEMPLATE); @@ -208,22 +261,28 @@ describe('Performance: Template Rendering at Scale', () => { it('keeps the parse cache bounded across many distinct templates', () => { // A worker sees templates from every project it serves. The cache is capped, so // cycling through far more templates than it holds must not accumulate ASTs. - const TEMPLATE_COUNT = 500; + // + // The count is what gives this assertion its teeth: retaining all 3000 ASTs costs + // ~90 MB, while the capped cache settles at ~6 MB — most of which is the 3000 + // source strings below being flattened as they are used as cache keys, not ASTs. + const TEMPLATE_COUNT = 3_000; const templates = Array.from( {length: TEMPLATE_COUNT}, (_, i) => `${CAMPAIGN_TEMPLATE}{{firstName ?? there}}`, ); - const initialMemory = process.memoryUsage().heapUsed; + const initialMemory = retainedHeapMB(); for (const template of templates) { renderTemplate(template, contactVariables(1)); } - const memoryIncrease = (process.memoryUsage().heapUsed - initialMemory) / 1024 / 1024; - console.log(`[PERF] ${TEMPLATE_COUNT} distinct templates: +${memoryIncrease.toFixed(1)} MB heap`); + const memoryIncrease = retainedHeapMB() - initialMemory; + console.log( + `[PERF] ${TEMPLATE_COUNT.toLocaleString()} distinct templates: +${memoryIncrease.toFixed(1)} MB retained`, + ); - expect(memoryIncrease).toBeLessThan(100); + expect(memoryIncrease).toBeLessThan(25); }, 60000); it('does not grow the heap while rendering one template repeatedly', () => { @@ -234,15 +293,17 @@ describe('Performance: Template Rendering at Scale', () => { compiled.render(contactVariables(i)); } - const initialMemory = process.memoryUsage().heapUsed; + const initialMemory = retainedHeapMB(); for (let i = 0; i < 50_000; i += 1) { compiled.render(contactVariables(i)); } - const memoryIncrease = (process.memoryUsage().heapUsed - initialMemory) / 1024 / 1024; + const memoryIncrease = retainedHeapMB() - initialMemory; - console.log(`[PERF] 50k renders: +${memoryIncrease.toFixed(1)} MB heap`); + console.log(`[PERF] 50k renders: +${memoryIncrease.toFixed(1)} MB retained`); - expect(memoryIncrease).toBeLessThan(100); + // Nothing survives a render, so this measures ~0 MB. Anything that accumulated + // per-render state would show up long before 10 MB across 50k iterations. + expect(memoryIncrease).toBeLessThan(10); }, 60000); }); }); diff --git a/test/setup.ts b/test/setup.ts index 6e2e0674f..a18b82648 100644 --- a/test/setup.ts +++ b/test/setup.ts @@ -31,7 +31,28 @@ if (process.env.REDIS_URL) { } process.env.NODE_ENV = 'test'; -process.env.JWT_SECRET = process.env.JWT_SECRET || 'test-jwt-secret-key-for-testing'; + +// constants.ts calls validateEnv() with no default for the SES credentials, so +// importing any module that transitively pulls in constants.ts throws when they +// are unset — which is exactly what `cp apps/api/.env.example .env` leaves you +// with, since those two ship empty. Tests never reach SES, so fill in placeholders +// rather than requiring every contributor to invent credentials. +// +// Only these two: every other required var (JWT_SECRET, the *_URI values, +// AWS_SES_REGION, DATABASE_URL, REDIS_URL) ships with a value in .env.example and +// is set by the CI workflow, and the DB/Redis URLs must point at real services. +const TEST_ENV_DEFAULTS: Record = { + JWT_SECRET: 'test-jwt-secret-key-for-testing', + AWS_SES_ACCESS_KEY_ID: 'test-ses-access-key-id', + AWS_SES_SECRET_ACCESS_KEY: 'test-ses-secret-access-key', +}; + +for (const [key, value] of Object.entries(TEST_ENV_DEFAULTS)) { + // Empty strings count as unset — validateEnv treats "" as missing. + if (!process.env[key]) { + process.env[key] = value; + } +} // Static import is safe: database.ts only reads env in initialize(), which runs // in beforeAll — well after the env mutations above. diff --git a/vitest.config.ts b/vitest.config.ts index fd348973d..d770436a1 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -26,15 +26,19 @@ export default defineConfig({ // (see test/setup.ts). That isolation is what lets us run files in parallel // without the cross-test interference we used to hit with a shared DB. pool: 'forks', - poolOptions: { - forks: { - // Cap at 4 to stay within Postgres' default max_connections=100 - // when each worker uses connection_limit=20. - maxForks: 4, - minForks: 1, - }, - }, + // Cap at 4 to stay within Postgres' default max_connections=100 when each + // worker uses connection_limit=20 (see test/helpers/database.ts). It also keeps + // the Redis db-number in test/setup.ts below its wrap-around at 16. + // Vitest 4 removed `poolOptions`; the cap is now the top-level `maxWorkers`. + // The old form only logs a deprecation notice and is otherwise ignored, so it + // does not fail the run — it just drops the cap and lets vitest fork per core. + maxWorkers: 4, maxConcurrency: 5, + // Exposes global.gc to the workers. The memory assertions in test/performance + // read heapUsed, which counts garbage the collector has not reached yet — without + // a forced collection the number is GC timing, not retained memory, and cannot + // tell a bounded cache from an unbounded one. + execArgv: ['--expose-gc'], // Only include our test files, not dependency tests include: [ 'apps/**/__tests__/**/*.{test,spec}.{ts,tsx}', From e78abb70ce07a9807dc19a420b4fbf9980aed375 Mon Sep 17 00:00:00 2001 From: Dries Augustyns Date: Mon, 10 Aug 2026 07:50:20 +0200 Subject: [PATCH 04/12] perf(templates): stop re-rendering a template that blew a runtime limit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A template that trips the render or memory ceiling falls back to the legacy placeholder renderer, but nothing recorded that it had failed. The per-email call sites re-compile the same source for every recipient, so each contact re-entered the engine and spent the budget again before falling back — on a large campaign that is a lot of CPU burned on a send that is already degraded. Latch the failure on the cached parse result, which is shared across compileTemplate calls, so only the first recipient pays. The latch is per template and cleared with the cache. Co-Authored-By: Claude Opus 5 --- .../shared/src/__tests__/template.test.ts | 32 +++++++++++++++++++ packages/shared/src/template/index.ts | 21 ++++++++++-- 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/packages/shared/src/__tests__/template.test.ts b/packages/shared/src/__tests__/template.test.ts index 8a5581337..5657072dc 100644 --- a/packages/shared/src/__tests__/template.test.ts +++ b/packages/shared/src/__tests__/template.test.ts @@ -1,5 +1,6 @@ import {beforeEach, describe, expect, it, vi} from 'vitest'; +import {renderEngine} from '../template/engine.js'; import {clearTemplateCache, compileTemplate, renderTemplate, validateTemplate} from '../template/index.js'; /** @@ -300,6 +301,37 @@ describe('renderTemplate', () => { expect(result).toContain('{% for i in (1..100000000) %}'); expect(console.warn).toHaveBeenCalled(); }); + + it('stops re-running a template that blew a runtime limit', () => { + // A campaign renders the same body once per recipient. Without a latch, every one + // of them re-enters the engine and spends the budget again before falling back. + const renderSync = vi.spyOn(renderEngine, 'renderSync').mockImplementation(() => { + throw new Error('memory alloc limit exceeded'); + }); + + for (let recipient = 0; recipient < 5; recipient += 1) { + expect(renderTemplate('{{name}} x', {name: 'Ada'})).toBe('Ada x'); + } + + expect(renderSync).toHaveBeenCalledTimes(1); + renderSync.mockRestore(); + }); + + it('latches per template, not globally', () => { + const renderSync = vi.spyOn(renderEngine, 'renderSync'); + let calls = 0; + renderSync.mockImplementation(() => { + calls += 1; + throw new Error('memory alloc limit exceeded'); + }); + + renderTemplate('{{name}} first', {name: 'Ada'}); + renderTemplate('{{name}} second', {name: 'Ada'}); + + // Each template gets its own chance; one failing does not poison its neighbour. + expect(calls).toBe(2); + renderSync.mockRestore(); + }); }); // ======================================== diff --git a/packages/shared/src/template/index.ts b/packages/shared/src/template/index.ts index 19a09c628..7cb268357 100644 --- a/packages/shared/src/template/index.ts +++ b/packages/shared/src/template/index.ts @@ -35,7 +35,16 @@ const MAX_CACHEABLE_LENGTH = 200_000; /** Cap on distinct templates we remember having warned about. */ const MAX_REPORTED_SOURCES = 64; -type ParseResult = {templates: Template[]; error?: undefined} | {templates?: undefined; error: Error}; +/** + * `renderFailed` latches the first render-time failure (a runtime limit hit, e.g. an + * unbounded loop). It lives on the cached parse result rather than in a `compileTemplate` + * closure because the per-email call sites re-compile the same source for every + * recipient, so a closure-local flag would reset on each one and every recipient would + * pay the render budget again. + */ +type ParseResult = + | {templates: Template[]; renderFailed: boolean; error?: undefined} + | {templates?: undefined; renderFailed?: undefined; error: Error}; const parseCache = new Map(); @@ -59,7 +68,7 @@ function reportOnce(source: string, stage: string, error: unknown): void { function parse(source: string): ParseResult { try { - return {templates: renderEngine.parse(preprocessTemplate(source))}; + return {templates: renderEngine.parse(preprocessTemplate(source)), renderFailed: false}; } catch (error) { reportOnce(source, 'parse', error); return {error: error instanceof Error ? error : new Error(String(error))}; @@ -131,10 +140,18 @@ export function compileTemplate(source: string): CompiledTemplate { source, valid: true, render: variables => { + // A template that already blew a runtime limit will blow it again for every other + // recipient, so stop asking. Retrying spends the render budget per contact on a + // campaign that is going out through the legacy renderer regardless. + if (parsed.renderFailed) { + return renderLegacyTemplate(source, variables); + } + try { return String(renderEngine.renderSync(parsed.templates, buildScope(variables))); } catch (error) { // A render error means a runtime limit was hit (e.g. an unbounded loop). + parsed.renderFailed = true; reportOnce(source, 'render', error); return renderLegacyTemplate(source, variables); } From 474034df9b521a600ca25147b616dcccbba48624 Mon Sep 17 00:00:00 2001 From: Dries Augustyns Date: Mon, 10 Aug 2026 08:13:24 +0200 Subject: [PATCH 05/12] fix(templates): stop rejecting inline bodies on /v1/send MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Syntax checking inline subject/body turned a lenient renderer into a hard 400 on the transactional path. Those bodies are generated by whatever system calls the API — Handlebars output, framework markup, a stray `{{` in a code sample — and they sent fine before Liquid existed. Rendering already handles them the way the old renderer did (unknown placeholders drop to empty, unparseable markup falls back to plain substitution), so a live integration should not start failing over markup that still renders identically. Validation stays on the authoring surfaces, templates and campaigns, where a human is editing and a whole audience is downstream of the mistake. Co-Authored-By: Claude Opus 5 --- apps/api/src/controllers/Actions.ts | 11 +++++++---- apps/api/src/utils/templateValidation.ts | 4 ++++ apps/wiki/content/docs/guides/template-language.mdx | 2 ++ packages/shared/src/__tests__/template.test.ts | 13 +++++++++++++ 4 files changed, 26 insertions(+), 4 deletions(-) diff --git a/apps/api/src/controllers/Actions.ts b/apps/api/src/controllers/Actions.ts index ff280192e..906d87f5f 100644 --- a/apps/api/src/controllers/Actions.ts +++ b/apps/api/src/controllers/Actions.ts @@ -11,7 +11,6 @@ import {EmailVerificationService} from '../services/EmailVerificationService.js' import {EventService} from '../services/EventService.js'; import {NotFound, ValidationError} from '../exceptions/index.js'; import {CatchAsync} from '../utils/asyncHandler.js'; -import {assertValidTemplateSyntax} from '../utils/templateValidation.js'; import {DASHBOARD_URI} from '../app/constants.js'; /** @@ -189,9 +188,13 @@ export class Actions { const {to, subject, body, subscribed, name, from, reply, headers, data, template, attachments} = ActionSchemas.send.parse(req.body); - // Inline subject/body are templates too. Stored templates are checked when they're - // saved, but these arrive with the request and have never been validated. - assertValidTemplateSyntax({subject, body}); + // Inline subject/body are templates too, but they are deliberately NOT syntax + // checked here. Transactional bodies are generated by whatever system calls us — + // Handlebars output, front-end framework markup, an unbalanced `{{` in a code + // sample — and those sent fine before Liquid existed. Rejecting them now would + // break live integrations over markup the renderer already handles by falling back + // to plain placeholder substitution. Authoring-time surfaces (templates, campaigns) + // are where a syntax error is worth failing the write. // Normalize recipients to array and parse email/name type Recipient = {email: string; name?: string}; diff --git a/apps/api/src/utils/templateValidation.ts b/apps/api/src/utils/templateValidation.ts index 21b7c775c..578cae410 100644 --- a/apps/api/src/utils/templateValidation.ts +++ b/apps/api/src/utils/templateValidation.ts @@ -9,6 +9,10 @@ import {ErrorCode, HttpException} from '../exceptions/index.js'; * falling back to plain `{{variable}}` substitution — so authoring time is the only * place a syntax error can be surfaced. Failing the write is what stops a broken * `{% if %}` from reaching a whole audience. + * + * Only for authoring surfaces — templates and campaigns, where a human is editing and + * an audience is at stake. `/v1/send` is left unchecked on purpose: its bodies come + * from other systems and rendering already degrades gracefully. */ export function assertValidTemplateSyntax(fields: Record): void { for (const [field, source] of Object.entries(fields)) { diff --git a/apps/wiki/content/docs/guides/template-language.mdx b/apps/wiki/content/docs/guides/template-language.mdx index f55d2ac35..37e932a72 100644 --- a/apps/wiki/content/docs/guides/template-language.mdx +++ b/apps/wiki/content/docs/guides/template-language.mdx @@ -153,6 +153,8 @@ To show `{{ }}` literally in an email, wrap it in `raw`: Saving a template or campaign whose syntax Liquid can't parse returns `400` with the line and column of the problem, so mistakes surface while you're editing. +`/v1/send` is not checked this way. Transactional bodies are usually generated by another system, so an inline `subject` or `body` is always accepted and rendered on a best-effort basis — markup Liquid can't parse falls through to plain `{{variable}}` substitution instead of failing the request. + Rendering itself is deliberately forgiving — nothing about a template can fail a send: - An unknown variable renders empty. diff --git a/packages/shared/src/__tests__/template.test.ts b/packages/shared/src/__tests__/template.test.ts index 5657072dc..11ed81012 100644 --- a/packages/shared/src/__tests__/template.test.ts +++ b/packages/shared/src/__tests__/template.test.ts @@ -353,6 +353,19 @@ describe('renderTemplate', () => { expect(renderTemplate('{{name}} and {{ unclosed', {name: 'Ada'})).toBe('Ada and {{ unclosed'); }); + it('handles foreign templating markup exactly as the old renderer did', () => { + // Transactional bodies are frequently produced by another system before reaching + // /v1/send, and leftover markup must not cost the caller their email. Unknown + // placeholders drop to empty here just as they did pre-Liquid, which is why + // /v1/send does not syntax check inline bodies. + expect(renderTemplate('Hi {{name}} {{#each items}}
  • {{this}}
    • {{/each}}', {name: 'Ada'})).toBe( + 'Hi Ada
  • ', + ); + + // Block markup Liquid can't parse at all still sends, via the legacy renderer. + expect(renderTemplate('Hi {{name}} {% each items %}', {name: 'Ada'})).toBe('Hi Ada {% each items %}'); + }); + it('logs once per template rather than once per recipient', () => { const template = '{% if %}'; From c48401bd79ea3558f3772dbcd58fac27caf59b02 Mon Sep 17 00:00:00 2001 From: Dries Augustyns Date: Mon, 10 Aug 2026 08:48:36 +0200 Subject: [PATCH 06/12] feat(web): report template syntax errors while editing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The API rejects an unparseable template on save, but a 400 after the fact is a poor way to learn you mistyped a tag — and its "line 12, column 30" is unusable in the visual editor, whose HTML is a single line with no gutter. Validation is pure and already ships in @plunk/shared, so run it client-side on a debounce and show the problem above the editor. In HTML mode the CodeMirror gutter makes line and column meaningful, so they are shown. In visual mode they are not, so the offending {{ }} or {% %} block is extracted from the source and shown instead — the only locator that means anything there. Co-Authored-By: Claude Opus 5 --- .../components/EmailEditor/EmailEditor.tsx | 28 +++- .../src/lib/hooks/useTemplateValidation.ts | 134 ++++++++++++++++++ 2 files changed, 161 insertions(+), 1 deletion(-) create mode 100644 apps/web/src/lib/hooks/useTemplateValidation.ts diff --git a/apps/web/src/components/EmailEditor/EmailEditor.tsx b/apps/web/src/components/EmailEditor/EmailEditor.tsx index e55f85cf9..22e6e40a7 100644 --- a/apps/web/src/components/EmailEditor/EmailEditor.tsx +++ b/apps/web/src/components/EmailEditor/EmailEditor.tsx @@ -13,6 +13,7 @@ import {ResizableImage} from './ResizableImage'; import {HtmlEditor} from './HtmlEditor'; import {useContactFields, useContacts, useSegmentContacts} from '../../lib/hooks/useContacts'; import {useConfig} from '../../lib/hooks/useConfig'; +import {useTemplateValidation} from '../../lib/hooks/useTemplateValidation'; import {useEffect, useRef, useState} from 'react'; import {renderTemplate} from '@plunk/shared'; import { @@ -29,7 +30,7 @@ import { SelectTrigger, SelectValue, } from '@plunk/ui'; -import {Code2, Eye, Monitor, Smartphone, Tablet, Upload, X} from 'lucide-react'; +import {AlertTriangle, Code2, Eye, Monitor, Smartphone, Tablet, Upload, X} from 'lucide-react'; import {network} from '../../lib/network'; import {detectCustomHtmlPatterns, wrapEmailWithStyles} from '../../lib/emailStyles'; import 'tippy.js/dist/tippy.css'; @@ -76,6 +77,10 @@ export function EmailEditor({value, onChange, placeholder, subject, from, replyT // Fetch available contact fields using SWR const {fields: availableFields} = useContactFields(); + // Surface Liquid syntax errors while editing. Saving rejects an unparseable template + // anyway, so the alternative is learning about a typo from a 400 after the fact. + const syntaxIssue = useTemplateValidation(htmlContent); + // Fetch contacts for preview using SWR. // When the campaign targets a segment, scope the dropdown to that segment's // members; otherwise fall back to all contacts in the project. @@ -401,6 +406,27 @@ export function EmailEditor({value, onChange, placeholder, subject, from, replyT + {/* Template syntax errors, reported as you type rather than on save */} + {syntaxIssue && ( +
    + +
    +

    {syntaxIssue.message}

    + {syntaxIssue.excerpt && ( + + {syntaxIssue.excerpt} + + )} +

    + {/* Line numbers only exist in the HTML editor's gutter. */} + {mode === 'html' && syntaxIssue.line !== undefined + ? `Line ${syntaxIssue.line}, column ${syntaxIssue.column} — saving will fail until this is fixed.` + : 'Saving will fail until this is fixed.'} +

    +
    +
    + )} + {/* Editor content */} {mode === 'visual' ? ( <> diff --git a/apps/web/src/lib/hooks/useTemplateValidation.ts b/apps/web/src/lib/hooks/useTemplateValidation.ts new file mode 100644 index 000000000..939b6044a --- /dev/null +++ b/apps/web/src/lib/hooks/useTemplateValidation.ts @@ -0,0 +1,134 @@ +import {validateTemplate} from '@plunk/shared'; +import {useEffect, useState} from 'react'; + +export interface TemplateSyntaxIssue { + /** Human-readable description of the problem, without Liquid's trailing position. */ + message: string; + /** Position in the source. Only meaningful where the author can see line numbers. */ + line?: number; + column?: number; + /** + * The offending `{{ }}` or `{% %}` block, extracted from the source. The visual editor + * has no line numbers — its HTML is a single line — so this is the only locator that + * means anything there. + */ + excerpt?: string; +} + +/** Long enough that a half-typed `{% if` doesn't flash an error mid-keystroke. */ +const DEBOUNCE_MS = 500; + +/** Cap on the excerpt, so a tag spanning half the document doesn't fill the strip. */ +const MAX_EXCERPT_LENGTH = 80; + +/** How far back to look for the delimiter that opened the offending block. */ +const EXCERPT_LOOKBEHIND = 200; + +const DISPLAY_ENTITIES: Record = { + '<': '<', + '>': '>', + '"': '"', + '"': '"', + ''': "'", + ''': "'", + '&': '&', + ' ': ' ', + ' ': ' ', +}; + +/** + * The rich-text editor stores markup typed inside delimiters HTML-escaped, so a + * condition reaches us as `{% if age > 18 %}`. Show it back the way it was typed. + */ +function decodeForDisplay(text: string): string { + return text.replace(/&(?:lt|gt|quot|apos|amp|nbsp|#34|#39|#160);/g, entity => DISPLAY_ENTITIES[entity] ?? entity); +} + +/** Liquid's messages start lowercase ("tag {% if %} not closed"); we lead a sentence. */ +function sentenceCase(message: string): string { + const first = message.charAt(0); + return first >= 'a' && first <= 'z' ? first.toUpperCase() + message.slice(1) : message; +} + +/** Convert a 1-based line/column pair into an index into `source`. */ +function toOffset(source: string, line: number, column: number): number | undefined { + const lines = source.split('\n'); + if (line < 1 || line > lines.length) { + return undefined; + } + + let offset = 0; + for (let index = 0; index < line - 1; index += 1) { + offset += (lines[index]?.length ?? 0) + 1; + } + + return Math.min(offset + Math.max(column - 1, 0), source.length); +} + +/** + * Pull the delimiter block containing `offset` out of the source. + * + * Positions come from the preprocessed source, so the column can drift by the length of + * a rewrite earlier on the same line. Anchoring on the nearest opening delimiter at or + * before the reported offset absorbs that drift. + */ +function extractExcerpt(source: string, offset: number): string | undefined { + const searchFrom = Math.max(0, offset - EXCERPT_LOOKBEHIND); + const window = source.slice(searchFrom, offset + 2); + + const openAt = Math.max(window.lastIndexOf('{{'), window.lastIndexOf('{%')); + if (openAt === -1) { + return undefined; + } + + const start = searchFrom + openAt; + const closeAt = source.slice(start).search(/\}\}|%\}/); + const end = closeAt === -1 ? Math.min(start + MAX_EXCERPT_LENGTH, source.length) : start + closeAt + 2; + + const excerpt = decodeForDisplay(source.slice(start, end)).trim(); + if (excerpt.length === 0) { + return undefined; + } + + return excerpt.length > MAX_EXCERPT_LENGTH ? `${excerpt.slice(0, MAX_EXCERPT_LENGTH)}…` : excerpt; +} + +/** + * Report Liquid syntax errors while the author is still typing. + * + * The API rejects an unparseable template on save, but a 400 after the fact is a poor + * way to learn you mistyped a tag — and its line/column is unreadable in the visual + * editor. Validation is pure and runs client-side, so it can run on every pause instead. + * + * Returns `null` while the template is valid. + */ +export function useTemplateValidation(source: string): TemplateSyntaxIssue | null { + const [issue, setIssue] = useState(null); + + useEffect(() => { + const timer = setTimeout(() => { + const result = validateTemplate(source || ''); + + if (result.valid) { + setIssue(null); + return; + } + + const offset = + result.line !== undefined && result.column !== undefined + ? toOffset(source, result.line, result.column) + : undefined; + + setIssue({ + message: result.error ? sentenceCase(result.error) : 'This template could not be parsed', + line: result.line, + column: result.column, + excerpt: offset === undefined ? undefined : extractExcerpt(source, offset), + }); + }, DEBOUNCE_MS); + + return () => clearTimeout(timer); + }, [source]); + + return issue; +} From 556167da9085f053fc703abf42be626df0381042 Mon Sep 17 00:00:00 2001 From: Dries Augustyns Date: Mon, 10 Aug 2026 08:54:00 +0200 Subject: [PATCH 07/12] feat(web): highlight Liquid logic tags in the visual editor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `{{ }}` already rendered as a chip, but `{% %}` rendered as prose — same font, weight and colour as the sentence around it. The data layer was visible and the control flow was invisible, so a template with a few conditionals read as one undifferentiated block of text. Decorate logic tags too, deliberately styled apart from the blue value chips rather than in a second accent colour: outputting a value and branching are different kinds of thing and should not look like variations of one. Matching is on the delimiters rather than a list of tag names, so if/else/for/case/assign and anything Liquid gains later are all covered. Also resets lastIndex per text node. The shared /g regex kept its offset between nodes, so a document with several text nodes silently skipped matches. Co-Authored-By: Claude Opus 5 --- .../components/EmailEditor/EmailEditor.tsx | 16 ++++++++++ .../EmailEditor/VariableExtension.ts | 30 ++++++++++++++----- 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/apps/web/src/components/EmailEditor/EmailEditor.tsx b/apps/web/src/components/EmailEditor/EmailEditor.tsx index 22e6e40a7..46b05ed10 100644 --- a/apps/web/src/components/EmailEditor/EmailEditor.tsx +++ b/apps/web/src/components/EmailEditor/EmailEditor.tsx @@ -876,6 +876,22 @@ export function EmailEditor({value, onChange, placeholder, subject, from, replyT font-size: 14px; } + /* + * Logic tags are structure, not data, so they read as a different kind of thing + * from the blue value chips above rather than as a second accent colour. + * Decoration-only: these never appear in the sent email, where the tag has + * already been rendered away. + */ + .logic-highlight { + background-color: #f5f5f5; + color: #171717; + padding: 2px 6px; + border-radius: 3px; + border: 1px solid #e5e5e5; + font-family: 'Courier New', monospace; + font-size: 13px; + } + .ProseMirror table { border-collapse: collapse; width: 100%; diff --git a/apps/web/src/components/EmailEditor/VariableExtension.ts b/apps/web/src/components/EmailEditor/VariableExtension.ts index fabb029d6..da542f7e0 100644 --- a/apps/web/src/components/EmailEditor/VariableExtension.ts +++ b/apps/web/src/components/EmailEditor/VariableExtension.ts @@ -91,20 +91,34 @@ export const Variable = Node.create({ props: { decorations: ({doc}) => { const decorations: Decoration[] = []; - const regex = /\{\{([^}]+)\}\}/g; + + // Two kinds of markup, deliberately styled apart: `{{ }}` outputs a value, + // `{% %}` controls structure. Left as plain prose, a conditional is + // indistinguishable from the sentence around it and a template with a few + // branches reads as one undifferentiated block of text. + const patterns = [ + {regex: /\{\{([^}]+)\}\}/g, class: 'variable-highlight'}, + // Any tag, not an allow-list of names: if/else/for/assign/raw and anything + // Liquid gains later all read as structure. Lazy, so `{% if %}{% endif %}` + // side by side stays two chips rather than one long one. + {regex: /\{%[\s\S]*?%\}/g, class: 'logic-highlight'}, + ]; doc.descendants((node, pos) => { - if (node.isText && node.text) { + if (!node.isText || !node.text) { + return; + } + + for (const pattern of patterns) { + // Shared regex objects carry lastIndex between nodes. + pattern.regex.lastIndex = 0; + let match; - while ((match = regex.exec(node.text)) !== null) { + while ((match = pattern.regex.exec(node.text)) !== null) { const from = pos + match.index; const to = from + match[0].length; - decorations.push( - Decoration.inline(from, to, { - class: 'variable-highlight', - }), - ); + decorations.push(Decoration.inline(from, to, {class: pattern.class})); } } }); From 7c3b2ca8fa267261d1950fc18bf6af5edea225ef Mon Sep 17 00:00:00 2001 From: Dries Augustyns Date: Mon, 10 Aug 2026 10:16:11 +0200 Subject: [PATCH 08/12] feat(web): offer ready-made Liquid blocks, and rebuild both suggestion menus The {{ menu works because it is populated from the project's real contact fields: nothing has to be remembered. Control flow had no equivalent, so typing {% now offers the shapes people actually reach for, labelled in plain language with the Liquid shown underneath. Both tags are inserted together with the caret between them, so an unbalanced block, the most common reason a template fails to save, is not reachable through the menu. Building it surfaced problems in the existing {{ menu, so both now share one list component and one row grammar: what you are choosing, what it writes, and what we know about it. Specifically: - Rows lead with the field name rather than raw {{syntax}}, and carry the type and coverage the fields endpoint already returned and the UI discarded. A {{plan}} that only 4% of contacts have renders blank for the rest, and the only way to discover that was to send the campaign. - Hover no longer paints a second highlight competing with the keyboard selection. Pointer movement moves the selection, so exactly one row is ever marked and it is always the one Enter takes. - The selected row uses the same neutral-100 highlight as Select and DropdownMenu instead of inventing a second vocabulary for the same gesture. - Arrowing past the fold scrolls the selection into view. It previously moved invisibly. - Field names are escaped before being interpolated into the list markup. They come from contact data keys, which are supplied through the API and CSV import, so a crafted key could inject markup into the dashboard. - The list is a listbox with options and aria-selected rather than anonymous divs, and Tab accepts alongside Enter. - Logic blocks are type-aware: no == "value" on a boolean, no numeric comparison on a name. Results are capped per field so a query matching several fields shows several fields rather than one field's variants. Two supporting fixes: - Suggestion lists rebind command on update. It closes over the match range, which grows with every keystroke, so the one captured at onStart deleted the trigger and left the query behind as literal text. This affected {{ too. - The syntax strip stays quiet while a menu is open. A half-typed {%ema is not valid Liquid, and flagging it contradicts the menu offering to finish it. Co-Authored-By: Claude Opus 5 --- .../components/EmailEditor/EmailEditor.tsx | 90 ++++-- .../components/EmailEditor/LogicMention.ts | 147 ++++++++++ .../components/EmailEditor/VariableMention.ts | 254 ++++------------ .../components/EmailEditor/suggestionPopup.ts | 275 ++++++++++++++++++ apps/web/src/lib/hooks/useContacts.ts | 28 +- 5 files changed, 567 insertions(+), 227 deletions(-) create mode 100644 apps/web/src/components/EmailEditor/LogicMention.ts create mode 100644 apps/web/src/components/EmailEditor/suggestionPopup.ts diff --git a/apps/web/src/components/EmailEditor/EmailEditor.tsx b/apps/web/src/components/EmailEditor/EmailEditor.tsx index 46b05ed10..9f2ff5b96 100644 --- a/apps/web/src/components/EmailEditor/EmailEditor.tsx +++ b/apps/web/src/components/EmailEditor/EmailEditor.tsx @@ -8,6 +8,8 @@ import {Link} from '@tiptap/extension-link'; import Placeholder from '@tiptap/extension-placeholder'; import {Variable} from './VariableExtension'; import {setAvailableVariables, VariableMention} from './VariableMention'; +import {LogicMention} from './LogicMention'; +import {subscribeSuggestionOpen} from './suggestionPopup'; import {Toolbar} from './Toolbar'; import {ResizableImage} from './ResizableImage'; import {HtmlEditor} from './HtmlEditor'; @@ -75,12 +77,17 @@ export function EmailEditor({value, onChange, placeholder, subject, from, replyT const fileInputRef = useRef(null); // Fetch available contact fields using SWR - const {fields: availableFields} = useContactFields(); + const {fields: availableFields, fieldDetails} = useContactFields(); // Surface Liquid syntax errors while editing. Saving rejects an unparseable template // anyway, so the alternative is learning about a typo from a 400 after the fact. const syntaxIssue = useTemplateValidation(htmlContent); + // A trigger being completed (`{%ema` with its menu open) is not valid Liquid yet. + // Reporting that as an error would contradict the menu offering to finish it. + const [suggestionOpen, setSuggestionOpen] = useState(false); + useEffect(() => subscribeSuggestionOpen(setSuggestionOpen), []); + // Fetch contacts for preview using SWR. // When the campaign targets a segment, scope the dropdown to that segment's // members; otherwise fall back to all contacts in the project. @@ -89,10 +96,10 @@ export function EmailEditor({value, onChange, placeholder, subject, from, replyT const contacts = segmentId ? segmentContacts : allContacts; useEffect(() => { - if (availableFields.length > 0) { - setAvailableVariables(availableFields); + if (fieldDetails.length > 0) { + setAvailableVariables(fieldDetails); } - }, [availableFields]); + }, [fieldDetails]); // Clear the preview selection when the chosen contact is no longer in the // available list (e.g. the segment changed). Otherwise the preview stays @@ -129,6 +136,7 @@ export function EmailEditor({value, onChange, placeholder, subject, from, replyT ResizableImage, Variable, VariableMention, + LogicMention, Placeholder.configure({ placeholder: placeholder || 'Your next email starts here!', }), @@ -407,7 +415,7 @@ export function EmailEditor({value, onChange, placeholder, subject, from, replyT {/* Template syntax errors, reported as you type rather than on save */} - {syntaxIssue && ( + {syntaxIssue && !suggestionOpen && (
    @@ -972,47 +980,75 @@ export function EmailEditor({value, onChange, placeholder, subject, from, replyT padding: 0; } - .variable-suggestion-list { - min-width: 200px; + .suggestion-menu { + width: 320px; + display: flex; + flex-direction: column; + gap: 1px; } - .suggestion-item { + .suggestion-row { display: flex; - align-items: center; - padding: 8px 12px; + flex-direction: column; + gap: 2px; + padding: 7px 10px; cursor: pointer; border-radius: 4px; - transition: background-color 0.15s; + /* Selection follows the pointer, so this only ever paints one row. */ + transition: background-color 0.12s ease-out; } - .suggestion-item:hover, - .suggestion-item.is-selected { - background-color: #e5e7eb; + /* Matches the focus:bg-neutral-100 used by Select and DropdownMenu: the same + gesture gets the same highlight. Legible on its own because pointer movement + moves the selection, so this is the only painted row rather than one of two + competing ones. */ + .suggestion-row.is-selected { + background-color: #f5f5f5; } - .suggestion-item:hover code, - .suggestion-item.is-selected code { - background-color: #dbeafe; - color: #1e3a8a; + .suggestion-row-head { + display: flex; + align-items: baseline; + justify-content: space-between; + gap: 12px; } - .suggestion-item code { - font-size: 14px; + .suggestion-row-label { + font-size: 13px; + line-height: 1.3; + color: #171717; + } + + .suggestion-row-meta { + flex-shrink: 0; + font-size: 11px; + color: #525252; + font-variant-numeric: tabular-nums; + } + + .suggestion-row-syntax { font-family: 'Courier New', monospace; - font-weight: 500; - color: #1f2937; - background-color: #f3f4f6; - padding: 4px 8px; - border-radius: 4px; + font-size: 11px; + line-height: 1.3; + color: #525252; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; } - .suggestion-item-empty { + .suggestion-empty { padding: 12px; text-align: center; - color: #9ca3af; + color: #525252; font-size: 13px; } + @media (prefers-reduced-motion: reduce) { + .suggestion-row { + transition: none; + } + } + .variable-mention { background-color: #dbeafe; color: #1e40af; diff --git a/apps/web/src/components/EmailEditor/LogicMention.ts b/apps/web/src/components/EmailEditor/LogicMention.ts new file mode 100644 index 000000000..0e7a7f347 --- /dev/null +++ b/apps/web/src/components/EmailEditor/LogicMention.ts @@ -0,0 +1,147 @@ +import {Extension} from '@tiptap/core'; +import type {Editor, Range} from '@tiptap/core'; +import Suggestion from '@tiptap/suggestion'; + +import {createSuggestionRenderer, SuggestionMenu, type SuggestionRow} from './suggestionPopup'; +import {getSuggestionFields} from './VariableMention'; + +/** + * A block of Liquid offered by the `{%` menu. + * + * `prefix` and `suffix` sandwich the cursor, so picking a block leaves the caret between + * a matched pair of tags. Unbalanced tags are the most common way a template fails to + * save, and the author never types the closing tag at all. + */ +interface LogicBlock extends SuggestionRow { + prefix: string; + suffix: string; +} + +/** Stand-in used before the author has named a field. */ +const FIELD_PLACEHOLDER = 'field'; + +/** Rows per matching field, so a query surfaces several fields rather than one field's variants. */ +const SHAPES_PER_FIELD = 3; +const MAX_ROWS = 9; + +/** + * `syntax` shows the whole shape, not just the opening tag: an if and an if/else open + * identically and differ only in what closes them, so previewing the prefix alone makes + * two different blocks advertise the same thing. + */ +function block(label: string, prefix: string, suffix: string): LogicBlock { + return {label, syntax: `${prefix}\u2026${suffix.replace('{% else %}', '{% else %}\u2026')}`, prefix, suffix}; +} + +/** + * The shapes worth offering for a field, ordered by how often they are the right answer. + * + * Type-aware because the alternative is offering nonsense: `== "value"` on a boolean, or + * a numeric comparison on a name. The type comes from the same endpoint that populates + * the `{{` menu. + */ +function blocksForField(field: string, type?: string): LogicBlock[] { + if (type === 'boolean') { + return [ + block(`Show when ${field} is true`, `{% if ${field} %}`, '{% endif %}'), + block(`Show when ${field} is false`, `{% unless ${field} %}`, '{% endunless %}'), + block(`Show one thing, or another if not`, `{% if ${field} %}`, '{% else %}{% endif %}'), + ]; + } + + if (type === 'number' || type === 'date') { + return [ + block(`Show when ${field} is above a value`, `{% if ${field} > 0 %}`, '{% endif %}'), + block(`Show when ${field} is set`, `{% if ${field} %}`, '{% endif %}'), + block(`Show one thing, or another if not`, `{% if ${field} %}`, '{% else %}{% endif %}'), + ]; + } + + return [ + block(`Show when ${field} is set`, `{% if ${field} %}`, '{% endif %}'), + block(`Show when ${field} matches a value`, `{% if ${field} == "value" %}`, '{% endif %}'), + block(`Show one thing, or another if missing`, `{% if ${field} %}`, '{% else %}{% endif %}'), + block(`Hide when ${field} is set`, `{% unless ${field} %}`, '{% endunless %}'), + ]; +} + +/** Offered before a field is named. The loop lives here: field types can't identify lists. */ +function defaultBlocks(): LogicBlock[] { + return [ + ...blocksForField(FIELD_PLACEHOLDER).slice(0, 3), + block(`Hide when ${FIELD_PLACEHOLDER} is set`, `{% unless ${FIELD_PLACEHOLDER} %}`, '{% endunless %}'), + block('Repeat for each item in a list', '{% for item in items %}', '{% endfor %}'), + ]; +} + +/** + * Build the menu. + * + * Typing matches against the project's real contact fields, so `{% ema` offers blocks + * already filled in with `email` — the same "pick from what your contacts actually + * have" affordance the `{{` menu provides, extended to control flow. Capped per field + * so a query matching several fields shows several fields. + */ +function logicItems(query: string): LogicBlock[] { + if (!query) { + return defaultBlocks(); + } + + const lowerQuery = query.toLowerCase(); + const matchingFields = getSuggestionFields().filter(field => field.field.toLowerCase().includes(lowerQuery)); + + if (matchingFields.length > 0) { + return matchingFields + .flatMap(field => + blocksForField(field.field, field.type) + .slice(0, SHAPES_PER_FIELD) + .map(shape => ({ + ...shape, + meta: field.coverage < 100 ? `${Math.round(field.coverage)}% of contacts` : undefined, + })), + ) + .slice(0, MAX_ROWS); + } + + return defaultBlocks().filter(item => item.label.toLowerCase().includes(lowerQuery)); +} + +/** + * Offer ready-made Liquid blocks when the author types `{%`. + * + * The `{{` menu works because it is populated from the project's real contact fields — + * nothing has to be remembered. Control flow had no equivalent: you had to know the + * syntax, spell `endif` correctly, and close what you opened. + */ +export const LogicMention = Extension.create({ + name: 'logicMention', + + addProseMirrorPlugins() { + return [ + Suggestion({ + editor: this.editor, + char: '{%', + allowSpaces: false, + startOfLine: false, + + items: ({query}: {query: string}) => logicItems(query), + + command: ({editor, range, props}: {editor: Editor; range: Range; props: LogicBlock}) => { + editor + .chain() + .focus() + .deleteRange(range) + .insertContent(props.prefix + props.suffix) + .run(); + + // Land the caret between the tags, where the content goes. Measured back from + // where the insert left the selection rather than forward from `range`, which + // describes the document before the edit. + editor.commands.setTextSelection(editor.state.selection.from - props.suffix.length); + }, + + render: createSuggestionRenderer(props => new SuggestionMenu(props, 'No matching field or block')), + }), + ]; + }, +}); diff --git a/apps/web/src/components/EmailEditor/VariableMention.ts b/apps/web/src/components/EmailEditor/VariableMention.ts index b409bd94d..6b2741426 100644 --- a/apps/web/src/components/EmailEditor/VariableMention.ts +++ b/apps/web/src/components/EmailEditor/VariableMention.ts @@ -1,108 +1,70 @@ import {Mention} from '@tiptap/extension-mention'; import type {Editor, Range} from '@tiptap/core'; -import tippy, {Instance as TippyInstance, sticky} from 'tippy.js'; -import type {SuggestionProps} from '@tiptap/suggestion'; -// This will be set from the component -let availableVariables: string[] = []; +import type {ContactField} from '../../lib/hooks/useContacts'; +import {createSuggestionRenderer, SuggestionMenu, type SuggestionRow} from './suggestionPopup'; -export function setAvailableVariables(variables: string[]) { - availableVariables = variables || []; +interface VariableRow extends SuggestionRow { + /** Named `id` to satisfy Mention's node-attribute shape for the command callback. */ + id: string; } -// Suggestion component that will be rendered -class VariableSuggestionList { - public element: HTMLDivElement; - private items: string[]; - private selectedIndex: number; - private command: (props: {id: string}) => void; - - constructor(props: SuggestionProps) { - this.items = Array.isArray(props.items) ? props.items : []; - this.selectedIndex = 0; - this.command = props.command; - - this.element = document.createElement('div'); - this.element.className = 'variable-suggestion-list'; - this.render(); - } - - render() { - if (!Array.isArray(this.items) || this.items.length === 0) { - this.element.innerHTML = '
    No variables found
    '; - return; - } - - this.element.innerHTML = this.items - .map( - (item, index) => ` -
    - {{${item}}} -
    - `, - ) - .join(''); - - // Add click handlers - this.element.querySelectorAll('.suggestion-item').forEach((el, index) => { - el.addEventListener('click', () => { - this.selectItem(index); - }); - }); - } - - selectItem(index: number) { - const item = this.items[index]; - if (item && this.command) { - this.command({id: item}); - } - } - - onKeyDown(event: KeyboardEvent): boolean { - if (event.key === 'ArrowUp') { - this.upHandler(); - return true; - } - - if (event.key === 'ArrowDown') { - this.downHandler(); - return true; - } - - if (event.key === 'Enter') { - this.enterHandler(); - return true; - } - - return false; - } - - upHandler() { - if (!Array.isArray(this.items) || this.items.length === 0) return; - this.selectedIndex = (this.selectedIndex + this.items.length - 1) % this.items.length; - this.render(); - } - - downHandler() { - if (!Array.isArray(this.items) || this.items.length === 0) return; - this.selectedIndex = (this.selectedIndex + 1) % this.items.length; - this.render(); - } +/** + * Variables always available at send time, whatever a contact's data holds. Typed and + * covered like real fields so every row in the menu reads the same way. + */ +const RUNTIME_FIELDS: ContactField[] = [ + {field: 'id', type: 'string', coverage: 100}, + {field: 'email', type: 'string', coverage: 100}, + {field: 'unsubscribeUrl', type: 'string', coverage: 100}, + {field: 'subscribeUrl', type: 'string', coverage: 100}, + {field: 'manageUrl', type: 'string', coverage: 100}, + {field: 'locale', type: 'string', coverage: 100}, +]; + +const TYPE_LABELS: Record = { + string: 'text', + number: 'number', + boolean: 'true/false', + date: 'date', +}; + +// Set from the component once the project's fields have loaded. +let contactFields: ContactField[] = []; + +export function setAvailableVariables(fields: ContactField[]) { + contactFields = fields || []; +} - enterHandler() { - if (!Array.isArray(this.items) || this.items.length === 0) return; - this.selectItem(this.selectedIndex); - } +/** The project's fields, runtime variables first. Shared with the `{%` logic menu. */ +export function getSuggestionFields(): ContactField[] { + const seen = new Set(RUNTIME_FIELDS.map(field => field.field)); + return [...RUNTIME_FIELDS, ...contactFields.filter(field => !seen.has(field.field))]; +} - update(props: SuggestionProps) { - this.items = Array.isArray(props.items) ? props.items : []; - this.selectedIndex = 0; - this.render(); - } +/** + * Describe a field in the terms that decide whether a template works. + * + * Coverage is the number that matters and was previously not surfaced anywhere: a + * `{{plan}}` that only 4% of contacts carry renders blank for everyone else, and the + * only way to find that out used to be sending the campaign. + */ +function describe(field: ContactField): string | undefined { + const type = TYPE_LABELS[field.type] ?? 'text'; + return field.coverage < 100 ? `${type} · ${Math.round(field.coverage)}% of contacts` : type; +} - destroy() { - this.element.remove(); - } +function variableItems(query: string): VariableRow[] { + const fields = getSuggestionFields(); + const lowerQuery = query.toLowerCase(); + const matches = lowerQuery ? fields.filter(field => field.field.toLowerCase().includes(lowerQuery)) : fields; + + return matches.slice(0, 10).map(field => ({ + id: field.field, + label: field.field, + syntax: `{{${field.field}}}`, + meta: describe(field), + })); } export const VariableMention = Mention.configure({ @@ -115,106 +77,16 @@ export const VariableMention = Mention.configure({ suggestion: { char: '{{', - items: ({query}) => { - const safeVariables = Array.isArray(availableVariables) ? availableVariables : []; - const allVariables = ['id', 'email', 'unsubscribeUrl', 'subscribeUrl', 'manageUrl', 'locale', ...safeVariables]; - const uniqueVariables = Array.from(new Set(allVariables)).filter(v => typeof v === 'string'); + items: ({query}: {query: string}) => variableItems(query), - if (!query) { - return uniqueVariables.slice(0, 10); + command: ({editor, range, props}: {editor: Editor; range: Range; props: {id: string | null}}) => { + if (!props?.id) { + return; } - const lowerQuery = query.toLowerCase(); - return uniqueVariables.filter(item => item.toLowerCase().startsWith(lowerQuery)).slice(0, 10); - }, - - command: ({editor, range, props}: {editor: Editor; range: Range; props: {id: string | null}}) => { - // Delete the {{ trigger characters and insert the variable as plain text - if (!props.id) return; editor.chain().focus().deleteRange(range).insertContent(`{{${props.id}}}`).run(); }, - render: () => { - let component: VariableSuggestionList; - let popup: TippyInstance[]; - let scrollHandler: (() => void) | null = null; - - return { - onStart: (props: SuggestionProps) => { - component = new VariableSuggestionList(props); - - if (!props.clientRect) { - return; - } - - popup = tippy('body', { - getReferenceClientRect: props.clientRect as () => DOMRect, - appendTo: () => document.body, - content: component.element, - showOnCreate: true, - interactive: true, - trigger: 'manual', - placement: 'bottom-start', - theme: 'variable-suggestion', - plugins: [sticky], - sticky: 'reference', - popperOptions: { - strategy: 'fixed', - }, - }); - - // Update position on scroll - scrollHandler = () => { - if (popup?.[0] && props.clientRect) { - popup[0].setProps({ - getReferenceClientRect: props.clientRect as () => DOMRect, - }); - } - }; - - // Find the scrolling editor container and add listener - const editorContainer = document.querySelector('.overflow-y-auto'); - if (editorContainer) { - editorContainer.addEventListener('scroll', scrollHandler); - } - window.addEventListener('scroll', scrollHandler, true); - }, - - onUpdate(props: SuggestionProps) { - component?.update(props); - - if (!props.clientRect) { - return; - } - - popup?.[0]?.setProps({ - getReferenceClientRect: props.clientRect as () => DOMRect, - }); - }, - - onKeyDown(props: {event: KeyboardEvent}) { - if (props.event.key === 'Escape') { - popup?.[0]?.hide(); - return true; - } - - return component?.onKeyDown(props.event) || false; - }, - - onExit() { - // Clean up scroll listeners - if (scrollHandler) { - const editorContainer = document.querySelector('.overflow-y-auto'); - if (editorContainer) { - editorContainer.removeEventListener('scroll', scrollHandler); - } - window.removeEventListener('scroll', scrollHandler, true); - } - - popup?.[0]?.destroy(); - component?.destroy(); - }, - }; - }, + render: createSuggestionRenderer(props => new SuggestionMenu(props, 'No matching field')), }, }); diff --git a/apps/web/src/components/EmailEditor/suggestionPopup.ts b/apps/web/src/components/EmailEditor/suggestionPopup.ts new file mode 100644 index 000000000..47592ba4f --- /dev/null +++ b/apps/web/src/components/EmailEditor/suggestionPopup.ts @@ -0,0 +1,275 @@ +import tippy, {Instance as TippyInstance, sticky} from 'tippy.js'; +import type {SuggestionProps} from '@tiptap/suggestion'; + +/** + * The part of a suggestion list the popup needs to drive. Both the `{{` variable list + * and the `{%` logic list implement this; everything else here is item-agnostic. + */ +export interface SuggestionListView { + element: HTMLElement; + update(props: SuggestionProps): void; + onKeyDown(event: KeyboardEvent): boolean; + destroy(): void; +} + +/** + * One row. Both menus use the same three slots so inserting a value and inserting a + * block are recognisably the same gesture: what you are choosing, what it will write, + * and what the editor knows about it. + */ +export interface SuggestionRow { + /** What the author is choosing, in their words. Leads the row. */ + label: string; + /** The Liquid this writes. Secondary — shown so the syntax is learned in passing. */ + syntax: string; + /** Optional qualifier: a field's type, or how many contacts actually have it. */ + meta?: string; +} + +/** Field names come from contact data keys, which are user-supplied. */ +function escapeHtml(value: string): string { + return value + .replace(/&/g, '&') + .replace(//g, '>') + .replace(/"/g, '"'); +} + +/** + * The list rendered inside the popup, shared by both triggers. + * + * Selection is single-sourced: moving the pointer over a row *moves the selection* to + * it rather than painting a second, competing highlight. Two differently-highlighted + * rows leave the author guessing which one Enter takes, which is the sort of small + * ambiguity that makes a menu feel untrustworthy without being nameable. + */ +export class SuggestionMenu implements SuggestionListView { + public element: HTMLDivElement; + private items: T[]; + private selectedIndex = 0; + private command: (item: T) => void; + + constructor(props: SuggestionProps, private readonly emptyMessage: string) { + this.items = (Array.isArray(props.items) ? props.items : []) as T[]; + this.command = props.command; + + this.element = document.createElement('div'); + this.element.className = 'suggestion-menu'; + this.element.setAttribute('role', 'listbox'); + this.renderRows(); + } + + /** Full rebuild. Only when the items themselves change, never on selection movement. */ + private renderRows() { + if (this.items.length === 0) { + this.element.innerHTML = `
    ${escapeHtml(this.emptyMessage)}
    `; + return; + } + + this.element.innerHTML = this.items + .map( + (item, index) => ` +
    +
    + ${escapeHtml(item.label)} + ${item.meta ? `${escapeHtml(item.meta)}` : ''} +
    + ${escapeHtml(item.syntax)} +
    + `, + ) + .join(''); + + this.element.querySelectorAll('.suggestion-row').forEach((row, index) => { + row.addEventListener('click', () => this.selectItem(index)); + // Pointer movement drives the same selection the arrow keys do. + row.addEventListener('mousemove', () => { + if (this.selectedIndex !== index) { + this.selectedIndex = index; + this.syncSelection(); + } + }); + }); + } + + /** Selection moved: repaint state only, so the pointer never fights a DOM rebuild. */ + private syncSelection() { + this.element.querySelectorAll('.suggestion-row').forEach((row, index) => { + const selected = index === this.selectedIndex; + row.classList.toggle('is-selected', selected); + row.setAttribute('aria-selected', String(selected)); + + if (selected) { + // Arrowing past the fold has to bring the row with it. + row.scrollIntoView({block: 'nearest'}); + } + }); + } + + selectItem(index: number) { + const item = this.items[index]; + if (item && this.command) { + this.command(item); + } + } + + onKeyDown(event: KeyboardEvent): boolean { + if (event.key === 'ArrowUp') { + this.move(-1); + return true; + } + + if (event.key === 'ArrowDown') { + this.move(1); + return true; + } + + if (event.key === 'Enter' || event.key === 'Tab') { + this.selectItem(this.selectedIndex); + return true; + } + + return false; + } + + private move(delta: number) { + if (this.items.length === 0) { + return; + } + + this.selectedIndex = (this.selectedIndex + delta + this.items.length) % this.items.length; + this.syncSelection(); + } + + update(props: SuggestionProps) { + this.items = (Array.isArray(props.items) ? props.items : []) as T[]; + // Rebind: the command closes over the match range, which grows with every keystroke + // of the query. Keeping the one from `onStart` deletes the trigger and leaves the + // query behind as literal text. + this.command = props.command; + this.selectedIndex = 0; + this.renderRows(); + this.syncSelection(); + } + + destroy() { + this.element.remove(); + } +} + +/** + * Whether any suggestion menu is currently open. + * + * A half-typed trigger — `{%ema` with the menu open on it — is not valid Liquid, so + * syntax validation would report an error on markup the author is in the middle of + * completing, using the very menu that completes it. Consumers subscribe to stay quiet + * while a menu is up. Counted rather than boolean so two triggers can't unset each other. + */ +let openMenus = 0; +const openListeners = new Set<(open: boolean) => void>(); + +function setMenuOpen(delta: number) { + openMenus = Math.max(0, openMenus + delta); + for (const listener of openListeners) { + listener(openMenus > 0); + } +} + +/** Subscribe to suggestion-menu visibility. Returns an unsubscribe function. */ +export function subscribeSuggestionOpen(listener: (open: boolean) => void): () => void { + openListeners.add(listener); + return () => { + openListeners.delete(listener); + }; +} + +/** + * Tippy plumbing shared by the editor's suggestion triggers: positioning, keeping the + * popup pinned while the editor scrolls, and teardown. Only the list rendering differs + * between triggers, so that is all a caller supplies. + */ +export function createSuggestionRenderer(createList: (props: SuggestionProps) => SuggestionListView) { + return () => { + let component: SuggestionListView; + let popup: TippyInstance[]; + let scrollHandler: (() => void) | null = null; + + return { + onStart: (props: SuggestionProps) => { + component = createList(props); + setMenuOpen(1); + + if (!props.clientRect) { + return; + } + + popup = tippy('body', { + getReferenceClientRect: props.clientRect as () => DOMRect, + appendTo: () => document.body, + content: component.element, + showOnCreate: true, + interactive: true, + trigger: 'manual', + placement: 'bottom-start', + theme: 'variable-suggestion', + plugins: [sticky], + sticky: 'reference', + popperOptions: { + strategy: 'fixed', + }, + }); + + // Keep the popup attached to the caret while the editor scrolls under it. + scrollHandler = () => { + if (popup?.[0] && props.clientRect) { + popup[0].setProps({ + getReferenceClientRect: props.clientRect as () => DOMRect, + }); + } + }; + + const editorContainer = document.querySelector('.overflow-y-auto'); + if (editorContainer) { + editorContainer.addEventListener('scroll', scrollHandler); + } + window.addEventListener('scroll', scrollHandler, true); + }, + + onUpdate(props: SuggestionProps) { + component?.update(props); + + if (!props.clientRect) { + return; + } + + popup?.[0]?.setProps({ + getReferenceClientRect: props.clientRect as () => DOMRect, + }); + }, + + onKeyDown(props: {event: KeyboardEvent}) { + if (props.event.key === 'Escape') { + popup?.[0]?.hide(); + return true; + } + + return component?.onKeyDown(props.event) || false; + }, + + onExit() { + setMenuOpen(-1); + + if (scrollHandler) { + const editorContainer = document.querySelector('.overflow-y-auto'); + if (editorContainer) { + editorContainer.removeEventListener('scroll', scrollHandler); + } + window.removeEventListener('scroll', scrollHandler, true); + } + + popup?.[0]?.destroy(); + component?.destroy(); + }, + }; + }; +} diff --git a/apps/web/src/lib/hooks/useContacts.ts b/apps/web/src/lib/hooks/useContacts.ts index 4407e06b0..81648152c 100644 --- a/apps/web/src/lib/hooks/useContacts.ts +++ b/apps/web/src/lib/hooks/useContacts.ts @@ -7,6 +7,14 @@ interface UseContactsOptions { search?: string; } +/** A contact field as reported by `GET /contacts/fields`. */ +export interface ContactField { + field: string; + type: 'string' | 'number' | 'boolean' | 'date'; + /** Percentage of the project's contacts that carry this field. */ + coverage: number; +} + /** * Hook to fetch contacts with optional search */ @@ -66,19 +74,21 @@ export function useSegmentContacts(segmentId?: string, pageSize = 50) { * Hook to fetch available contact fields for variable usage */ export function useContactFields() { - const {data, error, mutate, isLoading} = useSWR<{fields: {field: string; type: string}[]; count: number}>( - '/contacts/fields', - { - revalidateOnFocus: false, - // Cache fields for longer since they don't change often - dedupingInterval: 60000, // 1 minute - }, - ); + const {data, error, mutate, isLoading} = useSWR<{fields: ContactField[]; count: number}>('/contacts/fields', { + revalidateOnFocus: false, + // Cache fields for longer since they don't change often + dedupingInterval: 60000, // 1 minute + }); - const fieldNames = (data?.fields || []).map(f => f.field); + const fieldDetails = data?.fields || []; + const fieldNames = fieldDetails.map(f => f.field); return { fields: fieldNames, + // The endpoint also returns an inferred type and what share of contacts actually + // carry each field. The editor's suggestion menus use both: a field only 4% of + // contacts have is the difference between a working template and a silent blank. + fieldDetails, error, isLoading, mutate, From bfa3ee94c4783e8398e753d5ad3c2a834b46a932 Mon Sep 17 00:00:00 2001 From: Dries Augustyns Date: Mon, 10 Aug 2026 10:24:16 +0200 Subject: [PATCH 09/12] feat(web): insert logic blocks across lines when starting a fresh line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Picking a block put both tags on one line with the caret between them, so writing anything multi-line meant pressing Enter from a spot that reads like the inside of a chip. The block arrived complete but with no room in it. Blocks are now described as lines, where an empty string is an editable gap, and the insertion picks its shape from context: on an otherwise empty line the block is written across paragraphs with the gap already open and the caret in it, and mid-sentence the same lines collapse to the inline pair as before. An if/else opens both of its gaps. Inserting a multi-line block mid-sentence would break the sentence, and an inline pair on an empty line is the awkward case above, so neither shape works everywhere — which is why this reads the caret's surroundings rather than picking one. Co-Authored-By: Claude Opus 5 --- .../components/EmailEditor/LogicMention.ts | 91 ++++++++++++++----- 1 file changed, 67 insertions(+), 24 deletions(-) diff --git a/apps/web/src/components/EmailEditor/LogicMention.ts b/apps/web/src/components/EmailEditor/LogicMention.ts index 0e7a7f347..9b137a32b 100644 --- a/apps/web/src/components/EmailEditor/LogicMention.ts +++ b/apps/web/src/components/EmailEditor/LogicMention.ts @@ -8,13 +8,21 @@ import {getSuggestionFields} from './VariableMention'; /** * A block of Liquid offered by the `{%` menu. * - * `prefix` and `suffix` sandwich the cursor, so picking a block leaves the caret between - * a matched pair of tags. Unbalanced tags are the most common way a template fails to - * save, and the author never types the closing tag at all. + * Described as lines, where an empty string is an editable gap. That single shape covers + * both insertions: on an empty line the block is written across paragraphs with the gap + * already open, and mid-sentence the same lines collapse to an inline pair around the + * caret. Either way both tags arrive together, so an unbalanced block — the most common + * reason a template fails to save — is not reachable through the menu. */ interface LogicBlock extends SuggestionRow { - prefix: string; - suffix: string; + lines: string[]; + /** Index of the gap the caret lands in. */ + caretLine: number; +} + +/** Everything after the opening tag, for the inline form. */ +function inlineSuffix(block: LogicBlock): string { + return block.lines.slice(1).join(''); } /** Stand-in used before the author has named a field. */ @@ -29,8 +37,13 @@ const MAX_ROWS = 9; * identically and differ only in what closes them, so previewing the prefix alone makes * two different blocks advertise the same thing. */ -function block(label: string, prefix: string, suffix: string): LogicBlock { - return {label, syntax: `${prefix}\u2026${suffix.replace('{% else %}', '{% else %}\u2026')}`, prefix, suffix}; +function block(label: string, ...lines: string[]): LogicBlock { + return { + label, + syntax: lines.map(line => line || '\u2026').join(''), + lines, + caretLine: lines.indexOf(''), + }; } /** @@ -43,25 +56,25 @@ function block(label: string, prefix: string, suffix: string): LogicBlock { function blocksForField(field: string, type?: string): LogicBlock[] { if (type === 'boolean') { return [ - block(`Show when ${field} is true`, `{% if ${field} %}`, '{% endif %}'), - block(`Show when ${field} is false`, `{% unless ${field} %}`, '{% endunless %}'), - block(`Show one thing, or another if not`, `{% if ${field} %}`, '{% else %}{% endif %}'), + block(`Show when ${field} is true`, `{% if ${field} %}`, '', '{% endif %}'), + block(`Show when ${field} is false`, `{% unless ${field} %}`, '', '{% endunless %}'), + block(`Show one thing, or another if not`, `{% if ${field} %}`, '', '{% else %}', '', '{% endif %}'), ]; } if (type === 'number' || type === 'date') { return [ - block(`Show when ${field} is above a value`, `{% if ${field} > 0 %}`, '{% endif %}'), - block(`Show when ${field} is set`, `{% if ${field} %}`, '{% endif %}'), - block(`Show one thing, or another if not`, `{% if ${field} %}`, '{% else %}{% endif %}'), + block(`Show when ${field} is above a value`, `{% if ${field} > 0 %}`, '', '{% endif %}'), + block(`Show when ${field} is set`, `{% if ${field} %}`, '', '{% endif %}'), + block(`Show one thing, or another if not`, `{% if ${field} %}`, '', '{% else %}', '', '{% endif %}'), ]; } return [ - block(`Show when ${field} is set`, `{% if ${field} %}`, '{% endif %}'), - block(`Show when ${field} matches a value`, `{% if ${field} == "value" %}`, '{% endif %}'), - block(`Show one thing, or another if missing`, `{% if ${field} %}`, '{% else %}{% endif %}'), - block(`Hide when ${field} is set`, `{% unless ${field} %}`, '{% endunless %}'), + block(`Show when ${field} is set`, `{% if ${field} %}`, '', '{% endif %}'), + block(`Show when ${field} matches a value`, `{% if ${field} == "value" %}`, '', '{% endif %}'), + block(`Show one thing, or another if missing`, `{% if ${field} %}`, '', '{% else %}', '', '{% endif %}'), + block(`Hide when ${field} is set`, `{% unless ${field} %}`, '', '{% endunless %}'), ]; } @@ -69,8 +82,8 @@ function blocksForField(field: string, type?: string): LogicBlock[] { function defaultBlocks(): LogicBlock[] { return [ ...blocksForField(FIELD_PLACEHOLDER).slice(0, 3), - block(`Hide when ${FIELD_PLACEHOLDER} is set`, `{% unless ${FIELD_PLACEHOLDER} %}`, '{% endunless %}'), - block('Repeat for each item in a list', '{% for item in items %}', '{% endfor %}'), + block(`Hide when ${FIELD_PLACEHOLDER} is set`, `{% unless ${FIELD_PLACEHOLDER} %}`, '', '{% endunless %}'), + block('Repeat for each item in a list', '{% for item in items %}', '', '{% endfor %}'), ]; } @@ -127,17 +140,47 @@ export const LogicMention = Extension.create({ items: ({query}: {query: string}) => logicItems(query), command: ({editor, range, props}: {editor: Editor; range: Range; props: LogicBlock}) => { + // Was the author starting a fresh line, or writing mid-sentence? Inserting a + // multi-line block into the middle of a sentence would break the sentence; an + // inline pair on an empty line leaves you typing between two adjacent tags with + // no room, which is the awkward case this distinction exists to avoid. + const parent = editor.state.doc.resolve(range.from).parent; + const trigger = editor.state.doc.textBetween(range.from, range.to); + const onOwnLine = parent.textContent.trim() === trigger.trim(); + + if (!onOwnLine) { + const suffix = inlineSuffix(props); + + editor + .chain() + .focus() + .deleteRange(range) + .insertContent(props.lines[0] + suffix) + .run(); + + // Measured back from where the insert left the selection rather than forward + // from `range`, which describes the document before the edit. + editor.commands.setTextSelection(editor.state.selection.from - suffix.length); + return; + } + editor .chain() .focus() .deleteRange(range) - .insertContent(props.prefix + props.suffix) + .insertContent( + props.lines.map(line => ({ + type: 'paragraph', + ...(line ? {content: [{type: 'text', text: line}]} : {}), + })), + ) .run(); - // Land the caret between the tags, where the content goes. Measured back from - // where the insert left the selection rather than forward from `range`, which - // describes the document before the edit. - editor.commands.setTextSelection(editor.state.selection.from - props.suffix.length); + // Walk back from the end of the last line to the gap's own paragraph. Each + // paragraph boundary costs one position on top of its text. + const trailing = props.lines.slice(props.caretLine + 1); + const offset = trailing.reduce((total, line) => total + line.length + 2, 0); + editor.commands.setTextSelection(editor.state.selection.from - offset); }, render: createSuggestionRenderer(props => new SuggestionMenu(props, 'No matching field or block')), From 066470c77bcc1a1139d3e5a8ad764b682ad6ae16 Mon Sep 17 00:00:00 2001 From: Dries Augustyns Date: Mon, 10 Aug 2026 11:05:44 +0200 Subject: [PATCH 10/12] feat(web): warn about field references that will never resolve MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Syntax validation is structurally blind to the most common template mistake. `{% if emai %}` parses, and is then silently false for every contact forever: the campaign sends, that block renders nothing, and nobody finds out. The fields endpoint already reports every field a project's contacts carry, along with what share of them carry it, so the editor can check references against reality and say two useful things: - emai matches no field on any contact, and email is one character away. - trialEndsAt exists, but only 12% of contacts have it, so the other 88% see this block as empty. That number was previously undiscoverable without sending the campaign. Warnings only, shown below the syntax strip and never blocking a save. A field that legitimately has no data yet is a real scenario: you write the template before the data starts flowing. The risk in a lint like this is crying wolf, since a warning that fires on correct templates teaches people to ignore the strip. So it stays quiet for loop variables, assigned and captured names, filters and their arguments, string literals, the legacy ?? fallback, raw blocks, and the HTML entities the rich-text editor escapes markup into — > would otherwise report a missing field called gt. It also says nothing at all until the field list has loaded, when every reference would look like a mistake. Names bound anywhere silence that name everywhere rather than tracking scope per block. That can miss a genuine mistake, which is the better failure. Co-Authored-By: Claude Opus 5 --- .../components/EmailEditor/EmailEditor.tsx | 22 +- .../src/lib/__tests__/templateLint.test.ts | 134 +++++++++ apps/web/src/lib/hooks/useContacts.ts | 5 +- .../src/lib/hooks/useTemplateValidation.ts | 25 ++ apps/web/src/lib/templateLint.ts | 281 ++++++++++++++++++ 5 files changed, 465 insertions(+), 2 deletions(-) create mode 100644 apps/web/src/lib/__tests__/templateLint.test.ts create mode 100644 apps/web/src/lib/templateLint.ts diff --git a/apps/web/src/components/EmailEditor/EmailEditor.tsx b/apps/web/src/components/EmailEditor/EmailEditor.tsx index 9f2ff5b96..c5ce0d0bb 100644 --- a/apps/web/src/components/EmailEditor/EmailEditor.tsx +++ b/apps/web/src/components/EmailEditor/EmailEditor.tsx @@ -15,7 +15,7 @@ import {ResizableImage} from './ResizableImage'; import {HtmlEditor} from './HtmlEditor'; import {useContactFields, useContacts, useSegmentContacts} from '../../lib/hooks/useContacts'; import {useConfig} from '../../lib/hooks/useConfig'; -import {useTemplateValidation} from '../../lib/hooks/useTemplateValidation'; +import {useTemplateFieldWarnings, useTemplateValidation} from '../../lib/hooks/useTemplateValidation'; import {useEffect, useRef, useState} from 'react'; import {renderTemplate} from '@plunk/shared'; import { @@ -83,6 +83,11 @@ export function EmailEditor({value, onChange, placeholder, subject, from, replyT // anyway, so the alternative is learning about a typo from a 400 after the fact. const syntaxIssue = useTemplateValidation(htmlContent); + // Fields that parse but resolve to nothing: a typo, or a field so few contacts carry + // that the template is blank for most of the audience. Only shown once the template + // parses — a broken template makes every reference in it suspect. + const fieldWarnings = useTemplateFieldWarnings(htmlContent, fieldDetails); + // A trigger being completed (`{%ema` with its menu open) is not valid Liquid yet. // Reporting that as an error would contradict the menu offering to finish it. const [suggestionOpen, setSuggestionOpen] = useState(false); @@ -435,6 +440,21 @@ export function EmailEditor({value, onChange, placeholder, subject, from, replyT
    )} + {/* Field references that parse but will not resolve */} + {!syntaxIssue && !suggestionOpen && fieldWarnings.length > 0 && ( +
    + +
    + {fieldWarnings.map(warning => ( +

    + {warning.message} +

    + ))} +

    This still sends. Nothing here blocks saving.

    +
    +
    + )} + {/* Editor content */} {mode === 'visual' ? ( <> diff --git a/apps/web/src/lib/__tests__/templateLint.test.ts b/apps/web/src/lib/__tests__/templateLint.test.ts new file mode 100644 index 000000000..a20b191ce --- /dev/null +++ b/apps/web/src/lib/__tests__/templateLint.test.ts @@ -0,0 +1,134 @@ +import {describe, expect, it} from 'vitest'; + +import type {ContactField} from '../hooks/useContacts'; +import {lintTemplateFields} from '../templateLint'; + +/** + * This lint catches the mistake syntax validation cannot see: `{% if emai %}` parses, + * and is then silently false for every contact forever. + * + * Its only real failure mode is crying wolf. A warning that fires on a correct template + * teaches authors to ignore the strip, which costs more than the mistakes it catches — + * so most of what follows checks that it stays quiet. + */ +const FIELDS: ContactField[] = [ + {field: 'firstName', type: 'string', coverage: 96}, + {field: 'plan', type: 'string', coverage: 88}, + {field: 'cart', type: 'string', coverage: 71}, + {field: 'trialEndsAt', type: 'date', coverage: 12}, +]; + +describe('lintTemplateFields', () => { + describe('typos', () => { + it('flags a field no contact has, and proposes the near miss', () => { + const [warning] = lintTemplateFields('{% if firstNam %}Hi{% endif %}', FIELDS); + + expect(warning?.kind).toBe('unknown'); + expect(warning?.field).toBe('firstNam'); + expect(warning?.message).toContain('firstName'); + }); + + it('flags an unknown field with no near miss, without inventing one', () => { + const [warning] = lintTemplateFields('{{ favouriteColour }}', FIELDS); + + expect(warning?.kind).toBe('unknown'); + expect(warning?.message).toContain('always be empty'); + expect(warning?.message).not.toContain('Did you mean'); + }); + + it('catches the case from the report: a valid tag that is silently always false', () => { + const warnings = lintTemplateFields('{% if emai %}You have an address{% endif %}', FIELDS); + + expect(warnings).toHaveLength(1); + expect(warnings[0]?.message).toContain('email'); + }); + }); + + describe('sparse fields', () => { + it('reports how few contacts carry a field', () => { + const [warning] = lintTemplateFields('{% if trialEndsAt %}Your trial ends soon{% endif %}', FIELDS); + + expect(warning?.kind).toBe('sparse'); + expect(warning?.message).toContain('12%'); + }); + + it('stays quiet for well-covered fields', () => { + expect(lintTemplateFields('Hi {{firstName}}, you are on {{plan}}', FIELDS)).toEqual([]); + }); + + it('leads with the typo when both are present', () => { + const warnings = lintTemplateFields('{{trialEndsAt}} {{firstNam}}', FIELDS); + + expect(warnings[0]?.kind).toBe('unknown'); + }); + }); + + describe('staying quiet', () => { + it('says nothing before the field list has loaded', () => { + expect(lintTemplateFields('{{ anything }}', [])).toEqual([]); + }); + + it('accepts variables the renderer always provides', () => { + expect(lintTemplateFields('{{email}} {{unsubscribeUrl}} {{id}} {{locale}}', FIELDS)).toEqual([]); + }); + + it('accepts data-prefixed access', () => { + expect(lintTemplateFields('{{data.plan}}', FIELDS)).toEqual([]); + }); + + it('accepts loop variables and forloop', () => { + const source = '{% for item in cart %}{{item.name}} {{forloop.index}}{% endfor %}'; + + expect(lintTemplateFields(source, FIELDS)).toEqual([]); + }); + + it('accepts assigned and captured names', () => { + const source = '{% assign tier = plan %}{{tier}}{% capture greeting %}Hi{% endcapture %}{{greeting}}'; + + expect(lintTemplateFields(source, FIELDS)).toEqual([]); + }); + + it('does not read filters or their arguments as fields', () => { + expect(lintTemplateFields('{{ plan | upcase | default: "none" }}', FIELDS)).toEqual([]); + }); + + it('does not read string literals as fields', () => { + expect(lintTemplateFields('{% if plan == "enterprise" %}Enterprise{% endif %}', FIELDS)).toEqual([]); + }); + + it('does not read a legacy ?? fallback as a field', () => { + // The fallback has always been a literal, so `there` is text and not a variable. + expect(lintTemplateFields('Hi {{firstName ?? there}}', FIELDS)).toEqual([]); + }); + + it('ignores everything inside a raw block', () => { + expect(lintTemplateFields('{% raw %}{{ notAField }}{% endraw %}', FIELDS)).toEqual([]); + }); + + it('ignores Liquid keywords and operators', () => { + const source = '{% if plan == "pro" and cart %}A{% elsif plan %}B{% else %}C{% endif %}'; + + expect(lintTemplateFields(source, FIELDS)).toEqual([]); + }); + + it('ignores the entities the rich-text editor escapes markup into', () => { + // TipTap serialises through the DOM, so a typed `>` is stored as `>` — whose + // letters tokenize as an identifier if entities are not stripped first. + expect(lintTemplateFields('{% if plan == "pro" and cart > 0 %}A{% endif %}', FIELDS)).toEqual([]); + }); + + it('ignores plain prose and HTML around the markup', () => { + const source = '

    Hello there, friend

    Link'; + + expect(lintTemplateFields(source, FIELDS)).toEqual([]); + }); + }); + + it('reports each field once and caps the list', () => { + const source = '{{a1}} {{a1}} {{b2}} {{c3}} {{d4}} {{e5}}'; + const warnings = lintTemplateFields(source, FIELDS); + + expect(warnings.length).toBeLessThanOrEqual(3); + expect(new Set(warnings.map(w => w.field)).size).toBe(warnings.length); + }); +}); diff --git a/apps/web/src/lib/hooks/useContacts.ts b/apps/web/src/lib/hooks/useContacts.ts index 81648152c..283f5ac84 100644 --- a/apps/web/src/lib/hooks/useContacts.ts +++ b/apps/web/src/lib/hooks/useContacts.ts @@ -7,6 +7,9 @@ interface UseContactsOptions { search?: string; } +/** Stable identity so consumers' effect dependencies don't churn before the fetch lands. */ +const NO_FIELDS: ContactField[] = []; + /** A contact field as reported by `GET /contacts/fields`. */ export interface ContactField { field: string; @@ -80,7 +83,7 @@ export function useContactFields() { dedupingInterval: 60000, // 1 minute }); - const fieldDetails = data?.fields || []; + const fieldDetails = data?.fields ?? NO_FIELDS; const fieldNames = fieldDetails.map(f => f.field); return { diff --git a/apps/web/src/lib/hooks/useTemplateValidation.ts b/apps/web/src/lib/hooks/useTemplateValidation.ts index 939b6044a..6006395eb 100644 --- a/apps/web/src/lib/hooks/useTemplateValidation.ts +++ b/apps/web/src/lib/hooks/useTemplateValidation.ts @@ -1,6 +1,9 @@ import {validateTemplate} from '@plunk/shared'; import {useEffect, useState} from 'react'; +import {lintTemplateFields, type TemplateFieldWarning} from '../templateLint'; +import type {ContactField} from './useContacts'; + export interface TemplateSyntaxIssue { /** Human-readable description of the problem, without Liquid's trailing position. */ message: string; @@ -132,3 +135,25 @@ export function useTemplateValidation(source: string): TemplateSyntaxIssue | nul return issue; } + +/** + * Warn about field references that parse but will not resolve. + * + * The counterpart to `useTemplateValidation`: that one catches markup Liquid cannot + * read, this one catches markup Liquid reads happily and then silently does nothing + * with. Debounced on the same cadence so the two strips never disagree about whether + * the author has stopped typing. + */ +export function useTemplateFieldWarnings(source: string, fields: ContactField[]): TemplateFieldWarning[] { + const [warnings, setWarnings] = useState([]); + + useEffect(() => { + const timer = setTimeout(() => { + setWarnings(lintTemplateFields(source || '', fields)); + }, DEBOUNCE_MS); + + return () => clearTimeout(timer); + }, [source, fields]); + + return warnings; +} diff --git a/apps/web/src/lib/templateLint.ts b/apps/web/src/lib/templateLint.ts new file mode 100644 index 000000000..a3e3999a5 --- /dev/null +++ b/apps/web/src/lib/templateLint.ts @@ -0,0 +1,281 @@ +import type {ContactField} from './hooks/useContacts'; + +/** + * A reference that parses but probably will not do what the author meant. + * + * Distinct from a syntax error: `{% if emai %}` is valid Liquid. It is simply false for + * every contact, forever, silently — the template sends, renders nothing, and nobody + * finds out. Syntax validation is structurally blind to this class of mistake. + */ +export interface TemplateFieldWarning { + kind: 'unknown' | 'sparse'; + field: string; + message: string; +} + +/** Below this, a conditional is likely to take the branch the author didn't picture. */ +const SPARSE_COVERAGE_THRESHOLD = 50; + +/** More than a few and the strip becomes a wall; the rest are usually the same mistake. */ +const MAX_WARNINGS = 3; + +/** Liquid's own vocabulary, plus operators and literals. Never contact fields. */ +const KEYWORDS = new Set([ + 'if', + 'elsif', + 'else', + 'endif', + 'unless', + 'endunless', + 'case', + 'when', + 'endcase', + 'for', + 'endfor', + 'in', + 'and', + 'or', + 'not', + 'contains', + 'assign', + 'capture', + 'endcapture', + 'increment', + 'decrement', + 'cycle', + 'tablerow', + 'endtablerow', + 'break', + 'continue', + 'raw', + 'endraw', + 'comment', + 'endcomment', + 'liquid', + 'echo', + 'include', + 'render', + 'layout', + 'with', + 'as', + 'limit', + 'offset', + 'reversed', + 'by', + 'true', + 'false', + 'nil', + 'null', + 'empty', + 'blank', + 'forloop', + 'tablerowloop', +]); + +/** Present in every render scope regardless of what a contact's data holds. */ +const RUNTIME_NAMES = ['id', 'email', 'subscribed', 'unsubscribeUrl', 'subscribeUrl', 'manageUrl', 'locale', 'data']; + +const IDENTIFIER = /[A-Za-z_][A-Za-z0-9_.]*/g; + +/** + * Remove what is not a reference before tokenizing. + * + * Entities first: the rich-text editor serialises through the DOM, so a typed `>` is + * stored as `>` and a quote as `"`. Their letters tokenize as identifiers, which + * would report `gt` and `quot` as missing contact fields on a perfectly good template. + * Decoding rather than deleting keeps `"pro"` a quoted literal for the next + * step, which then strips it. + */ +function stripLiterals(expression: string): string { + return expression + .replace(/&(?:quot|#34);/g, '"') + .replace(/&(?:apos|#39);/g, "'") + .replace(/&[a-zA-Z]+;|&#\d+;/g, ' ') + .replace(/"[^"]*"|'[^']*'/g, ' '); +} + +/** + * Reduce a path to the name worth checking. `plan` stays `plan`; `profile.city` checks + * `profile`, since that is the key a contact either has or does not; `data.plan` checks + * `plan`, because `data` is the container the renderer always provides. + */ +function rootOf(path: string): string | undefined { + const segments = path.split('.').filter(Boolean); + + if (segments[0] === 'data') { + return segments[1]; + } + + return segments[0]; +} + +function identifiersIn(expression: string): string[] { + const names: string[] = []; + // Filters and their arguments are Liquid's vocabulary, not the contact's. + const beforeFilter = stripLiterals(expression).split('|')[0] ?? ''; + + for (const match of beforeFilter.matchAll(IDENTIFIER)) { + const root = rootOf(match[0]); + if (root && !KEYWORDS.has(root)) { + names.push(root); + } + } + + return names; +} + +interface References { + referenced: string[]; + /** Names the template defines for itself: loop variables, `assign`, `capture`. */ + bound: Set; +} + +/** + * Collect what a template reads and what it defines. + * + * Binding is tracked template-wide rather than per scope. A name assigned anywhere + * silences it everywhere, which can miss a genuine mistake — the right trade, because a + * warning that fires on correct templates is worse than one that stays quiet on a rare + * wrong one. + */ +export function collectReferences(source: string): References { + const referenced: string[] = []; + const bound = new Set(); + + // `{% raw %}` content is literal text at send time, so nothing in it is a reference. + const withoutRaw = source.replace(/\{%-?\s*raw\s*-?%\}[\s\S]*?\{%-?\s*endraw\s*-?%\}/g, ' '); + + for (const match of withoutRaw.matchAll(/\{\{([\s\S]*?)\}\}/g)) { + // Plunk's legacy `?? fallback` is a literal string, not a second variable. + const expression = (match[1] ?? '').split('??')[0] ?? ''; + referenced.push(...identifiersIn(expression)); + } + + for (const match of withoutRaw.matchAll(/\{%([\s\S]*?)%\}/g)) { + const body = (match[1] ?? '').replace(/^[-+]|[-+]$/g, '').trim(); + const [tag = '', ...rest] = body.split(/\s+/); + const remainder = rest.join(' '); + + if (tag === 'assign') { + const [target, value = ''] = remainder.split('='); + const name = target?.trim(); + if (name) { + bound.add(name); + } + referenced.push(...identifiersIn(value)); + continue; + } + + if (tag === 'capture' || tag === 'increment' || tag === 'decrement') { + const name = remainder.trim(); + if (name) { + bound.add(name); + } + continue; + } + + if (tag === 'for' || tag === 'tablerow') { + const [item, collection = ''] = remainder.split(/\s+in\s+/); + const name = item?.trim(); + if (name) { + bound.add(name); + } + referenced.push(...identifiersIn(collection)); + continue; + } + + referenced.push(...identifiersIn(remainder)); + } + + return {referenced, bound}; +} + +/** Edit distance, capped: only near-misses are worth proposing as a correction. */ +function distance(a: string, b: string): number { + const rows = Array.from({length: a.length + 1}, (_, i) => [i, ...Array(b.length).fill(0)]); + + for (let column = 0; column <= b.length; column += 1) { + rows[0]![column] = column; + } + + for (let row = 1; row <= a.length; row += 1) { + for (let column = 1; column <= b.length; column += 1) { + const substitution = a[row - 1] === b[column - 1] ? 0 : 1; + rows[row]![column] = Math.min( + rows[row - 1]![column]! + 1, + rows[row]![column - 1]! + 1, + rows[row - 1]![column - 1]! + substitution, + ); + } + } + + return rows[a.length]![b.length]!; +} + +function nearest(name: string, candidates: string[]): string | undefined { + let best: string | undefined; + let bestDistance = Number.POSITIVE_INFINITY; + + for (const candidate of candidates) { + const score = distance(name.toLowerCase(), candidate.toLowerCase()); + if (score < bestDistance) { + bestDistance = score; + best = candidate; + } + } + + // One or two characters out on a name long enough for that to be a typo, not a + // different word. `emai`/`email` qualifies; `plan`/`name` does not. + return best !== undefined && bestDistance <= 2 && name.length >= 4 ? best : undefined; +} + +/** + * Check a template's field references against the project's actual contact fields. + * + * Returns nothing when `fields` is empty: that means the field list has not loaded (or + * the project has no contacts yet), and every reference would look like a mistake. + */ +export function lintTemplateFields(source: string, fields: ContactField[]): TemplateFieldWarning[] { + if (fields.length === 0 || !source) { + return []; + } + + const {referenced, bound} = collectReferences(source); + const byName = new Map(fields.map(field => [field.field, field])); + const known = [...RUNTIME_NAMES, ...byName.keys()]; + const knownSet = new Set(known); + + const warnings: TemplateFieldWarning[] = []; + const seen = new Set(); + + for (const name of referenced) { + if (seen.has(name) || bound.has(name)) { + continue; + } + seen.add(name); + + if (!knownSet.has(name)) { + const suggestion = nearest(name, known); + warnings.push({ + kind: 'unknown', + field: name, + message: suggestion + ? `No contact has a field called ${name}. Did you mean ${suggestion}?` + : `No contact has a field called ${name}, so this will always be empty.`, + }); + continue; + } + + const field = byName.get(name); + if (field && field.coverage < SPARSE_COVERAGE_THRESHOLD) { + warnings.push({ + kind: 'sparse', + field: name, + message: `Only ${Math.round(field.coverage)}% of contacts have ${name}. The rest see this as empty.`, + }); + } + } + + // A typo is actionable; a sparse field is context. Lead with the actionable one. + return warnings.sort((a, b) => (a.kind === b.kind ? 0 : a.kind === 'unknown' ? -1 : 1)).slice(0, MAX_WARNINGS); +} From 54c8f302fe898a1176c937296d3eff586429aa09 Mon Sep 17 00:00:00 2001 From: Dries Augustyns Date: Mon, 10 Aug 2026 11:14:33 +0200 Subject: [PATCH 11/12] feat(web): offer case, contains, for/else, comment and raw blocks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The menu covered if/unless/for and stopped there, so the multi-way branch people reach for most after a plain conditional — case/when — could only be written from memory, which is the thing this menu exists to avoid. Added, each still inserting complete and balanced: - case/when/else, for picking a version per value. Offered for text, number and date fields but not booleans, where a multi-way branch on two states is noise. - contains, for matching within a value. - for/else, which renders the fallback when the list is empty. - comment, for notes that never send. - raw, for showing template markup as literal text. The docs already tell authors to reach for it and the menu did not offer it. Also a below-a-value comparison for numbers and dates, which only had above. The block definitions are now checked against the same parser the API validates with, both filled and left empty. A typo in a closing tag would otherwise ship as a menu entry that produces an unsaveable template, and nothing in the type system would catch it. Co-Authored-By: Claude Opus 5 --- .../components/EmailEditor/LogicMention.ts | 41 +++++++++++++---- .../__tests__/LogicMention.test.ts | 44 +++++++++++++++++++ 2 files changed, 77 insertions(+), 8 deletions(-) create mode 100644 apps/web/src/components/EmailEditor/__tests__/LogicMention.test.ts diff --git a/apps/web/src/components/EmailEditor/LogicMention.ts b/apps/web/src/components/EmailEditor/LogicMention.ts index 9b137a32b..43c2e97b6 100644 --- a/apps/web/src/components/EmailEditor/LogicMention.ts +++ b/apps/web/src/components/EmailEditor/LogicMention.ts @@ -29,7 +29,7 @@ function inlineSuffix(block: LogicBlock): string { const FIELD_PLACEHOLDER = 'field'; /** Rows per matching field, so a query surfaces several fields rather than one field's variants. */ -const SHAPES_PER_FIELD = 3; +const SHAPES_PER_FIELD = 4; const MAX_ROWS = 9; /** @@ -49,11 +49,11 @@ function block(label: string, ...lines: string[]): LogicBlock { /** * The shapes worth offering for a field, ordered by how often they are the right answer. * - * Type-aware because the alternative is offering nonsense: `== "value"` on a boolean, or - * a numeric comparison on a name. The type comes from the same endpoint that populates - * the `{{` menu. + * Type-aware because the alternative is offering nonsense: `== "value"` on a boolean, a + * numeric comparison on a name, or a multi-way `case` on something with two states. The + * type comes from the same endpoint that populates the `{{` menu. */ -function blocksForField(field: string, type?: string): LogicBlock[] { +export function blocksForField(field: string, type?: string): LogicBlock[] { if (type === 'boolean') { return [ block(`Show when ${field} is true`, `{% if ${field} %}`, '', '{% endif %}'), @@ -65,6 +65,7 @@ function blocksForField(field: string, type?: string): LogicBlock[] { if (type === 'number' || type === 'date') { return [ block(`Show when ${field} is above a value`, `{% if ${field} > 0 %}`, '', '{% endif %}'), + block(`Show when ${field} is below a value`, `{% if ${field} < 0 %}`, '', '{% endif %}'), block(`Show when ${field} is set`, `{% if ${field} %}`, '', '{% endif %}'), block(`Show one thing, or another if not`, `{% if ${field} %}`, '', '{% else %}', '', '{% endif %}'), ]; @@ -73,17 +74,41 @@ function blocksForField(field: string, type?: string): LogicBlock[] { return [ block(`Show when ${field} is set`, `{% if ${field} %}`, '', '{% endif %}'), block(`Show when ${field} matches a value`, `{% if ${field} == "value" %}`, '', '{% endif %}'), + block( + `Pick a version per ${field} value`, + `{% case ${field} %}`, + `{% when "value" %}`, + '', + '{% else %}', + '', + '{% endcase %}', + ), block(`Show one thing, or another if missing`, `{% if ${field} %}`, '', '{% else %}', '', '{% endif %}'), + block(`Show when ${field} contains a value`, `{% if ${field} contains "value" %}`, '', '{% endif %}'), block(`Hide when ${field} is set`, `{% unless ${field} %}`, '', '{% endunless %}'), ]; } -/** Offered before a field is named. The loop lives here: field types can't identify lists. */ -function defaultBlocks(): LogicBlock[] { +/** + * Offered before a field is named: the shapes above against a placeholder, plus the ones + * that are about structure rather than a particular field. Loops live here because field + * types cannot identify a list — the endpoint reports arrays as strings. + */ +export function defaultBlocks(): LogicBlock[] { return [ - ...blocksForField(FIELD_PLACEHOLDER).slice(0, 3), + ...blocksForField(FIELD_PLACEHOLDER).slice(0, 4), block(`Hide when ${FIELD_PLACEHOLDER} is set`, `{% unless ${FIELD_PLACEHOLDER} %}`, '', '{% endunless %}'), block('Repeat for each item in a list', '{% for item in items %}', '', '{% endfor %}'), + block( + 'Repeat for each item, or show a fallback when empty', + '{% for item in items %}', + '', + '{% else %}', + '', + '{% endfor %}', + ), + block('Add a note that never sends', '{% comment %}', '', '{% endcomment %}'), + block('Show template markup as literal text', '{% raw %}', '', '{% endraw %}'), ]; } diff --git a/apps/web/src/components/EmailEditor/__tests__/LogicMention.test.ts b/apps/web/src/components/EmailEditor/__tests__/LogicMention.test.ts new file mode 100644 index 000000000..194285dda --- /dev/null +++ b/apps/web/src/components/EmailEditor/__tests__/LogicMention.test.ts @@ -0,0 +1,44 @@ +import {validateTemplate} from '@plunk/shared'; +import {describe, expect, it} from 'vitest'; + +import {blocksForField, defaultBlocks} from '../LogicMention'; + +/** + * The menu's promise is that picking a block cannot produce a template that fails to + * save. That holds only if every block definition is itself valid Liquid, which is easy + * to break with a typo in a closing tag — and the typo would ship as a broken menu + * entry, not a compile error. So the definitions are checked against the same parser the + * API validates with. + */ +const TYPES = [undefined, 'string', 'number', 'date', 'boolean']; + +const ALL_BLOCKS = [...TYPES.flatMap(type => blocksForField('plan', type)), ...defaultBlocks()]; + +describe('logic blocks', () => { + it.each(ALL_BLOCKS.map(block => [block.label, block] as const))('%s is valid Liquid', (_label, block) => { + // Content in the gaps, as an author would fill them. + const filled = block.lines.map(line => line || 'content').join(''); + + expect(validateTemplate(filled)).toMatchObject({valid: true}); + }); + + it.each(ALL_BLOCKS.map(block => [block.label, block] as const))('%s parses when left empty', (_label, block) => { + // Picking a block and typing nothing must still leave a saveable template. + expect(validateTemplate(block.lines.join(''))).toMatchObject({valid: true}); + }); + + it('gives every block somewhere for the caret to land', () => { + for (const block of ALL_BLOCKS) { + expect(block.caretLine).toBeGreaterThanOrEqual(0); + expect(block.lines[block.caretLine]).toBe(''); + } + }); + + it('offers case only where more than two outcomes are possible', () => { + const labels = (type?: string) => blocksForField('plan', type).map(block => block.syntax); + + expect(labels('string').some(syntax => syntax.includes('{% case'))).toBe(true); + // A boolean has two states; a multi-way branch on it is noise. + expect(labels('boolean').some(syntax => syntax.includes('{% case'))).toBe(false); + }); +}); From 1fac3a883d9b7ec2ca3bc1c9021759333177f08a Mon Sep 17 00:00:00 2001 From: Dries Augustyns Date: Mon, 10 Aug 2026 11:18:28 +0200 Subject: [PATCH 12/12] fix(web): don't warn about the workflow event payload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Templates used as workflow steps are edited on the same screen as campaign templates, and are rendered with the trigger's event alongside contact data (WorkflowExecutionService). Reading `event.plan` is correct there, but the field lint only knew about contact fields and reported it as a missing one — a warning firing on a working template, which is the failure mode this lint most needs to avoid. Co-Authored-By: Claude Opus 5 --- .../src/lib/__tests__/templateLint.test.ts | 6 ++++++ apps/web/src/lib/templateLint.ts | 19 +++++++++++++++++-- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/apps/web/src/lib/__tests__/templateLint.test.ts b/apps/web/src/lib/__tests__/templateLint.test.ts index a20b191ce..198279eb7 100644 --- a/apps/web/src/lib/__tests__/templateLint.test.ts +++ b/apps/web/src/lib/__tests__/templateLint.test.ts @@ -72,6 +72,12 @@ describe('lintTemplateFields', () => { expect(lintTemplateFields('{{email}} {{unsubscribeUrl}} {{id}} {{locale}}', FIELDS)).toEqual([]); }); + it('accepts the workflow event payload', () => { + // Workflow step templates are edited on the same screen and read the trigger's + // event, whose keys are not contact fields. + expect(lintTemplateFields('{% if event %}{{event.plan}}{% endif %}', FIELDS)).toEqual([]); + }); + it('accepts data-prefixed access', () => { expect(lintTemplateFields('{{data.plan}}', FIELDS)).toEqual([]); }); diff --git a/apps/web/src/lib/templateLint.ts b/apps/web/src/lib/templateLint.ts index a3e3999a5..37ce7657a 100644 --- a/apps/web/src/lib/templateLint.ts +++ b/apps/web/src/lib/templateLint.ts @@ -72,8 +72,23 @@ const KEYWORDS = new Set([ 'tablerowloop', ]); -/** Present in every render scope regardless of what a contact's data holds. */ -const RUNTIME_NAMES = ['id', 'email', 'subscribed', 'unsubscribeUrl', 'subscribeUrl', 'manageUrl', 'locale', 'data']; +/** + * Present in the render scope regardless of what a contact's data holds. + * + * `event` is the workflow trigger's payload: templates used as workflow steps are edited + * on the same screen and legitimately read keys that no contact carries. + */ +const RUNTIME_NAMES = [ + 'id', + 'email', + 'subscribed', + 'unsubscribeUrl', + 'subscribeUrl', + 'manageUrl', + 'locale', + 'data', + 'event', +]; const IDENTIFIER = /[A-Za-z_][A-Za-z0-9_.]*/g;