Skip to content

[Omnidoc] Fix prop comments of Label, LabelList, Text; extend generation#6749

Merged
PavelVanecek merged 8 commits intomainfrom
omnidoc
Dec 8, 2025
Merged

[Omnidoc] Fix prop comments of Label, LabelList, Text; extend generation#6749
PavelVanecek merged 8 commits intomainfrom
omnidoc

Conversation

@PavelVanecek
Copy link
Collaborator

@PavelVanecek PavelVanecek commented Dec 7, 2025

Description

Extends the generation script:

  • Formats the comments using marked
  • Generates component description, if available

Related Issue

#6069

Summary by CodeRabbit

  • New Features

    • Label: optional id for deterministic identifiers
    • LabelList: new customization props (id, valueAccessor, dataKey, angle, position, offset, formatter, content, clockwise)
    • Text: new width prop for wrapping; angle-related docs clarified
  • Documentation

    • API docs now support richer JSX content and updated entries for BarStack, Label, LabelList, and Text

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 7, 2025

Walkthrough

Convert omnidoc to async API-doc generation with JSX-capable descriptions, add JSDoc metadata access, parse per-prop markdown to JSX, introduce automated-component allowlist, update omnidoc tests/exceptions, and add optional public props to Label/LabelList/Text alongside migrated TSX API-doc files.

Changes

Cohort / File(s) Summary
Omnidoc pipeline & tooling
omnidoc/generateApiDoc.ts, omnidoc/readProject.ts, omnidoc/omnidoc.spec.ts
generateApiDoc made async and returns Promise; added OMNIDOC_AUTOMATED_API_DOCS_COMPONENTS; added getComponentJsDocMeta to ProjectDocReader; use marked.parse for prop descriptions; stringifyApiDoc emits JSX TSX outputs; tests filter auto-generated and inconsistent components.
Omnidoc exceptions & tests
omnidoc/cross-component-prop-comments.spec.ts, omnidoc/componentsWithInconsistentCommentsInApiDoc.ts
Broadened children exception to wildcard; added exceptions for id, dataKey, width; removed Label, LabelList, Text from componentsWithInconsistentCommentsInApiDoc.
Component public prop changes
src/component/Label.tsx, src/component/LabelList.tsx, src/component/Text.tsx
Introduced new optional public props: id (Label), multiple props including id, dataKey, valueAccessor, angle, content, etc. (LabelList), and width plus minor JSDoc updates (Text).
API docs migration & updates
www/src/docs/api/BarStack.ts, www/src/docs/api/BarStackAPI.tsx, www/src/docs/api/TextAPI.ts, www/src/docs/api/TextAPI.tsx, www/src/docs/api/Label.ts, www/src/docs/api/LabelList.ts, www/src/docs/api/index.ts
Replaced some .ts API-doc modules with .tsx JSX-capable ApiDoc exports (BarStack, Text); updated Label/LabelList API docs and index imports; added BarStack to examples; removed old .ts doc modules.
API types & locale handling
www/src/docs/api/types.ts, www/src/utils/LocaleUtils.ts, package.json
Change ApiDoc/ApiProps.desc types to accept ReactNode; Locale utils narrowed typing to SupportedLocale keys; added marked dependency.
Selectors & utilities
src/state/selectors/axisSelectors.ts, src/util/getActiveCoordinate.ts
Added selectUnfilteredCartesianItems selector; changed calculateActiveTickIndex return type from `number
Docs/exception list removal
omnidoc/componentsWithInconsistentCommentsInApiDoc.ts
Removed 'Label', 'LabelList', and 'Text' entries from the exported exception array.

Sequence Diagram(s)

sequenceDiagram
    participant Main as main()
    participant Gen as generateApiDoc()
    participant Reader as ProjectDocReader
    participant Marked as marked.parse()
    participant Stringify as stringifyApiDoc()
    participant FS as writeApiDocFile()

    Main->>Gen: await generateApiDoc(componentName)
    Gen->>Reader: getComponentJsDocMeta(componentName)
    Reader-->>Gen: JSDocMeta (may include `@since`)
    Gen->>Marked: await marked.parse(propComment)
    Marked-->>Gen: HTML/JSX nodes
    Gen->>Gen: assemble ApiDoc (desc may include component JSDoc)
    Gen-->>Main: Promise<ApiDoc>
    Main->>Stringify: stringifyApiDoc(apiDoc)
    Stringify-->>FS: TSX source string
    FS->>FS: write `${componentName}API.tsx`
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Focus review on: omnidoc/generateApiDoc.ts (async/marked/stringify logic), omnidoc/readProject.ts (JSDoc extraction and error handling), new TSX API doc files for correctness, and component prop additions in src/component/* for API stability.

Possibly related PRs

Suggested labels

typescript

Suggested reviewers

  • ckifer

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description check ❓ Inconclusive The description provides a brief overview but lacks sufficient detail about testing, implementation approach, and expected outcomes required by the template. Expand the description to include testing approach, motivation details, and whether this addresses breaking changes or new features in the type system updates.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main changes: fixing prop comments for Label, LabelList, and Text components, and extending the generation capabilities.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch omnidoc

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (7)
omnidoc/cross-component-prop-comments.spec.ts (1)

111-113: Consider extracting exception matching into a small helper

The inline exceptions.some(...) with both specific and '*' component matching is correct but a bit dense. A tiny helper like isExceptionFor(component, prop) would make the intent clearer and reusable if this logic grows.

-      if (
-        exceptions.some(ex => (ex.components.includes(component) || ex.components.includes('*')) && ex.prop === prop)
-      ) {
+      const isExceptionFor = (componentName: string, propName: string) =>
+        exceptions.some(
+          ex =>
+            (ex.components.includes(componentName) || ex.components.includes('*')) && ex.prop === propName,
+        );
+
+      if (isExceptionFor(component, prop)) {
         continue;
       }
www/src/docs/api/TextAPI.tsx (1)

1-1: Inconsistent import path: Remove .ts extension.

The import uses './types.ts' but www/src/docs/api/index.ts uses './types' without the extension. Keep imports consistent across the codebase.

-import { ApiDoc } from './types.ts';
+import { ApiDoc } from './types';
www/src/docs/api/BarStackAPI.tsx (1)

1-1: Inconsistent import path: Remove .ts extension.

Same issue as TextAPI.tsx – use './types' without the extension for consistency.

-import { ApiDoc } from './types.ts';
+import { ApiDoc } from './types';
omnidoc/generateApiDoc.ts (3)

128-134: Avoid as type assertion per coding guidelines.

The coding guidelines prohibit as assertions except for as const. Consider narrowing the type more explicitly or adjusting the ApiProps type to accept the known default value type.

One approach is to validate and narrow the type:

     // Add default value if available
     if (defaultValue.type === 'known') {
-      prop.defaultVal = defaultValue.value as
-        | string
-        | number
-        | boolean
-        | Array<unknown>
-        | Record<string, unknown>
-        | null;
+      const val = defaultValue.value;
+      if (
+        typeof val === 'string' ||
+        typeof val === 'number' ||
+        typeof val === 'boolean' ||
+        val === null ||
+        Array.isArray(val) ||
+        (typeof val === 'object' && val !== null)
+      ) {
+        prop.defaultVal = val;
+      }
     }

Alternatively, if defaultValue.value is already typed correctly in ProjectDocReader, consider updating ApiProps.defaultVal type to match.


164-195: Consider escaping special characters in stringifyApiDoc.

The function interpolates prop.name, prop.type, and apiDoc.desc directly into the output string without escaping. If any of these contain double quotes or backslashes, the generated file will be malformed.

Consider using JSON.stringify() for the string values:

   apiDoc.props.forEach(prop => {
     result += `{`;
-    result += `"name": "${prop.name}",`;
-    result += `"type": "${prop.type}",`;
+    result += `"name": ${JSON.stringify(prop.name)},`;
+    result += `"type": ${JSON.stringify(prop.type)},`;
     result += `"isOptional": ${prop.isOptional},`;
     // ...
   });
   // ...
   if (apiDoc.desc) {
-    result += `"desc": "${apiDoc.desc}",`;
+    result += `"desc": ${JSON.stringify(apiDoc.desc)},`;
   }

203-206: Generated import uses .ts extension inconsistently.

The template generates import { ApiDoc } from './types.ts' but www/src/docs/api/index.ts imports from './types' without the extension. Update the template to match the codebase convention.

-  const fileContent = `import { ApiDoc } from './types.ts';
+  const fileContent = `import { ApiDoc } from './types';

 export const ${varName}: ApiDoc = ${stringifyApiDoc(apiDoc)};
 `;
www/src/docs/api/Label.ts (1)

60-68: Consider adding type signature details.

While the formatter description mentions "only one parameter - the value of label," consider adding the parameter and return type information to the description for better clarity. For example: (value: string | number) => ReactNode or similar.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2792e96 and e71c74d.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (17)
  • omnidoc/componentsWithInconsistentCommentsInApiDoc.ts (0 hunks)
  • omnidoc/cross-component-prop-comments.spec.ts (3 hunks)
  • omnidoc/generateApiDoc.ts (7 hunks)
  • omnidoc/omnidoc.spec.ts (2 hunks)
  • omnidoc/readProject.ts (1 hunks)
  • package.json (1 hunks)
  • src/component/Label.tsx (2 hunks)
  • src/component/LabelList.tsx (1 hunks)
  • src/component/Text.tsx (6 hunks)
  • www/src/docs/api/BarStack.ts (0 hunks)
  • www/src/docs/api/BarStackAPI.tsx (1 hunks)
  • www/src/docs/api/Label.ts (3 hunks)
  • www/src/docs/api/LabelList.ts (5 hunks)
  • www/src/docs/api/TextAPI.ts (0 hunks)
  • www/src/docs/api/TextAPI.tsx (1 hunks)
  • www/src/docs/api/index.ts (2 hunks)
  • www/src/docs/api/types.ts (2 hunks)
💤 Files with no reviewable changes (3)
  • www/src/docs/api/BarStack.ts
  • omnidoc/componentsWithInconsistentCommentsInApiDoc.ts
  • www/src/docs/api/TextAPI.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

**/*.{ts,tsx}: Never use any type (implicit or explicit) in TypeScript code
Prefer unknown over any and refine the type in TypeScript
Type function parameters and return values explicitly in TypeScript, do not rely on implicit any or inference; exceptions are React components and trivial functions
Do not use as type assertions in TypeScript; the only exception is as const

Files:

  • www/src/docs/api/TextAPI.tsx
  • omnidoc/cross-component-prop-comments.spec.ts
  • src/component/LabelList.tsx
  • www/src/docs/api/types.ts
  • www/src/docs/api/LabelList.ts
  • omnidoc/readProject.ts
  • www/src/docs/api/BarStackAPI.tsx
  • omnidoc/omnidoc.spec.ts
  • src/component/Label.tsx
  • src/component/Text.tsx
  • www/src/docs/api/index.ts
  • omnidoc/generateApiDoc.ts
  • www/src/docs/api/Label.ts
**/*.{js,ts,tsx}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Ensure code lints by running npm run lint and follows Airbnb's Style Guide

Files:

  • www/src/docs/api/TextAPI.tsx
  • omnidoc/cross-component-prop-comments.spec.ts
  • src/component/LabelList.tsx
  • www/src/docs/api/types.ts
  • www/src/docs/api/LabelList.ts
  • omnidoc/readProject.ts
  • www/src/docs/api/BarStackAPI.tsx
  • omnidoc/omnidoc.spec.ts
  • src/component/Label.tsx
  • src/component/Text.tsx
  • www/src/docs/api/index.ts
  • omnidoc/generateApiDoc.ts
  • www/src/docs/api/Label.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (DEVELOPING.md)

All imports from recharts must use the public API entry point; imports from internal paths like recharts/types/* or recharts/src/* are not allowed

Files:

  • www/src/docs/api/TextAPI.tsx
  • omnidoc/cross-component-prop-comments.spec.ts
  • src/component/LabelList.tsx
  • www/src/docs/api/types.ts
  • www/src/docs/api/LabelList.ts
  • omnidoc/readProject.ts
  • www/src/docs/api/BarStackAPI.tsx
  • omnidoc/omnidoc.spec.ts
  • src/component/Label.tsx
  • src/component/Text.tsx
  • www/src/docs/api/index.ts
  • omnidoc/generateApiDoc.ts
  • www/src/docs/api/Label.ts
**/*.spec.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

When running unit tests, prefer to run a single test file using npm run test -- path/to/TestFile.spec.tsx rather than running all tests with npm test

Files:

  • omnidoc/cross-component-prop-comments.spec.ts
  • omnidoc/omnidoc.spec.ts
src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Do not hardcode any strings or formatting choices in library code; users should provide localized strings as desired

Files:

  • src/component/LabelList.tsx
  • src/component/Label.tsx
  • src/component/Text.tsx
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: recharts/recharts PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-25T01:22:59.729Z
Learning: Applies to storybook/stories/**/*.stories.tsx : Update Storybook stories when APIs have been changed to ensure they work as expected
📚 Learning: 2025-11-25T01:23:14.977Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T01:23:14.977Z
Learning: Applies to src/**/*.{ts,tsx} : Do not hardcode any strings or formatting choices in library code; users should provide localized strings as desired

Applied to files:

  • www/src/docs/api/TextAPI.tsx
📚 Learning: 2025-12-06T03:36:59.377Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: DEVELOPING.md:0-0
Timestamp: 2025-12-06T03:36:59.377Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : All imports from `recharts` must use the public API entry point; imports from internal paths like `recharts/types/*` or `recharts/src/*` are not allowed

Applied to files:

  • omnidoc/cross-component-prop-comments.spec.ts
  • www/src/docs/api/BarStackAPI.tsx
  • www/src/docs/api/index.ts
📚 Learning: 2025-11-25T01:22:59.729Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-25T01:22:59.729Z
Learning: Applies to storybook/stories/**/*.stories.tsx : Update Storybook stories when APIs have been changed to ensure they work as expected

Applied to files:

  • omnidoc/omnidoc.spec.ts
  • www/src/docs/api/index.ts
  • omnidoc/generateApiDoc.ts
🧬 Code graph analysis (6)
www/src/docs/api/TextAPI.tsx (1)
www/src/docs/api/types.ts (1)
  • ApiDoc (21-27)
src/component/LabelList.tsx (2)
src/util/types.ts (1)
  • DataKey (60-60)
src/component/Label.tsx (2)
  • LabelContentType (33-33)
  • LabelPosition (64-64)
www/src/docs/api/types.ts (1)
www/src/locale/index.ts (1)
  • SupportedLocale (4-4)
www/src/docs/api/BarStackAPI.tsx (1)
www/src/docs/api/types.ts (1)
  • ApiDoc (21-27)
src/component/Label.tsx (3)
src/util/types.ts (1)
  • ViewBox (874-874)
src/component/Text.tsx (1)
  • RenderableText (66-66)
src/index.ts (1)
  • RenderableText (25-25)
omnidoc/generateApiDoc.ts (1)
www/src/docs/api/types.ts (1)
  • ApiDoc (21-27)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Build, Test, Pack
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (15)
omnidoc/cross-component-prop-comments.spec.ts (1)

44-95: Exception list expansion and wildcard semantics look sound

Using components: ['*'] for children and adding focused exceptions for id, dataKey, and width matches the intent to avoid enforcing cross-component comment similarity where semantics genuinely diverge. The data and rationale text stay consistent with that behavior.

src/component/Text.tsx (1)

79-82: Text angle/width APIs and transform logic align with the docs

The new angle and width props are documented clearly, wired into getWordsByLines and the transform pipeline, and keep existing scale/wrapping behavior intact. The rotate transform around (x, y) and the standardized @defaultValue tags look consistent with the intended public API.

Also applies to: 83-92, 100-116, 130-133, 140-142, 170-174, 386-396, 422-433

omnidoc/omnidoc.spec.ts (1)

11-11: Automated-components filter cleanly narrows the API-doc comparison set

Using OMNIDOC_AUTOMATED_API_DOCS_COMPONENTS to pre-filter getPublicComponentNames() before the known-inconsistencies list is a straightforward way to skip auto-generated docs that are guaranteed to match, without changing the rest of the test behavior.

Also applies to: 267-277

omnidoc/readProject.ts (1)

554-564: New getComponentJsDocMeta helper is a minimal, well-scoped addition

The method cleanly reuses existing primitives (getComponentDeclaration and getJsDocMeta) and safely returns undefined when a component isn’t found, which is exactly what higher-level doc tooling needs without changing any existing code paths.

www/src/docs/api/types.ts (1)

1-1: ReactNode-based desc typing matches richer generated docs

Allowing desc to be ReactNode (optionally per-locale) aligns with the JSX-enriched descriptions produced by the new omnidoc pipeline and keeps the existing shape of the API doc objects intact.

Also applies to: 10-16, 21-24

www/src/docs/api/LabelList.ts (1)

5-16: LabelList API copy and new angle prop documentation are consistent

The expanded descriptions for dataKey, valueAccessor, content, position, and offset improve clarity, and the new angle prop (default 0, rotation around (x, y)) is documented in a way that matches the behavior exposed on the underlying text components.

Also applies to: 23-25, 33-36, 54-56, 64-66, 68-78

package.json (1)

163-163: Adding marked as a devDependency is appropriate and API-compatible

Including "marked": "^17.0.1" in devDependencies is correct for the omnidoc tooling that parses markdown in component documentation. The code in omnidoc/generateApiDoc.ts uses the v17 API correctly—it imports with import { marked } from 'marked' (line 15) and calls await marked.parse(comment) (line 122), which matches the v17 ESM API and properly awaits the returned Promise.

www/src/docs/api/index.ts (1)

33-33: LGTM!

The import path updates (removing .ts extensions) and new API doc entries for BarStack and Text follow the existing patterns in this file.

Also applies to: 52-53, 70-70, 85-85

www/src/docs/api/TextAPI.tsx (1)

3-227: LGTM!

The TextAPI documentation is comprehensive and well-structured. The JSX-based descriptions provide clear explanations of prop interactions (e.g., maxLines with width and scaleToFit), which will be valuable for API documentation consumers.

www/src/docs/api/BarStackAPI.tsx (1)

3-60: LGTM!

The BarStackAPI documentation clearly explains the behavior of radius across stacked bars and the stackId precedence rules. The @since information is helpful for version awareness.

omnidoc/generateApiDoc.ts (1)

97-156: LGTM on async implementation.

The async conversion is well-implemented. The no-await-in-loop disables are justified since props are processed sequentially and the component JSDoc metadata retrieval is appropriately integrated.

Also applies to: 215-231

src/component/LabelList.tsx (1)

53-107: LGTM on JSDoc documentation!

The prop documentation is comprehensive and clearly explains:

  • The precedence between dataKey and valueAccessor
  • The purpose of id for SSR scenarios
  • The rotation behavior for angle with proper @defaultValue tag
  • Component-specific context (e.g., clockWise for radial charts, dataKey required for Scatter)
www/src/docs/api/Label.ts (3)

14-23: Improved clarity on directional behavior.

The updated description effectively clarifies that the offset direction depends on the position value.


35-44: Good consistency with children prop.

The value prop documentation is clear and consistent with the children prop description.


45-59: Consistent prop documentation.

The children prop description now properly references the value prop alternative, maintaining consistency across the API documentation.

* Scatter requires this prop to be set.
* Other graphical components will show the same value as the dataKey of the component by default.
*/
dataKey?: DataKey<Record<string, any>>;
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for DataKey<Record<string, patterns to assess consistency
rg -n "DataKey<Record<string," --type=ts --type=tsx

Repository: recharts/recharts

Length of output: 88


🏁 Script executed:

#!/bin/bash
# Also search for broader DataKey usage patterns
rg -n "DataKey<" --type=ts --type=tsx | head -30

Repository: recharts/recharts

Length of output: 88


🏁 Script executed:

#!/bin/bash
# Check the DataKey type definition
cat -n src/util/types.ts | sed -n '50,70p'

Repository: recharts/recharts

Length of output: 1269


🏁 Script executed:

#!/bin/bash
# Check the actual LabelList.tsx file around line 79 for context
cat -n src/component/LabelList.tsx | sed -n '70,90p'

Repository: recharts/recharts

Length of output: 1013


🏁 Script executed:

#!/bin/bash
# Search for DataKey usage patterns with correct rg syntax
rg "DataKey<Record<string," -t ts

Repository: recharts/recharts

Length of output: 130


🏁 Script executed:

#!/bin/bash
# Search all DataKey usage more broadly
rg "DataKey<" -t ts -A 0 | head -40

Repository: recharts/recharts

Length of output: 3224


🏁 Script executed:

#!/bin/bash
# Check if Record<string, any> appears in other prop definitions
rg "Record<string, any>" -t ts

Repository: recharts/recharts

Length of output: 3178


🏁 Script executed:

#!/bin/bash
# Check for all uses of 'any' in LabelList.tsx
rg "any" src/component/LabelList.tsx -n

Repository: recharts/recharts

Length of output: 201


Replace Record<string, any> with Record<string, unknown> to comply with type safety guidelines.

The Record<string, any> parameter violates the coding guideline to prefer unknown over any in TypeScript. While the DataKey type definition itself uses any in the function callback return type, this specific instance can be improved by narrowing the input object type.

-  dataKey?: DataKey<Record<string, any>>;
+  dataKey?: DataKey<Record<string, unknown>>;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
dataKey?: DataKey<Record<string, any>>;
dataKey?: DataKey<Record<string, unknown>>;
🤖 Prompt for AI Agents
In src/component/LabelList.tsx around line 79, the property type uses
DataKey<Record<string, any>> which violates the guideline preferring unknown
over any; change the generic to DataKey<Record<string, unknown>> so the input
object is typed with unknown instead of any, update any related imports/types if
needed to keep compatibility, and run TypeScript to ensure no downstream type
errors remain.

Comment on lines +91 to +100
{
name: 'viewBox',
type: 'String | Number',
isOptional: false,
desc: {
'en-US':
'The box of viewing area. Used for positioning. If undefined, viewBox will be calculated based on surrounding context.',
'zh-CN': '图表的可视区域。',
},
},
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 7, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Fix contradiction between isOptional and description.

There's a critical inconsistency: isOptional: false indicates the prop is required, but the description states "If undefined, viewBox will be calculated based on surrounding context," which implies it's optional.

Please clarify and correct:

  • If viewBox is truly required, remove the "If undefined" clause from the description
  • If viewBox can be undefined and auto-calculated, change isOptional: false to isOptional: true

Apply this diff if viewBox is actually optional:

     {
       name: 'viewBox',
       type: 'String | Number',
-      isOptional: false,
+      isOptional: true,
       desc: {
         'en-US':
           'The box of viewing area. Used for positioning. If undefined, viewBox will be calculated based on surrounding context.',
         'zh-CN': '图表的可视区域。',
       },
     },

Or apply this diff if viewBox is actually required:

     {
       name: 'viewBox',
       type: 'String | Number',
       isOptional: false,
       desc: {
         'en-US':
-          'The box of viewing area. Used for positioning. If undefined, viewBox will be calculated based on surrounding context.',
+          'The box of viewing area. Used for positioning.',
         'zh-CN': '图表的可视区域。',
       },
     },
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
{
name: 'viewBox',
type: 'String | Number',
isOptional: false,
desc: {
'en-US':
'The box of viewing area. Used for positioning. If undefined, viewBox will be calculated based on surrounding context.',
'zh-CN': '图表的可视区域。',
},
},
{
name: 'viewBox',
type: 'String | Number',
isOptional: true,
desc: {
'en-US':
'The box of viewing area. Used for positioning. If undefined, viewBox will be calculated based on surrounding context.',
'zh-CN': '图表的可视区域。',
},
},
Suggested change
{
name: 'viewBox',
type: 'String | Number',
isOptional: false,
desc: {
'en-US':
'The box of viewing area. Used for positioning. If undefined, viewBox will be calculated based on surrounding context.',
'zh-CN': '图表的可视区域。',
},
},
{
name: 'viewBox',
type: 'String | Number',
isOptional: false,
desc: {
'en-US':
'The box of viewing area. Used for positioning.',
'zh-CN': '图表的可视区域。',
},
},
🤖 Prompt for AI Agents
In www/src/docs/api/Label.ts around lines 91 to 100, the prop metadata for
viewBox is inconsistent: isOptional is set to false but the description says "If
undefined, viewBox will be calculated...", implying optional. Decide which
behavior is correct and make one of two fixes: if viewBox can be omitted and
will be auto-calculated, change isOptional to true; otherwise (viewBox required)
remove the "If undefined..." clause from the description so it no longer
suggests optionality. Ensure the final description matches the chosen
required/optional status.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I will fix in next PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

@PavelVanecek, understood! Would you like me to create a follow-up issue to track this inconsistency fix for the viewBox property?

@ckifer
Copy link
Member

ckifer commented Dec 8, 2025

some typecheck failures looks like

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/util/getActiveCoordinate.ts (1)

157-157: Critical: Type mismatch in return statements.

The optional chaining operator at lines 157 and 164 (unsortedTicks[i]?.index) returns number | undefined, but the function is declared to return number. This creates a TypeScript compilation error, which likely explains the typecheck failures mentioned in the PR comments.

Since the guards at lines 129-131 ensure that unsortedTicks[i] exists (by checking that cur, which is unsortedTicks[i]?.coordinate, is not null), the optional chaining is unnecessary.

Apply this diff to remove the unnecessary optional chaining:

-        return unsortedTicks[i]?.index;
+        return unsortedTicks[i].index;

And at line 164:

-        return unsortedTicks[i]?.index;
+        return unsortedTicks[i].index;

Alternatively, if you want to be defensive, use nullish coalescing with a fallback:

-        return unsortedTicks[i]?.index;
+        return unsortedTicks[i]?.index ?? -1;

Also applies to: 164-164

♻️ Duplicate comments (2)
src/component/LabelList.tsx (1)

79-79: DataKey<Record<string, any>> is acceptable per codebase conventions.

Based on learnings, DataKey<any> is an intentional exception to the "no any" rule in this codebase while proper typing is being developed. The past review comment suggesting Record<string, unknown> can be disregarded for this specific case.

www/src/docs/api/Label.ts (1)

24-34: Clarify angle pivot coordinates and reconcile viewBox optionality

Two doc issues here that were already raised in earlier review feedback:

  1. angle description (Lines 24–34) mentions rotation around the (x, y) coordinates, but x/y aren’t documented in this API surface. Please either:

    • Explicitly state that this is rotation around the label’s position (x, y) — the SVG text position attributes, or
    • Add brief entries or a note explaining what x and y represent for Label.
  2. viewBox (Lines 91–100) has isOptional: false but the English description says “If undefined, viewBox will be calculated…”, which implies it’s optional. These need to agree:

    • If viewBox can be omitted and auto-calculated, change isOptional to true.
    • If it is truly required, remove the “If undefined…” clause from the description.

Also applies to: 91-100

🧹 Nitpick comments (1)
omnidoc/generateApiDoc.ts (1)

215-241: Align main behavior and error handling with the CLI contract

Two small mismatches here:

  1. The comment above OMNIDOC_AUTOMATED_API_DOCS_COMPONENTS still says “If no components are specified on the command line, these will be generated… Components not in this list can still be generated by passing them as arguments”, but main() now ignores process.argv and always uses the allowlist. Either restore argument handling or update the comment to match the new behavior.

  2. main is now async but invoked as main(); without awaiting or catching. That means errors may surface only as unhandled rejections and some linters (no-floating-promises-style rules) may complain.

Consider something like:

-async function main() {
+async function main() {
   const componentsToGenerate = OMNIDOC_AUTOMATED_API_DOCS_COMPONENTS;
   // ...
 }
 
 if (require.main === module) {
-  main();
+  main().catch(error => {
+    // eslint-disable-next-line no-console
+    console.error(error);
+    process.exitCode = 1;
+  });
 }

and (optionally) reintroducing process.argv.slice(2) to honor CLI overrides if that’s still desired.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e71c74d and b8ea20c.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (20)
  • omnidoc/componentsWithInconsistentCommentsInApiDoc.ts (0 hunks)
  • omnidoc/cross-component-prop-comments.spec.ts (3 hunks)
  • omnidoc/generateApiDoc.ts (7 hunks)
  • omnidoc/omnidoc.spec.ts (2 hunks)
  • omnidoc/readProject.ts (1 hunks)
  • package.json (1 hunks)
  • src/component/Label.tsx (2 hunks)
  • src/component/LabelList.tsx (1 hunks)
  • src/component/Text.tsx (6 hunks)
  • src/state/selectors/axisSelectors.ts (1 hunks)
  • src/util/getActiveCoordinate.ts (1 hunks)
  • www/src/docs/api/BarStack.ts (0 hunks)
  • www/src/docs/api/BarStackAPI.tsx (1 hunks)
  • www/src/docs/api/Label.ts (3 hunks)
  • www/src/docs/api/LabelList.ts (5 hunks)
  • www/src/docs/api/TextAPI.ts (0 hunks)
  • www/src/docs/api/TextAPI.tsx (1 hunks)
  • www/src/docs/api/index.ts (2 hunks)
  • www/src/docs/api/types.ts (2 hunks)
  • www/src/utils/LocaleUtils.ts (1 hunks)
💤 Files with no reviewable changes (3)
  • www/src/docs/api/BarStack.ts
  • omnidoc/componentsWithInconsistentCommentsInApiDoc.ts
  • www/src/docs/api/TextAPI.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/component/Text.tsx
  • package.json
  • www/src/docs/api/index.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

**/*.{ts,tsx}: Never use any type (implicit or explicit) in TypeScript code
Prefer unknown over any and refine the type in TypeScript
Type function parameters and return values explicitly in TypeScript, do not rely on implicit any or inference; exceptions are React components and trivial functions
Do not use as type assertions in TypeScript; the only exception is as const

Files:

  • src/util/getActiveCoordinate.ts
  • www/src/utils/LocaleUtils.ts
  • omnidoc/omnidoc.spec.ts
  • omnidoc/readProject.ts
  • www/src/docs/api/TextAPI.tsx
  • omnidoc/cross-component-prop-comments.spec.ts
  • src/state/selectors/axisSelectors.ts
  • src/component/Label.tsx
  • www/src/docs/api/Label.ts
  • www/src/docs/api/types.ts
  • www/src/docs/api/LabelList.ts
  • www/src/docs/api/BarStackAPI.tsx
  • omnidoc/generateApiDoc.ts
  • src/component/LabelList.tsx
**/*.{js,ts,tsx}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Ensure code lints by running npm run lint and follows Airbnb's Style Guide

Files:

  • src/util/getActiveCoordinate.ts
  • www/src/utils/LocaleUtils.ts
  • omnidoc/omnidoc.spec.ts
  • omnidoc/readProject.ts
  • www/src/docs/api/TextAPI.tsx
  • omnidoc/cross-component-prop-comments.spec.ts
  • src/state/selectors/axisSelectors.ts
  • src/component/Label.tsx
  • www/src/docs/api/Label.ts
  • www/src/docs/api/types.ts
  • www/src/docs/api/LabelList.ts
  • www/src/docs/api/BarStackAPI.tsx
  • omnidoc/generateApiDoc.ts
  • src/component/LabelList.tsx
src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Do not hardcode any strings or formatting choices in library code; users should provide localized strings as desired

Files:

  • src/util/getActiveCoordinate.ts
  • src/state/selectors/axisSelectors.ts
  • src/component/Label.tsx
  • src/component/LabelList.tsx
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (DEVELOPING.md)

All imports from recharts must use the public API entry point; imports from internal paths like recharts/types/* or recharts/src/* are not allowed

Files:

  • src/util/getActiveCoordinate.ts
  • www/src/utils/LocaleUtils.ts
  • omnidoc/omnidoc.spec.ts
  • omnidoc/readProject.ts
  • www/src/docs/api/TextAPI.tsx
  • omnidoc/cross-component-prop-comments.spec.ts
  • src/state/selectors/axisSelectors.ts
  • src/component/Label.tsx
  • www/src/docs/api/Label.ts
  • www/src/docs/api/types.ts
  • www/src/docs/api/LabelList.ts
  • www/src/docs/api/BarStackAPI.tsx
  • omnidoc/generateApiDoc.ts
  • src/component/LabelList.tsx
**/*.spec.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

When running unit tests, prefer to run a single test file using npm run test -- path/to/TestFile.spec.tsx rather than running all tests with npm test

Files:

  • omnidoc/omnidoc.spec.ts
  • omnidoc/cross-component-prop-comments.spec.ts
🧠 Learnings (9)
📓 Common learnings
Learnt from: CR
Repo: recharts/recharts PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-25T01:22:59.729Z
Learning: Applies to storybook/stories/**/*.stories.tsx : Update Storybook stories when APIs have been changed to ensure they work as expected
📚 Learning: 2025-11-25T01:23:14.977Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T01:23:14.977Z
Learning: Applies to src/**/*.{ts,tsx} : Do not hardcode any strings or formatting choices in library code; users should provide localized strings as desired

Applied to files:

  • www/src/utils/LocaleUtils.ts
  • www/src/docs/api/TextAPI.tsx
📚 Learning: 2025-12-06T03:36:59.377Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: DEVELOPING.md:0-0
Timestamp: 2025-12-06T03:36:59.377Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : All imports from `recharts` must use the public API entry point; imports from internal paths like `recharts/types/*` or `recharts/src/*` are not allowed

Applied to files:

  • omnidoc/cross-component-prop-comments.spec.ts
  • www/src/docs/api/BarStackAPI.tsx
📚 Learning: 2025-11-23T13:30:10.395Z
Learnt from: PavelVanecek
Repo: recharts/recharts PR: 6669
File: www/src/docs/exampleComponents/ScatterChart/ScatterChartWithLabels.tsx:2-2
Timestamp: 2025-11-23T13:30:10.395Z
Learning: The `TooltipIndex` type from recharts is defined in `src/state/tooltipSlice.ts` but is not currently exported from the public API entry points. It should not be imported from `recharts/types/state/tooltipSlice` as this is an internal implementation path. An ESLint rule is needed to prevent regressions.

Applied to files:

  • src/state/selectors/axisSelectors.ts
📚 Learning: 2025-11-25T01:23:08.250Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-11-25T01:23:08.250Z
Learning: Applies to test/**/*.{test,spec}.{ts,tsx} : Verify the number of selector calls using the spy object from `createSelectorTestCase` to spot unnecessary re-renders and improve performance

Applied to files:

  • src/state/selectors/axisSelectors.ts
📚 Learning: 2025-12-08T08:23:26.219Z
Learnt from: PavelVanecek
Repo: recharts/recharts PR: 6750
File: src/state/selectors/axisSelectors.ts:593-602
Timestamp: 2025-12-08T08:23:26.219Z
Learning: In the recharts codebase, `DataKey<any>` is an intentional exception to the "no any" rule while proper typing is being developed. This should not be flagged in reviews.

Applied to files:

  • src/component/Label.tsx
  • src/component/LabelList.tsx
📚 Learning: 2025-11-25T01:22:59.729Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-25T01:22:59.729Z
Learning: Applies to **/*.{ts,tsx} : Prefer `unknown` over `any` and refine the type in TypeScript

Applied to files:

  • src/component/LabelList.tsx
📚 Learning: 2025-11-25T01:22:59.729Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-25T01:22:59.729Z
Learning: Applies to **/*.{ts,tsx} : Type function parameters and return values explicitly in TypeScript, do not rely on implicit any or inference; exceptions are React components and trivial functions

Applied to files:

  • src/component/LabelList.tsx
📚 Learning: 2025-11-25T01:22:59.729Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-25T01:22:59.729Z
Learning: Applies to **/*.{ts,tsx} : Never use `any` type (implicit or explicit) in TypeScript code

Applied to files:

  • src/component/LabelList.tsx
🧬 Code graph analysis (8)
www/src/utils/LocaleUtils.ts (1)
www/src/locale/index.ts (1)
  • SupportedLocale (4-4)
omnidoc/omnidoc.spec.ts (2)
omnidoc/generateApiDoc.ts (1)
  • OMNIDOC_AUTOMATED_API_DOCS_COMPONENTS (24-28)
omnidoc/componentsWithInconsistentCommentsInApiDoc.ts (1)
  • componentsWithInconsistentCommentsInApiDoc (5-36)
www/src/docs/api/TextAPI.tsx (1)
www/src/docs/api/types.ts (1)
  • ApiDoc (21-27)
src/component/Label.tsx (3)
src/util/types.ts (1)
  • ViewBox (874-874)
src/component/Text.tsx (1)
  • RenderableText (66-66)
src/index.ts (1)
  • RenderableText (25-25)
www/src/docs/api/types.ts (1)
www/src/locale/index.ts (1)
  • SupportedLocale (4-4)
www/src/docs/api/BarStackAPI.tsx (1)
www/src/docs/api/types.ts (1)
  • ApiDoc (21-27)
omnidoc/generateApiDoc.ts (1)
www/src/docs/api/types.ts (1)
  • ApiDoc (21-27)
src/component/LabelList.tsx (2)
src/util/types.ts (1)
  • DataKey (60-60)
src/component/Label.tsx (2)
  • LabelContentType (33-33)
  • LabelPosition (64-64)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Build, Test, Pack
🔇 Additional comments (12)
www/src/utils/LocaleUtils.ts (2)

35-35: LGTM! Type safety improvement.

Restricting the type from Partial<Record<string, ReactNode>> to Partial<Record<SupportedLocale, ReactNode>> provides better type safety by ensuring only valid locale keys can be used.


42-48: The behavior change from truthy-based (||) to key-existence-based (in operator) is intentional and more semantically correct. However, this concern has no practical impact in the codebase: all locale entries are complete with both 'en-US' and 'zh-CN' keys defined with non-empty values. The function correctly handles missing locales by falling back to 'en-US', and the initial check if (!value) at line 37 prevents undefined values from reaching the key-existence logic. This change actually improves correctness by respecting explicitly set values.

omnidoc/cross-component-prop-comments.spec.ts (2)

44-95: LGTM! Well-structured exception handling with wildcard support.

The addition of the '*' wildcard pattern for universal exceptions and the three new component-specific exceptions for Label, LabelList, and Text are cleanly implemented. The rationale for each exception is clear and aligns with the PR objectives.


111-115: Correct wildcard filtering logic.

The condition properly checks both specific component membership and the universal wildcard, ensuring exceptions apply correctly.

src/component/LabelList.tsx (1)

53-107: LGTM! Comprehensive JSDoc documentation for LabelList props.

The added documentation clearly explains each prop's purpose, including:

  • Precedence rules (valueAccessor vs dataKey)
  • Rotation behavior for angle
  • Default values where applicable
  • Component-specific requirements (e.g., Scatter requires dataKey)

This aligns well with the PR objective to fix prop comments.

src/component/Label.tsx (2)

69-128: LGTM! Well-documented Label props with SSR-friendly id support.

The JSDoc additions provide clear documentation for all props. The new id prop enables deterministic clip path IDs for SSR scenarios, which is properly implemented at line 273 where it falls back to uniqueId() when not provided.


109-116: Angle prop documentation is accurate.

The documentation correctly describes the rotation behavior. As confirmed in past review discussions, the angle prop is properly wired through svgPropertiesAndEvents to the Text component.

omnidoc/readProject.ts (1)

553-564: LGTM! Clean addition for component-level JSDoc metadata retrieval.

The new getComponentJsDocMeta method follows the established patterns in the class:

  • Consistent try/catch error handling returning undefined on failure
  • Reuses existing private methods (getComponentDeclaration, getJsDocMeta)
  • Clear JSDoc comment explaining the purpose

This supports the broader API doc generation flow introduced in this PR.

omnidoc/omnidoc.spec.ts (2)

11-11: Import added for auto-generated component filtering.

The import of OMNIDOC_AUTOMATED_API_DOCS_COMPONENTS is correctly used to filter out auto-generated components from the consistency tests.


267-278: LGTM! Smart test filtering for API doc consistency.

The two-stage filtering is well-reasoned:

  1. Auto-generated components (OMNIDOC_AUTOMATED_API_DOCS_COMPONENTS) are excluded because they're guaranteed to match by design
  2. Components with known inconsistencies are excluded as they're work-in-progress

The inline comments clearly explain the rationale for each filter.

www/src/docs/api/BarStackAPI.tsx (1)

1-60: BarStackAPI shape and JSX descriptions look correct

The BarStackAPI object matches the ApiDoc/ApiProps contract, uses localized JSX descriptions correctly, and the documented radius / stackId behavior is coherent. No changes needed here.

www/src/docs/api/types.ts (1)

1-27: ReactNode-aware desc typings align with JSX-based docs

Extending ApiProps.desc and ApiDoc.desc to support ReactNode per-locale values is consistent with the new TSX API-doc files and remains backward-compatible with string descriptions.

Comment on lines +164 to 195
function stringifyApiDoc(apiDoc: ApiDoc): string {
let result = `{"name": "${apiDoc.name}","props": [`;

apiDoc.props.forEach(prop => {
result += `{`;
result += `"name": "${prop.name}",`;
result += `"type": "${prop.type}",`;
result += `"isOptional": ${prop.isOptional},`;

if (prop.desc) {
result += `"desc": {`;
for (const [locale, html] of Object.entries(prop.desc)) {
// Write HTML as JSX without quotes. Wrap in <section> to ensure valid JSX.
result += `"${locale}": (<section>${html}</section>),`;
}
result += `},`;
}

if (prop.defaultVal !== undefined) {
result += `"defaultVal": ${JSON.stringify(prop.defaultVal)},`;
}

result += `},`;
});
result += `],`;
// Add component-level description if available │
if (apiDoc.desc) {
result += `"desc": "${apiDoc.desc}",`;
}
result += `}`;
return result;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Fix stringifyApiDoc to handle desc’s union type safely (likely source of typecheck failure)

ApiProps.desc is now typed as string | Partial<Record<SupportedLocale, ReactNode>>, but stringifyApiDoc assumes it’s always an object:

if (prop.desc) {
  result += `"desc": {`;
  for (const [locale, html] of Object.entries(prop.desc)) {
    // ...
  }
  result += `},`;
}

This causes two problems:

  • TypeScript: Object.entries expects an object; string | ... doesn’t satisfy that, which matches the reported typecheck failures.
  • Runtime: if any ApiProps ever uses a plain string desc, Object.entries will iterate characters instead of locales.

You can fix both by narrowing on type and emitting strings vs locale maps separately, without using as:

-    if (prop.desc) {
-      result += `"desc": {`;
-      for (const [locale, html] of Object.entries(prop.desc)) {
-        // Write HTML as JSX without quotes. Wrap in <section> to ensure valid JSX.
-        result += `"${locale}": (<section>${html}</section>),`;
-      }
-      result += `},`;
-    }
+    const { desc } = prop;
+    if (typeof desc === 'string') {
+      result += `"desc": "${desc}",`;
+    } else if (desc) {
+      result += `"desc": {`;
+      for (const [locale, html] of Object.entries(desc)) {
+        // Write HTML as JSX without quotes. Wrap in <section> to ensure valid JSX.
+        result += `"${locale}": (<section>${html}</section>),`;
+      }
+      result += `},`;
+    }

Given the updated typings and the maintainer comment about typecheck problems, this is important to fix before relying on the new generator.

🤖 Prompt for AI Agents
In omnidoc/generateApiDoc.ts around lines 164–195, stringifyApiDoc currently
assumes prop.desc is always an object and calls Object.entries on it, which
breaks when desc is a string; change the logic to narrow the union: if typeof
prop.desc === "string" emit a JSON string value for "desc" (properly escaped /
JSON.stringify the string) and if it's an object (truthy) iterate
Object.entries(prop.desc) as before to emit the locale ->
(<section>...</section>) JSX entries; do the same kind of type check for the
component-level apiDoc.desc before emitting it; avoid using casts and ensure you
only call Object.entries when typeof desc === "object" (and not null).

Comment on lines +318 to 321
// TODO appears there is a bug where this selector is called from polar context, find and fix it.
export const selectUnfilteredCartesianItems = (
state: RechartsRootState,
): ReadonlyArray<CartesianGraphicalItemSettings> => state.graphicalItems.cartesianItems;
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find all usages of selectUnfilteredCartesianItems
rg -n "selectUnfilteredCartesianItems" -S

Repository: recharts/recharts

Length of output: 3431


🏁 Script executed:

#!/bin/bash
# Find polar selector files
fd -e "ts" "Selector" src/state/selectors/ | grep -i polar

# Also check if there are any polar-specific graphical item selectors
rg -l "polar" src/state/selectors/ --type ts

Repository: recharts/recharts

Length of output: 673


🏁 Script executed:

#!/bin/bash
# Check if polar selectors import or use selectUnfilteredCartesianItems
for file in src/state/selectors/polarSelectors.ts src/state/selectors/radarSelectors.ts src/state/selectors/radialBarSelectors.ts src/state/selectors/pieSelectors.ts; do
  echo "=== $file ==="
  grep -n "selectUnfilteredCartesianItems" "$file" || echo "Not found"
done

Repository: recharts/recharts

Length of output: 286


Selector implementation is correct; TODO about polar context misuse appears unfounded

The selectUnfilteredCartesianItems implementation is solid with proper types and no use of any. However, verification shows this selector is used exclusively by Cartesian-specific selectors (area, bar, line, scatter) and is not imported by any polar chart selectors (polar, radar, radialBar, pie). The TODO comment warning about "this selector called from polar context" does not reflect actual usage patterns in the codebase.

Either remove the TODO if it's outdated, or replace it with a more concrete note if there's a documented issue elsewhere:

-// TODO appears there is a bug where this selector is called from polar context, find and fix it.
+// Returns unfiltered Cartesian graphical items for Cartesian charts only.
🤖 Prompt for AI Agents
In src/state/selectors/axisSelectors.ts around lines 318 to 321, the TODO
comment claiming this selector is called from a polar context is
outdated/misleading; remove the TODO (or replace it with a concrete note
referencing the actual issue/PR if one exists) so the codebase does not carry a
false warning. Update the comment to either be deleted or changed to a one-line
note that points to the specific issue/PR number or test that verifies this
selector is Cartesian-only.

axisType: AxisType | undefined,
range: AxisRange | undefined,
): number | undefined => {
): number => {
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

git ls-files | head -20

Repository: recharts/recharts

Length of output: 571


🏁 Script executed:

stat src/util/getActiveCoordinate.ts && wc -l src/util/getActiveCoordinate.ts

Repository: recharts/recharts

Length of output: 105


🏁 Script executed:

cat -n src/util/getActiveCoordinate.ts | head -220

Repository: recharts/recharts

Length of output: 7448


🏁 Script executed:

rg -nP -C3 '\bcalculateActiveTickIndex\s*\(' --type=ts --type=tsx

Repository: recharts/recharts

Length of output: 88


🏁 Script executed:

rg -nP -C3 'calculateActiveTickIndex' --type=ts --type=js

Repository: recharts/recharts

Length of output: 4164


🏁 Script executed:

rg -nP -C3 'calculateActiveTickIndex'

Repository: recharts/recharts

Length of output: 4164


🏁 Script executed:

cat -n src/util/getActiveCoordinate.ts | sed -n '150,170p'

Repository: recharts/recharts

Length of output: 883


🏁 Script executed:

cat -n src/state/selectors/selectors.ts | sed -n '210,260p'

Repository: recharts/recharts

Length of output: 1951


Fix TypeScript errors on lines 157 and 164 where unsortedTicks[i]?.index returns number | undefined but the function declares return type number.

The return type declaration at line 113 states the function returns number, but lines 157 and 164 return unsortedTicks[i]?.index using optional chaining, which produces number | undefined. Remove the optional chaining operator or provide a fallback value to ensure these paths return a number.

🤖 Prompt for AI Agents
In src/util/getActiveCoordinate.ts around line 113 (affects returns at lines 157
and 164), the function is declared to return number but currently returns
unsortedTicks[i]?.index which is number | undefined; change those return
expressions to produce a number (either remove the optional chaining to return
unsortedTicks[i].index if you guarantee index is defined, or provide a numeric
fallback such as using the nullish coalescing operator to return a default like
0 or another appropriate numeric sentinel) so the function always returns a
number and TypeScript errors are resolved.

Comment on lines +68 to +77
{
name: 'angle',
type: 'Number',
defaultVal: 0,
isOptional: true,
desc: {
'en-US':
'Text rotation angle in degrees. The text will be rotated around the (x, y) coordinates as the pivot point. Positive values rotate clockwise, negative values rotate counterclockwise.',
'zh-CN': 'label的旋转角度',
},
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Clarify the (x, y) pivot in LabelList angle documentation

The angle prop description says labels rotate around the (x, y) coordinates, but LabelListAPI doesn’t document any x/y props. For readers, it would help to clarify that this refers to each label’s position coordinates (the underlying Text/SVG text (x, y) for each data point), or otherwise spell out what those coordinates mean in this context.

🤖 Prompt for AI Agents
In www/src/docs/api/LabelList.ts around lines 68 to 77, the angle prop docs
mention rotation around the "(x, y) coordinates" but LabelListAPI doesn't
document what those coordinates are; update the desc to explicitly state that
the pivot is each label's own position — i.e., the (x, y) coordinates of the
rendered text for that data point (the label's text/SVG text anchor), so readers
understand the rotation is around each label's individual position rather than a
global origin.

Comment on lines +3 to +22
export const TextAPI: ApiDoc = {
name: 'Text',
props: [
{
name: 'angle',
type: 'number',
isOptional: true,
desc: {
'en-US': (
<section>
<p>
Text rotation angle in degrees. The text will be rotated around the (x, y) coordinates as the pivot point.
Positive values rotate clockwise, negative values rotate counterclockwise. The rotation transform is
applied as <code>rotate(angle, x, y)</code>.
</p>
</section>
),
},
defaultVal: 0,
},
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Clarify what the (x, y) pivot refers to in angle description

The angle description talks about rotation around the (x, y) coordinates, but x and y aren’t documented in this API surface (they’re inherited SVG <text> position attributes). Consider rephrasing to something like “rotated around the text position (x, y) — the SVG text position attributes” or otherwise explicitly tying (x, y) to the Text’s position so readers aren’t left guessing.

🤖 Prompt for AI Agents
In www/src/docs/api/TextAPI.tsx around lines 3 to 22, the angle prop description
refers to rotating around “the (x, y) coordinates” but doesn’t clarify what (x,
y) are; update the English description to explicitly state that (x, y) are the
Text element’s position (the SVG <text> position attributes) — e.g. “rotated
around the text position (x, y) — the SVG text position attributes” — and keep
the rest of the explanation about rotation direction and the rotate(angle, x, y)
transform intact.

@codecov
Copy link

codecov bot commented Dec 8, 2025

Codecov Report

❌ Patch coverage is 98.71383% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.89%. Comparing base (1565198) to head (b8ea20c).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
www/src/utils/LocaleUtils.ts 50.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6749      +/-   ##
==========================================
+ Coverage   93.88%   93.89%   +0.01%     
==========================================
  Files         512      512              
  Lines       42524    42715     +191     
  Branches     4991     4990       -1     
==========================================
+ Hits        39923    40109     +186     
- Misses       2596     2601       +5     
  Partials        5        5              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@codecov
Copy link

codecov bot commented Dec 8, 2025

Bundle Report

Changes will increase total bundle size by 198 bytes (0.01%) ⬆️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
recharts/bundle-cjs 1.16MB 99 bytes (0.01%) ⬆️
recharts/bundle-es6 1.0MB 99 bytes (0.01%) ⬆️

Affected Assets, Files, and Routes:

view changes for bundle: recharts/bundle-cjs

Assets Changed:

Asset Name Size Change Total Size Change (%)
state/selectors/axisSelectors.js 99 bytes 63.78kB 0.16%
view changes for bundle: recharts/bundle-es6

Assets Changed:

Asset Name Size Change Total Size Change (%)
state/selectors/axisSelectors.js 99 bytes 53.93kB 0.18%

@github-actions
Copy link
Contributor

github-actions bot commented Dec 8, 2025

Staging Deployment Details

These deployments will remain available for 30 days.

To update snapshots: Comment /update-snapshots on this PR to automatically update the baseline screenshots.

@PavelVanecek PavelVanecek merged commit cf325bd into main Dec 8, 2025
39 of 40 checks passed
@PavelVanecek PavelVanecek deleted the omnidoc branch December 8, 2025 11:43
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.

2 participants