Skip to content

fix(comark): only complete unclosed frontmatter while streaming - #269

Merged
farnabaz merged 3 commits into
comarkdown:mainfrom
sandros94:fix/duplicate-thematic-break
Jul 27, 2026
Merged

fix(comark): only complete unclosed frontmatter while streaming#269
farnabaz merged 3 commits into
comarkdown:mainfrom
sandros94:fix/duplicate-thematic-break

Conversation

@sandros94

Copy link
Copy Markdown
Contributor

🔗 Linked issue

Resolves #268

❓ Type of change

  • 📖 Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • 👌 Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

autoCloseMarkdown treated any leading --- as an unclosed frontmatter opener and appended a closing ---. That completion only makes sense while streaming; applied to a complete document it caused two bugs:

  • Doubling: ------\n--- → parsed as two thematic breaks (and no hr round-tripped, since renderMarkdown emits ---).
  • Content loss: ---\n<body>---\n<body>\n--- → looked like valid frontmatter, so parseFrontmatter consumed the body as YAML and dropped it — contradicting its own contract that an unclosed --- stays content.

Fix: gate frontmatter completion on streaming. autoCloseMarkdown gains an optional { streaming } argument (default false, backward-compatible — it has no other callers), threaded from parse via opts.streaming. A complete document now leaves a leading unclosed --- alone, so the tokenizer reads it as a thematic break and keeps any following body; while streaming, partial frontmatter is still auto-completed exactly as before. A secondary non-empty guard means a lone --- is never doubled, even mid-stream.

Behavior

input (non-streaming) before after
--- [hr, hr] [hr]
---\n# Heading [] (heading lost) [hr, h1]
---\ntitle: x\n---\n\n# Body frontmatter + body frontmatter + body (unchanged)
streaming ---\ntitle: x completes to frontmatter completes to frontmatter (unchanged)

Tests

  • SPEC horizontal-rule-bare: a bare --- parses to a single hr.
  • SPEC frontmatter-unclosed-is-thematic-break: a leading --- + body stays a thematic break + body (swallow).
  • test/auto-close.test.ts: the frontmatter partial cases now assert streaming completion, plus new non-streaming cases asserting the body is preserved.

📝 Checklist

  • I have linked an issue or discussion.
  • I have run pnpm verify and it passes. (pre-existing issues)
  • I have updated the documentation accordingly.

@sandros94
sandros94 requested a review from farnabaz as a code owner July 5, 2026 16:45
@vercel

vercel Bot commented Jul 5, 2026

Copy link
Copy Markdown

@sandros94 is attempting to deploy a commit to the NuxtLabs Team on Vercel.

A member of the Team first needs to authorize it.

@sandros94

Copy link
Copy Markdown
Contributor Author

just to be clear, as I stated in the related issue #268 (comment): I'm not really satisfied with this approach, as it basically shifts the issue into another place (essentially the buggy situation happens at streaming rather than static content).

I just wanted to open this PR to hope for inspiration, happy if it gets discarded/closed anyway

@farnabaz

Copy link
Copy Markdown
Collaborator

Thanks @sandros94 for the Pr, But I'm thinking why we need to have thematic PR right at the beginning? Having an extra line before this hr will resolve the issue 🤔

@sandros94

Copy link
Copy Markdown
Contributor Author

Thanks @sandros94 for the Pr, But I'm thinking why we need to have thematic PR right at the beginning? Having an extra line before this hr will resolve the issue 🤔

True but this wont solve a broken frontmatter, potentially causing a it to throw.

Also another thing that come to mind is that renderMarkdown calls .trim() internally which would not solve the issue in node/block-level parsers.

One such use case I'm thinking of is someone pasing a markdown string from clipboard that needs to be parsed into AST before injecting it into an editor. If at any level that string gets trimmed and do happens to start with --- we are back to square one with this issue

@pkg-pr-new

pkg-pr-new Bot commented Jul 27, 2026

Copy link
Copy Markdown

Open in StackBlitz

comark

npm i https://pkg.pr.new/comark@269

@comark/angular

npm i https://pkg.pr.new/@comark/angular@269

@comark/ansi

npm i https://pkg.pr.new/@comark/ansi@269

@comark/html

npm i https://pkg.pr.new/@comark/html@269

@comark/nuxt

npm i https://pkg.pr.new/@comark/nuxt@269

@comark/react

npm i https://pkg.pr.new/@comark/react@269

@comark/svelte

npm i https://pkg.pr.new/@comark/svelte@269

@comark/vue

npm i https://pkg.pr.new/@comark/vue@269

commit: 30169d2

@farnabaz farnabaz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍
Thanks

@farnabaz
farnabaz merged commit aa8c235 into comarkdown:main Jul 27, 2026
3 of 11 checks passed
@sandros94
sandros94 deleted the fix/duplicate-thematic-break branch July 27, 2026 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: auto-close mishandles a leading ---: doubles a thematic break, and swallows body content

2 participants