fix(messaging): stop merge tags inserting double quotes - #91352
fix(messaging): stop merge tags inserting double quotes#91352posthog[bot] wants to merge 1 commit into
Conversation
🦔 PostHog Review reviewed this pull requestNothing worth raising this time, so here's a calming picture instead:
|
🤖 CI report✅ Trunk lane — non-backend laneThis PR is assigned to the non-backend lane. It does not run backend Python tests and may merge in parallel with PRs in other lanes. ✅ Bundle size — no changeUncompressed size of every built Total: 68.17 MiB · no change No file changed by more than 1000 B. Posted automatically by build-bundle-size-report · uncompressed bytes from dist-report ✅ Eager graph — within budgetHow much code each root ships on the eager path — downloaded and parsed before the surface is interactive. Measured from the esbuild output chunks (post-tree-shake, static imports only); lazy
🟢 Largest files eagerly shipped from
|
| Size | File |
|---|---|
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 24.6 KiB | ../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js |
| 6.3 KiB | ../node_modules/.pnpm/react@18.3.1/node_modules/react/cjs/react.production.min.js |
| 4.5 KiB | ../node_modules/.pnpm/@jspm+core@2.1.0/node_modules/@jspm/core/nodelibs/browser/process.js |
| 3.9 KiB | ../node_modules/.pnpm/scheduler@0.23.2/node_modules/scheduler/cjs/scheduler.production.min.js |
| 1.4 KiB | ../node_modules/.pnpm/base64-js@1.5.1/node_modules/base64-js/index.js |
| 1.3 KiB | src/RootErrorBoundary.tsx |
| 912 B | ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js |
| 789 B | src/scenes/ChunkLoadErrorBoundary.tsx |
| 762 B | src/index.tsx |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
| Size | File |
|---|---|
| 307.0 KiB | ../node_modules/.pnpm/posthog-js@1.422.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/rrweb.js |
| 267.7 KiB | ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 263.5 KiB | ../node_modules/.pnpm/posthog-js@1.422.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.js |
| 252.5 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 154.2 KiB | ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 104.7 KiB | src/lib/api.ts |
| 95.2 KiB | ../packages/quill/packages/quill/dist/index.js |
| 93.3 KiB | ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js |
| 90.6 KiB | ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js |
Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479
✅ Toolbar bundle — eager 2.26 MiB within budget
What the toolbar ships to customer pages, measured from the esbuild output (minified, post-tree-shake). The eager set is the entry plus everything statically imported from it — fetched before any feature runs; deferred chunks load lazily. The eager guardrail is 5.72 MiB. Each output file must also stay below 10 MB, where CloudFront stops compressing it. The module boundary is enforced separately by check-toolbar-graph.
| Metric | Size | Δ vs base | Budget |
|---|---|---|---|
| Eager (shipped) entry + static imports |
2.26 MiB · 18 files | no change | ████░░░░░░ 39.5% of 5.72 MiB |
| Deferred (lazy) | 2.11 MiB · 45 files | no change | n/a — loads on demand |
Loader dist/toolbar.js |
1.1 KiB | no change | █░░░░░░░░░ 5.8% of 19.5 KiB |
Largest eagerly-shipped chunks
| Size | File |
|---|---|
| 749.4 KiB | dist/toolbar/toolbar-app-MMYGYX5P.css |
| 588.2 KiB | dist/toolbar/chunk-chunk-KLLX3OJI.js |
| 484.7 KiB | dist/toolbar/chunk-chunk-OX4ECFTJ.js |
| 133.8 KiB | dist/toolbar/chunk-chunk-KDK7WOUW.js |
| 131.8 KiB | dist/toolbar/chunk-chunk-FDH2IBXT.js |
| 71.3 KiB | dist/toolbar/toolbar-app-DXVMY4J4.js |
| 69.0 KiB | dist/toolbar/chunk-chunk-TSAL54PB.js |
| 35.6 KiB | dist/toolbar/chunk-chunk-KKFNQPQW.js |
| 21.0 KiB | dist/toolbar/chunk-chunk-TKJC34GA.js |
| 6.8 KiB | dist/toolbar/chunk-chunk-DV7IWQNF.js |
Posted automatically by check-toolbar-size · sizes are toolbar output bytes (shipped, post-tree-shake) from the esbuild metafile
✅ Dist folder size — 🔺 +2.3 KiB (+0.0%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 1435.50 MiB · 🔺 +2.3 KiB (+0.0%)
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
There was a problem hiding this comment.
Small, contained frontend fix to how the merge-tag menu formats a person-property template string, with matrix unit tests covering the new cases; not in risky territory (no auth/billing/crypto/CI/data-model/API-contract surface), and no open reviews or holds block it.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 17L, 1F substantive, 33L/2F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1b-small (33L, 2F, single-area, fix) |
| stamphog 2.0.0b4 | .stamphog/policy.yml @ e7a6d3a · reviewed head e7a6d3a |
The PR was retargeted to a different base branch, so the approved diff is no longer what was reviewed. Stamphog re-reviews automatically.
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
The email templater menu built every person-property merge tag as
{{person.properties["name"]}}. The double quotes come from us, not the
user. Inside an HTML attribute like a link href, a double quote ends the
attribute and mangles the tag. When the quotes survive as an HTML entity,
the renderer resolves the tag to an empty string and the email sends a
blank value with no error.
Emit dot notation for identifier-safe names and single-quoted brackets for
everything else, matching buildDelayExpression in the workflows delay step.
Generated-By: PostHog Desktop
Task-Id: 4b35bbb8-8157-4e92-86f5-f619d4294408
e7a6d3a to
00b5773
Compare
There was a problem hiding this comment.
Contained frontend fix to how merge-tag values are generated, with matching unit tests covering the new helper; no risky-territory surface (no auth, billing, data model, or infra changes) and no unresolved concerns.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 17L, 1F substantive, 33L/2F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1b-small (33L, 2F, single-area, fix) |
| stamphog 2.0.0b4 | .stamphog/policy.yml @ 00b5773 · reviewed head 00b5773 |
|
👋 Visual changes detected for this PR. Review and approve in PostHog Visual Review If these changes are unexpected, they may be caused by a flaky test or a broken snapshot on master. Don't approve — rerun the job or wait for a fix. |
Problem
{{person.properties["name"]}}, so the double quotes come from us, not the user.href, a double quote ends the attribute and mangles the tag. This produced a paying-customer support ticket, with a Liquid parse error (expected "|" before filter).nodejs/src/cdp/utils/liquid.tsdoes not decode", so the tag resolves to an empty string and the email sends a blank value with no error.Changes
{{person.properties.name}}for identifier-safe names and{{person.properties['name']}}for everything else. A person pasting one into a link no longer breaks the link or sends a blank value.buildPersonPropertyMergeValuehelper emits dot notation for bare identifiers and single-quoted brackets otherwise, matchingbuildDelayExpressionin the workflows delay step. Single quotes are safe inside a double-quoted HTML attribute, so the failure mode goes away.How did you test this code?
Automated only; the local quill workspace was not built, so the editor UI could not be rendered.
emailTemplaterLogic.test.ts— added abuildPersonPropertyMergeValuecase matrix: a bare identifier stays dot-access; a leading-$name, a name with a space, a dotted name, and a name with an apostrophe each go through single-quoted, escaped brackets. This catches a regression that re-introduces double quotes or fails to bracket a non-identifier name — the exact fault in the report.pnpm --filter=@posthog/frontend fixapplied. Typecheck reports no errors in the changed files; the unrelated@posthog/quillmodule-resolution errors are pre-existing in this environment.Automatic notifications
Docs update
None.
🤖 Agent context
Autonomy: Fully autonomous
Authored by Claude Code (Opus 4.8) from an inbox report on the workflow email merge-tag menu. Skills invoked:
/writing-tests,/simplify,/writing-simplified-technical-english.liquid.ts("/%22not decoded); that is left alone, since single quotes never reach the renderer entity-encoded from a double-quoted attribute, so the observed symptom is fixed at the source.buildDelayExpressionfromproducts/workflowsstepDelayLogic, but importing it pulls a kea logic module (connected toworkflowLogic) intofrontend/src, and the frontend rules forbid importing acrossproducts/*. A small pure helper with a pointer comment to the sibling was the cleaner tradeoff.Created with PostHog Desktop from this inbox report.