fix(core): convert multi-line tags in cleanBody#77
Conversation
The HTML-tag replacement regex in cleanBody lacked the dotAll flag, so a tag whose content spanned multiple lines was never matched and its raw markup leaked into the cleaned markdown. Add the s flag while keeping the lazy quantifier so multi-line tags are converted and adjacent tags are not merged.
|
@abhay-codes07 is attempting to deploy a commit to the Dodo Payments Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Pull request overview
Fixes @dualmark/core’s cleanBody so configured HTML-ish tags (e.g. <Highlighted>…</Highlighted>) are correctly replaced even when their contents span multiple lines.
Changes:
- Add the
s(dotAll) flag to the tag-replacement regex incleanBodyso.matches newlines. - Add tests covering multi-line tag conversion and ensuring adjacent tags are not merged.
- Add a changeset for a patch release of
@dualmark/core.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/core/src/text.ts | Updates the tag-replacement regex flags (adds dotAll) and documents the behavior. |
| packages/core/test/text.test.ts | Adds regression tests for multi-line tag replacement and lazy matching across adjacent tags. |
| .changeset/cleanbody-multiline-tags.md | Patch changeset documenting the bug fix for release notes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const re = new RegExp(`<${tag}>(.*?)<\\/${tag}>`, "gs"); | ||
| out = out.replace(re, `${marker}$1${marker}`); |
There was a problem hiding this comment.
Review: fix(core): convert multi-line tags in cleanBody
Verdict: LGTM — approving.
Reviewed end-to-end and exercised locally.
Root cause (confirmed on main)
cleanBody built the tag-replacement regex as new RegExp(`<${tag}>(.*?)<\\/${tag}>`, "g") — no dotAll. Since . doesn't match newlines, a tag whose content spans multiple lines was never matched, so raw <Highlighted>…</Highlighted> (or any custom htmlTagReplacements tag) leaked into the "cleaned" markdown that AI clients consume. Verified main uses the "g" flag.
Fix verification
The one-char change to the "gs" flag is correct. The concern with dotAll + a greedy match would be over-matching across multiple tags, but the match stays lazy (.*?), so it still stops at the first closing tag. Exercised the important cases directly on the branch:
| Case | Result |
|---|---|
single-line <Highlighted>this</Highlighted> |
see **this** ✓ (unchanged) |
multi-line <Highlighted>multi\nline</Highlighted> |
**multi\nline** ✓ (the fix) |
adjacent tags across newline <Em>a</Em>\n<Em>b</Em> |
*a*\n*b* ✓ (not merged) |
| adjacent multi-line tags | *a\n1* mid *b\n2* ✓ (not merged) |
empty <Highlighted></Highlighted> |
**** ✓ |
The two added tests cover both the multi-line conversion and the lazy-across-multiple-tags guarantee — exactly the right coverage.
Tests
@dualmark/coretext suite: 35/35 pass locally.
Minimal, well-scoped, correct comment, changeset present. No concerns.
Summary
cleanBodyreplaced configured tags with a regex missing the dotAll flag, so a<Tag>…</Tag>whose content spanned multiple lines was left in the output as raw markup. This adds thesflag so multi-line tags are converted too.Closes #76.
Changes
packages/core/src/text.ts: add thes(dotAll) flag to the tag-replacement regex. The match stays lazy (.*?), so it still stops at the first closing tag and does not merge two adjacent tags.Type
AEO_SPEC_VERSION)Verification
bun run buildpassesbun run testpasses (core: 182 tests)bun run typecheckpassesdualmark verify(no examples touched)/spec/: ran sync-spec (no spec files touched)Changeset
@dualmark/core)cc @thepushkaraj @aagarwal1012. Small core fix, keeps the existing single-line behavior identical.