Conversation
🦋 Changeset detectedLatest commit: 1159333 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughAdds a standalone Changes
Sequence Diagram(s)sequenceDiagram
participant MD as MDX Author
participant RD as remark-directives
participant H as hastscript
participant RT as Runtime Components
participant CSS as Stylesheet
MD->>RD: Write `:callout[...]` or `<Callout/>` in MDX
RD->>H: Build node with attributes (h(...))
H-->>RD: Merged hProperties (className, hName)
RD->>RT: Emit HAST/props referencing `doom-callout(s)`
RT->>CSS: Apply `.doom-callout` / `.doom-callouts` styles
RT-->>MD: Rendered inline callout UI
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
commit: |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new inline callout directive/component to complement existing callouts, including styling and documentation updates, plus a small robustness tweak to the unmatched-anchor remark-lint rule.
Changes:
- Introduce
Calloutruntime component and:callout[...]directive handling (viaremark-directives) and associated styling. - Enhance
Calloutscomponent to accept standarddivHTML attributes and mergeclassName. - Update Chinese docs to document
Callouts/Callout, and adjustno-unmatched-anchorpath handling.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Adds/updates lock entries for hastscript dependency. |
| packages/doom/package.json | Adds hastscript dependency used by the directives plugin. |
| packages/doom/styles/global.scss | Refactors callout styling into callout-base; adds .doom-callout styling. |
| packages/doom/src/runtime/components/index.ts | Exports the new Callout component. |
| packages/doom/src/runtime/components/Callouts.tsx | Allows passing through HTML attributes and merges className via clsx. |
| packages/doom/src/runtime/components/Callout.tsx | New inline callout component (span) with doom-callout class. |
| packages/doom/src/plugins/directives/remark-directives.ts | Adds callout directive support and merges directive attributes via hastscript. |
| packages/doom/src/remark-lint/no-unmatched-anchor.ts | Uses configRoot local, and skips anchor extraction for non-doc referenced targets. |
| docs/zh/usage/mdx.mdx | Documents Callouts and Callout MDX usage examples. |
| docs/zh/usage/markdown.md | Adds explicit anchors for sections and documents Callout markdown directive usage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/doom/src/plugins/directives/remark-directives.ts (1)
38-53: Make the fallback wrapper tag explicit insetProperties().Currently,
setProperties()usesh(data.hName || 'div', ...)to normalize attributes, but only the computed properties are stored back todata.hProperties—the fallback 'div' tag is not persisted todata.hName. This means the 'callouts' directive relies on mdast-util-to-hast's default fallback (which is confirmed to be 'div') for the final emitted tag. Settingdata.hName ??= 'div'at the start ofsetProperties()makes this behavior explicit and ensures consistency between the tag used for attribute normalization and the one persisted through the transform.♻️ Suggested change
const setProperties = (className: string) => { + data.hName ??= 'div' // https://github.com/syntax-tree/hastscript/pull/24 - const n = h(data.hName || 'div', node.attributes!) + const n = h(data.hName, node.attributes!) data.hProperties = { ...n.properties, className: [ ...((n.properties.className || []) as string[]), className, ], } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/doom/src/plugins/directives/remark-directives.ts` around lines 38 - 53, setProperties currently normalizes attributes with h(data.hName || 'div', ...) but doesn't persist the fallback tag; update setProperties to explicitly set data.hName to 'div' when absent (e.g., data.hName ??= 'div') before calling h so the fallback tag used for normalization is also stored, then continue assigning data.hProperties from the normalized node properties (referencing setProperties, data.hName, data.hProperties, h, and node.attributes).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/doom/src/remark-lint/no-unmatched-anchor.ts`:
- Around line 144-146: The code currently only checks suffix with
isDoc(referencedFilepath) but then calls getAnchors(referencedFilepath) which
can throw ENOENT for missing files; update the logic in no-unmatched-anchor rule
to verify the target exists (e.g., using fs.existsSync or await
fs.promises.stat) before calling getAnchors, or wrap the getAnchors(...) call in
a try/catch that returns silently on ENOENT and logs/continues for other errors;
modify the block around isDoc(referencedFilepath) and getAnchors to perform this
existence check or catch so a missing ./missing.md#anchor does not abort
linting.
---
Nitpick comments:
In `@packages/doom/src/plugins/directives/remark-directives.ts`:
- Around line 38-53: setProperties currently normalizes attributes with
h(data.hName || 'div', ...) but doesn't persist the fallback tag; update
setProperties to explicitly set data.hName to 'div' when absent (e.g.,
data.hName ??= 'div') before calling h so the fallback tag used for
normalization is also stored, then continue assigning data.hProperties from the
normalized node properties (referencing setProperties, data.hName,
data.hProperties, h, and node.attributes).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 93d10ddf-4872-4233-b12f-eb8945045578
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (9)
docs/zh/usage/markdown.mddocs/zh/usage/mdx.mdxpackages/doom/package.jsonpackages/doom/src/plugins/directives/remark-directives.tspackages/doom/src/remark-lint/no-unmatched-anchor.tspackages/doom/src/runtime/components/Callout.tsxpackages/doom/src/runtime/components/Callouts.tsxpackages/doom/src/runtime/components/index.tspackages/doom/styles/global.scss
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/doom/src/plugins/directives/remark-directives.ts (1)
38-48: Consider handling potentially undefinednode.attributes.The
node.attributes!non-null assertion at line 40 assumes attributes always exist, but directive node types defineattributesas optional (Record<string, string | null | undefined> | undefined). Whilehastscript'sh()may handleundefinedgracefully, removing the assertion makes the code more defensive.🛡️ Suggested fix
const setProperties = (className: string) => { // https://github.com/syntax-tree/hastscript/pull/24 - const n = h(data.hName || 'div', node.attributes!) + const n = h(data.hName || 'div', node.attributes) data.hProperties = {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/doom/src/plugins/directives/remark-directives.ts` around lines 38 - 48, The helper setProperties currently uses the non-null assertion node.attributes! when calling h, which is unsafe; update setProperties to defensively handle missing attributes by passing node.attributes or an empty object (e.g., node.attributes ?? {}) into h(data.hName || 'div', ...), and then set data.hProperties as before (preserving n.properties and merging className) so h is always invoked with a defined attributes object; ensure you reference setProperties, node.attributes, h, data.hName, and data.hProperties when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/en/usage/mdx.mdx`:
- Around line 71-80: The Callouts block is written as continuous text instead of
an ordered list, breaking the expected `<Callouts>` → `<ol>` → `<li>` structure
used by the component (see `.doom-callouts > ol > li`); update the `<Callouts>`
content so each callout is a separate `<li>` inside an `<ol>` (i.e., convert the
numbered inline text into a proper ordered list with line breaks) to match the
Chinese version's formatting and ensure correct rendering.
---
Nitpick comments:
In `@packages/doom/src/plugins/directives/remark-directives.ts`:
- Around line 38-48: The helper setProperties currently uses the non-null
assertion node.attributes! when calling h, which is unsafe; update setProperties
to defensively handle missing attributes by passing node.attributes or an empty
object (e.g., node.attributes ?? {}) into h(data.hName || 'div', ...), and then
set data.hProperties as before (preserving n.properties and merging className)
so h is always invoked with a defined attributes object; ensure you reference
setProperties, node.attributes, h, data.hName, and data.hProperties when making
the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 658e47a1-d844-4270-a7ef-202ca7beafa2
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (12)
.changeset/seven-moments-take.mddocs/en/usage/markdown.mddocs/en/usage/mdx.mdxdocs/zh/usage/markdown.mddocs/zh/usage/mdx.mdxpackages/doom/package.jsonpackages/doom/src/plugins/directives/remark-directives.tspackages/doom/src/remark-lint/no-unmatched-anchor.tspackages/doom/src/runtime/components/Callout.tsxpackages/doom/src/runtime/components/Callouts.tsxpackages/doom/src/runtime/components/index.tspackages/doom/styles/global.scss
✅ Files skipped from review due to trivial changes (5)
- packages/doom/package.json
- .changeset/seven-moments-take.md
- packages/doom/src/runtime/components/index.ts
- docs/en/usage/markdown.md
- docs/zh/usage/markdown.md
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/doom/src/remark-lint/no-unmatched-anchor.ts
- packages/doom/src/runtime/components/Callout.tsx
close IDP-1438
There was a problem hiding this comment.
🧹 Nitpick comments (2)
docs/en/usage/mdx.mdx (2)
237-237: Use a hyphenated compound modifier in Line 237.Change “runtime related APIs” to “runtime-related APIs” for standard technical writing style.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/en/usage/mdx.mdx` at line 237, Update the phrase "runtime related APIs" to the hyphenated compound "runtime-related APIs" wherever it appears (specifically the sentence that begins "If you need to use more runtime related APIs...") to follow standard technical writing; edit the MDX content so the sentence reads "If you need to use more runtime-related APIs, you can implement components using `.jsx/.tsx` and then import and use them in `.mdx` files."
123-123: Consider tightening wording in Line 123 for readability.“exactly the same as that under” is a bit heavy; a shorter phrasing will read more clearly in docs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/en/usage/mdx.mdx` at line 123, Replace the verbose phrase "needs to be exactly the same as that under the `doc/zh` directory" in the sentence beginning "The directory structure of multilingual documents (`doc/en`)..." with a tighter wording such as "must mirror `doc/zh`" (e.g., "The directory structure of multilingual documents (`doc/en`) must mirror `doc/zh` so links differ only by the language identifier") to improve readability; locate and update the sentence in mdx.mdx where that fragment appears.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/en/usage/mdx.mdx`:
- Line 237: Update the phrase "runtime related APIs" to the hyphenated compound
"runtime-related APIs" wherever it appears (specifically the sentence that
begins "If you need to use more runtime related APIs...") to follow standard
technical writing; edit the MDX content so the sentence reads "If you need to
use more runtime-related APIs, you can implement components using `.jsx/.tsx`
and then import and use them in `.mdx` files."
- Line 123: Replace the verbose phrase "needs to be exactly the same as that
under the `doc/zh` directory" in the sentence beginning "The directory structure
of multilingual documents (`doc/en`)..." with a tighter wording such as "must
mirror `doc/zh`" (e.g., "The directory structure of multilingual documents
(`doc/en`) must mirror `doc/zh` so links differ only by the language
identifier") to improve readability; locate and update the sentence in mdx.mdx
where that fragment appears.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3c4efced-9f46-4110-9650-075cac66252e
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (12)
.changeset/seven-moments-take.mddocs/en/usage/markdown.mddocs/en/usage/mdx.mdxdocs/zh/usage/markdown.mddocs/zh/usage/mdx.mdxpackages/doom/package.jsonpackages/doom/src/plugins/directives/remark-directives.tspackages/doom/src/remark-lint/no-unmatched-anchor.tspackages/doom/src/runtime/components/Callout.tsxpackages/doom/src/runtime/components/Callouts.tsxpackages/doom/src/runtime/components/index.tspackages/doom/styles/global.scss
✅ Files skipped from review due to trivial changes (8)
- .changeset/seven-moments-take.md
- packages/doom/package.json
- packages/doom/src/runtime/components/index.ts
- packages/doom/src/remark-lint/no-unmatched-anchor.ts
- docs/en/usage/markdown.md
- packages/doom/src/runtime/components/Callout.tsx
- docs/zh/usage/mdx.mdx
- docs/zh/usage/markdown.md
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/doom/src/runtime/components/Callouts.tsx
- packages/doom/src/plugins/directives/remark-directives.ts
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
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.
close IDP-1438
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Chores