[feat]: bundler tabs#943
Conversation
📝 WalkthroughWalkthroughIntroduces bundler-specific tabbed documentation blocks by adding a persisted enum store utility, bundler type definitions, and a new BundlerTabs React component that syncs user bundler selection across the page. Updates markdown plugins to extract bundler panels from headings, refactors existing package manager tabs to use the new persistence pattern, and enhances tab component infrastructure with data-driven layout styling. ChangesBundler tabs feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/utils/markdown/plugins/transformTabsComponent.ts (1)
40-58:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHarden
data-attributesschema validation before returning parsed values.
JSON.parsesucceeding does not guarantee string-valued attributes. For example,{"variant":1}passes parsing but can throw later at Line 334 when calling.toLowerCase(). Return only string key/value pairs (or{}) to keep this transform resilient to malformed markdown metadata.Suggested fix
function parseAttributes(node: HastNode): Record<string, string> { const rawAttributes = node.properties?.['data-attributes'] if (typeof rawAttributes === 'string') { try { - return JSON.parse(rawAttributes) + const parsed: unknown = JSON.parse(rawAttributes) + if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) { + return {} + } + return Object.fromEntries( + Object.entries(parsed).filter( + (entry): entry is [string, string] => typeof entry[1] === 'string', + ), + ) } catch (error) { if (import.meta.env?.DEV) { // eslint-disable-next-line no-console console.warn( '[transformTabsComponent] Failed to parse data-attributes JSON:',🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/utils/markdown/plugins/transformTabsComponent.ts` around lines 40 - 58, The parseAttributes function currently returns whatever JSON.parse produces, which may include non-string values and later cause errors (e.g., calling .toLowerCase()); update parseAttributes to, after successful JSON.parse of node.properties['data-attributes'], validate the result is an object and create a new object that includes only keys whose values are strings (discard others), then return that filtered Record<string,string> (keep the existing try/catch and DEV console.warn behavior on parse failure); reference parseAttributes and node.properties['data-attributes'] when locating the change.
🧹 Nitpick comments (5)
src/styles/app.css (1)
1210-1215: ⚡ Quick winConsider using a data attribute for the title wrapper instead of relying on DOM structure.
The nested
> div:first-child > div:first-childselector is fragile and will break if the component DOM structure changes. A data attribute like[data-codeblock-title]would be more maintainable and self-documenting.♻️ More resilient alternative
In the component that renders the codeblock title wrapper, add a data attribute:
<div data-codeblock-title> {/* title content */} </div>Then update the CSS:
-[data-tab][data-content='code-only'] - .codeblock - > div:first-child - > div:first-child { +[data-tab][data-content='code-only'] .codeblock [data-codeblock-title] { `@apply` hidden; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/styles/app.css` around lines 1210 - 1215, The CSS rule using the fragile selector "[data-tab][data-content='code-only'] .codeblock > div:first-child > div:first-child" should be replaced by a data-attribute on the title wrapper to avoid DOM-structure coupling; add a data attribute (e.g. data-codeblock-title) to the element that wraps the codeblock title in the component and update the stylesheet to target "[data-tab][data-content='code-only'] .codeblock [data-codeblock-title]" (or simply "[data-codeblock-title]") and apply the same hidden styles so the rule references the stable attribute instead of chained :first-child selectors.src/components/markdown/BundlerTabs.tsx (2)
63-67: ⚡ Quick winSimplify fallback logic.
The current fallback chain
tabDefinitions[0]?.slug ?? activeBundlerwill useactiveBundleras the final fallback even when it's not present intabDefinitions. This could pass an invalid slug to theTabscomponent whentabDefinitionsis empty.Since line 78 guards against empty
tabDefinitions, the fallback will only execute when there's at least one tab. Consider simplifying:♻️ Proposed refactor
const resolvedActiveSlug = tabDefinitions.some( (tab) => tab.slug === activeBundler, ) ? activeBundler - : (tabDefinitions[0]?.slug ?? activeBundler) + : tabDefinitions[0]!.slugThe non-null assertion is safe here because line 78's early return guarantees
tabDefinitions.length > 0.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/markdown/BundlerTabs.tsx` around lines 63 - 67, resolvedActiveSlug fallback can incorrectly use activeBundler when tabDefinitions is empty; since the component already returns early when tabDefinitions.length === 0 (see the guard above), simplify the fallback to use the first tab's slug directly. Update the resolvedActiveSlug calculation to select activeBundler if present in tabDefinitions, otherwise use tabDefinitions[0].slug (with a non-null assertion or guaranteed access) so an invalid slug is not passed to Tabs; reference the variables resolvedActiveSlug, tabDefinitions, activeBundler and the Tabs component when making the change.
38-44: ⚡ Quick winConsider memoization stability.
The
childrenArraydependency is derived fromReact.Children.toArray(children)(line 36), which creates a new array reference on every render whenchildrenchanges. This causes thepanelsBySlugmemo to recompute even when the underlying children content hasn't changed.Since
childrenis already in scope, consider usingchildrendirectly as the dependency instead ofchildrenArray, or move thetoArraycall inside theuseMemoblock.♻️ Proposed refactor
- const childrenArray = React.Children.toArray(children) - const panelsBySlug = React.useMemo(() => { + const childrenArray = React.Children.toArray(children) const map = new Map<string, React.ReactNode>() tabs.forEach((tab, index) => { map.set(tab.slug, childrenArray[index]) }) return map - }, [tabs, childrenArray]) + }, [tabs, children])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/markdown/BundlerTabs.tsx` around lines 38 - 44, The memo for panelsBySlug is unstable because it depends on childrenArray (created via React.Children.toArray) which changes identity each render; fix by either moving React.Children.toArray(children) inside the React.useMemo callback and keeping [tabs, children] as dependencies, or keep childrenArray but replace it with children in the dependency array. Update the panelsBySlug logic (map.set(tab.slug, ...)) to use the locally created array inside the memo (if moving toArray) or continue using childrenArray but ensure useMemo deps are [tabs, children] so recomputation only occurs when actual children change.src/components/markdown/MdComponents.tsx (2)
152-152: 💤 Low valueRemove or clarify the
void indexstatement.The
void indexstatement on line 152 and its comment don't serve a clear purpose. The comment mentions "preserve insertion order for tabs that came in without metadata," but the code doesn't implement any order-preserving logic—indexis neither used nor stored.Consider removing both the statement and comment, or clarifying what ordering guarantee is intended.
♻️ Suggested cleanup
childrenBySlug.set(slug, panel.props.children) - // Preserve insertion order for tabs that came in without metadata - void index })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/markdown/MdComponents.tsx` at line 152, The stray "void index" expression and its comment in MdComponents.tsx (referring to index) do nothing and should be removed or made meaningful: either delete the "void index" line and the accompanying comment, or if you intended to preserve insertion order for tabs without metadata, capture and use that index (e.g., attach an insertionIndex field when building the tab objects and use it as a fallback key in the sorting/ordering logic such as in the code that renders or sorts tabs) so the comment matches implemented behavior; update references to use insertionIndex instead of the unused index variable.
207-209: Potentially unuseddata-tab-indexinMdTabPanelProps
data-tab-indexis written onmd-tab-panelelements bysrc/utils/markdown/plugins/transformTabsComponent.ts, but insrc/components/markdown/MdComponents.tsxit’s only present in theMdTabPanelPropstype and never read (MdTabPanel just renderschildren;data-tab-slugis the one consumed). Either removedata-tab-indexfromMdTabPanelPropsor document why it’s intentionally kept for future use.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/markdown/MdComponents.tsx` around lines 207 - 209, MdTabPanelProps declares 'data-tab-index' but MdTabPanel never reads it (MdTabPanel renders children and only consumes 'data-tab-slug'), so either remove 'data-tab-index' from the MdTabPanelProps type or add an explicit comment/docs indicating it's intentionally reserved; update the MdTabPanelProps declaration in MdComponents.tsx to drop 'data-tab-index' if unused, or add a clear comment on the property and keep it only if transformTabsComponent.ts relies on writing it for external tooling; ensure you update any related tests/types and keep references to MdTabPanel and the transformTabsComponent.ts behavior consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/utils/markdown/plugins/transformTabsComponent.ts`:
- Around line 40-58: The parseAttributes function currently returns whatever
JSON.parse produces, which may include non-string values and later cause errors
(e.g., calling .toLowerCase()); update parseAttributes to, after successful
JSON.parse of node.properties['data-attributes'], validate the result is an
object and create a new object that includes only keys whose values are strings
(discard others), then return that filtered Record<string,string> (keep the
existing try/catch and DEV console.warn behavior on parse failure); reference
parseAttributes and node.properties['data-attributes'] when locating the change.
---
Nitpick comments:
In `@src/components/markdown/BundlerTabs.tsx`:
- Around line 63-67: resolvedActiveSlug fallback can incorrectly use
activeBundler when tabDefinitions is empty; since the component already returns
early when tabDefinitions.length === 0 (see the guard above), simplify the
fallback to use the first tab's slug directly. Update the resolvedActiveSlug
calculation to select activeBundler if present in tabDefinitions, otherwise use
tabDefinitions[0].slug (with a non-null assertion or guaranteed access) so an
invalid slug is not passed to Tabs; reference the variables resolvedActiveSlug,
tabDefinitions, activeBundler and the Tabs component when making the change.
- Around line 38-44: The memo for panelsBySlug is unstable because it depends on
childrenArray (created via React.Children.toArray) which changes identity each
render; fix by either moving React.Children.toArray(children) inside the
React.useMemo callback and keeping [tabs, children] as dependencies, or keep
childrenArray but replace it with children in the dependency array. Update the
panelsBySlug logic (map.set(tab.slug, ...)) to use the locally created array
inside the memo (if moving toArray) or continue using childrenArray but ensure
useMemo deps are [tabs, children] so recomputation only occurs when actual
children change.
In `@src/components/markdown/MdComponents.tsx`:
- Line 152: The stray "void index" expression and its comment in
MdComponents.tsx (referring to index) do nothing and should be removed or made
meaningful: either delete the "void index" line and the accompanying comment, or
if you intended to preserve insertion order for tabs without metadata, capture
and use that index (e.g., attach an insertionIndex field when building the tab
objects and use it as a fallback key in the sorting/ordering logic such as in
the code that renders or sorts tabs) so the comment matches implemented
behavior; update references to use insertionIndex instead of the unused index
variable.
- Around line 207-209: MdTabPanelProps declares 'data-tab-index' but MdTabPanel
never reads it (MdTabPanel renders children and only consumes 'data-tab-slug'),
so either remove 'data-tab-index' from the MdTabPanelProps type or add an
explicit comment/docs indicating it's intentionally reserved; update the
MdTabPanelProps declaration in MdComponents.tsx to drop 'data-tab-index' if
unused, or add a clear comment on the property and keep it only if
transformTabsComponent.ts relies on writing it for external tooling; ensure you
update any related tests/types and keep references to MdTabPanel and the
transformTabsComponent.ts behavior consistent.
In `@src/styles/app.css`:
- Around line 1210-1215: The CSS rule using the fragile selector
"[data-tab][data-content='code-only'] .codeblock > div:first-child >
div:first-child" should be replaced by a data-attribute on the title wrapper to
avoid DOM-structure coupling; add a data attribute (e.g. data-codeblock-title)
to the element that wraps the codeblock title in the component and update the
stylesheet to target "[data-tab][data-content='code-only'] .codeblock
[data-codeblock-title]" (or simply "[data-codeblock-title]") and apply the same
hidden styles so the rule references the stable attribute instead of chained
:first-child selectors.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8446910c-af0c-4748-8799-84d6108434f4
📒 Files selected for processing (14)
docs-info.mdsrc/components/markdown/BundlerTabs.tsxsrc/components/markdown/CodeBlockView.tsxsrc/components/markdown/FileTabs.tsxsrc/components/markdown/MdComponents.tsxsrc/components/markdown/PackageManagerTabs.tsxsrc/components/markdown/Tabs.tsxsrc/components/markdown/index.tssrc/components/markdown/usePersistedEnumStore.tssrc/styles/app.csssrc/utils/markdown/bundler.tssrc/utils/markdown/installCommand.tssrc/utils/markdown/plugins/helpers.tssrc/utils/markdown/plugins/transformTabsComponent.ts
2 participants
Summary by CodeRabbit
New Features
Documentation