fix(markdown): migrate to unified v11 ecosystem#178
Merged
Conversation
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 33 minutes and 1 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThis pull request upgrades Markdown processing dependencies, introduces new TypeScript modules for AST node types and admonition handling, refactors existing markdown transformation modules with improved typing and utilities, and configures Jest preprocessing in the Stencil test setup. Changes
Sequence Diagram(s)sequenceDiagram
participant Input as Input Text
participant Normalize as Normalize Legacy
participant Pipeline as Unified Pipeline
participant Plugins as Plugins<br/>(Parse, GFM, FM, Directive)
participant Admonitions as Admonition<br/>Transform
participant Stringify as Stringify
participant Output as HTML Output
Input->>Normalize: raw markdown text
Normalize->>Normalize: regex rewrite<br/>legacy admonitions
Normalize->>Pipeline: normalized text
Pipeline->>Plugins: feed to remark
Plugins->>Plugins: parse & build AST
Plugins->>Admonitions: AST with directives
Admonitions->>Admonitions: transform directive<br/>nodes to divs
Admonitions->>Stringify: modified AST
Stringify->>Stringify: convert to HTML
Stringify->>Output: final HTML string
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/kompendium/markdown-frontmatter.ts (1)
17-18: Consider error handling for malformed YAML frontmatter.
YAML.parse()can throw aYAMLParseErrorif the frontmatter contains invalid YAML syntax. Currently, this would propagate up and fail the entiremarkdownToHtmlcall.Depending on the desired behavior, you might want to:
- Let it throw (current behavior) - strict validation
- Catch and set
frontmattertoundefinedwith a warning - lenient mode- Catch and include the error in the result
If strict validation is intended, this is fine as-is.
🛡️ Optional: Add error handling for lenient parsing
-const storeData = (file: VFile) => (item: Node & { value?: string }) => { - file.data.frontmatter = item.value ? YAML.parse(item.value) : undefined; +const storeData = (file: VFile) => (item: Node & { value?: string }) => { + if (!item.value) { + file.data.frontmatter = undefined; + return; + } + try { + file.data.frontmatter = YAML.parse(item.value); + } catch { + console.warn('Failed to parse frontmatter YAML'); + file.data.frontmatter = undefined; + } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/kompendium/markdown-frontmatter.ts` around lines 17 - 18, The YAML.parse call in storeData can throw for malformed frontmatter causing markdownToHtml to fail; wrap the YAML.parse(item.value) call in a try-catch inside storeData (function name: storeData) and on parse error set file.data.frontmatter = undefined and record the error (e.g. file.data.frontmatterError = error.message or attach the full error) and optionally emit a warning (console.warn or existing logger) so callers can choose lenient behavior; if strict behavior is desired, keep current behavior but document that YAMLParseError will propagate from storeData/markdownToHtml.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/kompendium/markdown-admonitions.ts`:
- Around line 24-26: normalizeLegacyAdmonitions currently applies LEGACY_SYNTAX
replacement across the entire input, which rewrites text inside fenced code
blocks; change normalizeLegacyAdmonitions so it skips fenced code segments by
detecting fences (``` or ~~~ with optional info string) and only applying
text.replace(LEGACY_SYNTAX, '$1[$2]') to non-fence regions—e.g., split/iterate
the string by fence boundaries or run a simple state machine toggling an inFence
flag when encountering a fence marker, preserving fence markers and their
contents unchanged while performing the legacy-syntax replacement only when
inFence is false.
- Around line 30-33: The admonition type check currently relies on a truthy
lookup (const ifmClass = ADMONITION_TYPES[node.name]) which can accept inherited
properties like toString; change the guard to only accept own properties by
checking ownership first (e.g., use
Object.prototype.hasOwnProperty.call(ADMONITION_TYPES, node.name) or
Object.hasOwn) and only then read ADMONITION_TYPES[node.name] into ifmClass so
visit's containerDirective handler rejects non-own keys; update the code around
visit(..., (node: DirectiveNode) => { ... }) and the ADMONITION_TYPES lookup
accordingly.
- Around line 96-99: The current label extraction only collects direct text
children of labelNode into textParts, which breaks for nested/inline formatted
labels; update the logic in markdown-admonitions where textParts is built
(around labelNode and isTextNode) to recursively traverse labelNode.children (or
use an existing helper) and collect text from all descendant text nodes, join
them into the label string, and keep the existing fallback to node.name when the
resulting label is empty. Ensure you update the reference where textParts is
used so the new recursive extraction replaces the direct-child-only approach.
---
Nitpick comments:
In `@src/kompendium/markdown-frontmatter.ts`:
- Around line 17-18: The YAML.parse call in storeData can throw for malformed
frontmatter causing markdownToHtml to fail; wrap the YAML.parse(item.value) call
in a try-catch inside storeData (function name: storeData) and on parse error
set file.data.frontmatter = undefined and record the error (e.g.
file.data.frontmatterError = error.message or attach the full error) and
optionally emit a warning (console.warn or existing logger) so callers can
choose lenient behavior; if strict behavior is desired, keep current behavior
but document that YAMLParseError will propagate from storeData/markdownToHtml.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3f761e0f-13d5-4d8a-bc90-c4022f75483e
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
package.jsonsrc/kompendium/markdown-admonitions.tssrc/kompendium/markdown-code.tssrc/kompendium/markdown-frontmatter.tssrc/kompendium/markdown-nodes.tssrc/kompendium/markdown-typelinks.tssrc/kompendium/markdown.tssrc/kompendium/test/markdown.spec.tsstencil.config.ts
unified v9 and several of its plugins are abandoned or incompatible with current ESM-only tooling. Replaces the entire remark/rehype stack with v11 equivalents and swaps unmaintained plugins (`remark-admonitions`, `remark-parse-yaml`, `unist-util-flatmap`) for custom implementations backed by `remark-directive` and `yaml`. Co-Authored-By: Claude <noreply@anthropic.com>
PR #166 added ~60 new tests against the old unified v9 pipeline. Merging them required fixing admonition types (`danger` -> `caution`, `info` -> `important`), flipping the code-block type-link test to match the v11 fix, and flattening the `describe` nesting. Co-Authored-By: Claude <noreply@anthropic.com>
jgroth
force-pushed
the
unified-ecosystem-upgrade
branch
from
c86601d to
9a0d98f
Compare
Owner
Author
|
🎉 This PR is included in version 1.1.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
remark-admonitionswith a custom plugin built onremark-directive, preserving the same HTML output and supporting both legacy (:::note title) and standard (:::note[title]) syntaxremark-parse-yamlwith directyamlparsing in the frontmatter pluginunist-util-flatmap(unmaintained) with an inline tree transform inmarkdown-typelinksisElement,isParent,isTextNode) and TypeScript types across all markdown pluginsremark-gfmfor GitHub Flavored Markdown supportstencil.config.tsto handle ESM-only dependenciesSummary by CodeRabbit
New Features
Bug Fixes
Chores