Skip to content

Omnidoc: Area, AreaChart, Bar prop comments#6715

Merged
ckifer merged 4 commits intomainfrom
prop-comments
Dec 3, 2025
Merged

Omnidoc: Area, AreaChart, Bar prop comments#6715
ckifer merged 4 commits intomainfrom
prop-comments

Conversation

@PavelVanecek
Copy link
Collaborator

@PavelVanecek PavelVanecek commented Dec 2, 2025

Description

  • Adds a new test: same props on different components should have the same comment
  • Fixes comments in AreaChart and Bar (and updates some in Area)

Related Issue

#6069

Summary by CodeRabbit

  • New Features

    • Bar component gains many new configuration options and richer event handlers for customization, interactivity, and animation.
    • Charts expose extended mouse/touch event hooks.
  • Documentation

    • Expanded and clarified API docs and prop descriptions across chart components (Area, Bar, AreaChart, etc.), including usage, stacking, axes, labeling, and ID behavior.
  • Tests

    • Added cross-component prop/comment consistency tests with fuzzy similarity/diff checks and temporary exemptions.
  • Chores

    • Updated project dependencies.

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 2, 2025

Walkthrough

This PR adds fuzzy text-normalization and cross-component prop-comment similarity tests, records a normalizedPath on PropMeta, broadens prop comparisons in omnidoc tests, expands Bar component props/events and JSDoc comments across chart types, updates several API docs, and adds the diff package dependency.

Changes

Cohort / File(s) Change Summary
Omnidoc tests & metadata
omnidoc/omnidoc.spec.ts, omnidoc/readProject.ts, omnidoc/readProject.spec.ts
Added text-normalization and similarity utilities (normalizeText, calculateSimilarityScore, coloredDiff), imported diff.diffChars, introduced cross-component prop-consistency tests with a similarity threshold and exception lists, replaced getRechartsPropsOf usages with getAllPropsOf, and added normalizedPath to PropMeta (and updated test expectations).
Component API surface
src/cartesian/Bar.tsx, src/chart/types.ts
Expanded BarProps with many configuration options (axis IDs, stackId, sizing, unit/name, id/label, shape/background/activeBar, animation controls) and added multiple mouse event handlers to BarEvents; added/expanded JSDoc for ExternalMouseEvents.
Code JSDoc (internal components)
src/cartesian/Area.tsx, src/util/types.ts
Rewrote and clarified JSDoc comments for Area props and CartesianChartProps; documentation-only edits (no signature changes).
Website API docs
www/src/docs/api/Area.ts, www/src/docs/api/AreaChart.ts, www/src/docs/api/Bar.ts
Revised en-US/zh-CN property descriptions for Area, AreaChart, and Bar (clarified dataKey extraction modes, axis/stack behavior, rendering options, SSR notes). Changed AreaChart width doc type from Number to `Percent
Dependencies & config
package.json
Added the diff package (appears in dependencies/devDependencies in this diff) and adjusted eslint-plugin-react-perf entries in devDependencies.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

  • Review omnidoc fuzzy-matching logic, similarity threshold, and exception lists in omnidoc/omnidoc.spec.ts.
  • Verify normalizedPath computation and propagation in omnidoc/readProject.ts and updated tests.
  • Inspect newly added Bar props/events in src/cartesian/Bar.tsx for correct types, duplicates, and JSDoc accuracy.
  • Check website API doc changes (www/src/docs/api/*) for consistency with component behavior where applicable.

Possibly related PRs

Suggested reviewers

  • ckifer

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning The description addresses the key changes and links to the related issue, but lacks detail on motivation, testing approach, and change type classification as required by the template. Complete missing template sections: add 'Motivation and Context' explaining why consistent prop comments are important, describe testing approach in 'How Has This Been Tested', and check appropriate change type boxes.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Omnidoc: Area, AreaChart, Bar prop comments' clearly indicates the PR updates prop documentation/comments across three specific components, which aligns with the main changes.
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 prop-comments

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: 3

Caution

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

⚠️ Outside diff range comments (1)
www/src/docs/api/Bar.ts (1)

226-236: Type definition is incomplete for isAnimationActive.

The type is listed as 'Boolean' but the defaultVal is 'auto' and the description mentions the "auto" option. This should match the actual type in Bar.tsx which is boolean | 'auto'.

     {
       name: 'isAnimationActive',
-      type: 'Boolean',
+      type: "Boolean | 'auto'",
       defaultVal: 'auto',
       isOptional: true,
🧹 Nitpick comments (3)
omnidoc/readProject.ts (1)

199-207: Consider extracting duplicated path normalization logic.

The path normalization (filePath.replace(/\\/g, '/')) is duplicated between getDeclarationOrigin (line 112) and here (line 203). Consider extracting this into a small helper function to keep it DRY.

+  private normalizePath(filePath: string): string {
+    return filePath.replace(/\\/g, '/');
+  }
+
   private getDeclarationOrigin(declaration: Node): PropOrigin {
     const sourceFile = declaration.getSourceFile();
     const filePath = sourceFile.getFilePath();
-    const normalizedPath = filePath.replace(/\\/g, '/');
+    const normalizedPath = this.normalizePath(filePath);

And use it in collectPropertiesFromType as well:

         const sourceFile = declaration.getSourceFile();
         const filePath = sourceFile.getFilePath();
-        const normalizedPath = filePath.replace(/\\/g, '/');
+        const normalizedPath = this.normalizePath(filePath);
src/cartesian/Bar.tsx (2)

227-231: Minor inconsistency: @default vs @defaultValue tag.

This JSDoc uses @default 0 while other props in this file use @defaultValue. Consider using @defaultValue 0 for consistency.

   /**
    * Specifies when the animation should begin, the unit of this option is ms.
-   * @default 0
+   * @defaultValue 0
    */
   animationBegin?: number;

238-243: Same inconsistency: @default vs @defaultValue tag.

   /**
    * The type of easing function.
    *
-   * @default 'ease'
+   * @defaultValue 'ease'
    */
   animationEasing?: EasingInput;
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 837d76f and 59fb292.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (11)
  • omnidoc/omnidoc.spec.ts (6 hunks)
  • omnidoc/readProject.spec.ts (2 hunks)
  • omnidoc/readProject.ts (3 hunks)
  • package.json (2 hunks)
  • src/cartesian/Area.tsx (5 hunks)
  • src/cartesian/Bar.tsx (2 hunks)
  • src/chart/types.ts (1 hunks)
  • src/util/types.ts (2 hunks)
  • www/src/docs/api/Area.ts (7 hunks)
  • www/src/docs/api/AreaChart.ts (5 hunks)
  • www/src/docs/api/Bar.ts (13 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (DEVELOPING.md)

Run type checking on the codebase using npm run check-types

**/*.{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:

  • omnidoc/omnidoc.spec.ts
  • omnidoc/readProject.spec.ts
  • src/chart/types.ts
  • src/util/types.ts
  • www/src/docs/api/Area.ts
  • omnidoc/readProject.ts
  • src/cartesian/Area.tsx
  • www/src/docs/api/Bar.ts
  • www/src/docs/api/AreaChart.ts
  • src/cartesian/Bar.tsx
**/*.{js,ts,tsx}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

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

Files:

  • omnidoc/omnidoc.spec.ts
  • omnidoc/readProject.spec.ts
  • src/chart/types.ts
  • src/util/types.ts
  • www/src/docs/api/Area.ts
  • omnidoc/readProject.ts
  • src/cartesian/Area.tsx
  • www/src/docs/api/Bar.ts
  • www/src/docs/api/AreaChart.ts
  • src/cartesian/Bar.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/readProject.spec.ts
src/**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (DEVELOPING.md)

Run ESLint and Prettier on the codebase using npm run lint

Files:

  • src/chart/types.ts
  • src/util/types.ts
  • src/cartesian/Area.tsx
  • src/cartesian/Bar.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/chart/types.ts
  • src/util/types.ts
  • src/cartesian/Area.tsx
  • src/cartesian/Bar.tsx
🧠 Learnings (9)
📚 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
  • omnidoc/readProject.spec.ts
  • package.json
📚 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 : Use Storybook for smoke tests and add play functions with assertions for actual tests

Applied to files:

  • omnidoc/omnidoc.spec.ts
📚 Learning: 2025-11-25T01:22:48.289Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: DEVELOPING.md:0-0
Timestamp: 2025-11-25T01:22:48.289Z
Learning: Applies to storybook/**/*.stories.{ts,tsx} : Use Storybook stories in the `storybook` directory for high-fidelity component examples that will be published and used for visual regression testing

Applied to files:

  • omnidoc/omnidoc.spec.ts
  • package.json
📚 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:

  • omnidoc/omnidoc.spec.ts
  • omnidoc/readProject.spec.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 test/component/**/*.spec.tsx : Use React Testing Library for testing component interactions and behavior upon rendering

Applied to files:

  • omnidoc/omnidoc.spec.ts
📚 Learning: 2025-11-25T01:22:48.289Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: DEVELOPING.md:0-0
Timestamp: 2025-11-25T01:22:48.289Z
Learning: Applies to {test,www/test}/**/*.spec.{ts,tsx} : Write unit tests in the `test` or `www/test` directories with `.spec.tsx` file extension

Applied to files:

  • omnidoc/readProject.spec.ts
📚 Learning: 2025-11-16T09:14:24.918Z
Learnt from: PavelVanecek
Repo: recharts/recharts PR: 6640
File: src/cartesian/Bar.tsx:156-159
Timestamp: 2025-11-16T09:14:24.918Z
Learning: In recharts, SSR (Server-Side Rendering) is not yet supported—charts don't render at all in SSR environments. The `isAnimationActive: 'auto'` mode is preparatory work for future SSR support, so testing of this mode should be deferred until SSR support is actually implemented.

Applied to files:

  • www/src/docs/api/Bar.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 **/*.{js,ts,tsx} : Ensure code lints by running `npm run lint` and follows Airbnb's Style Guide

Applied to files:

  • package.json
📚 Learning: 2025-11-25T01:22:48.289Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: DEVELOPING.md:0-0
Timestamp: 2025-11-25T01:22:48.289Z
Learning: Applies to src/**/*.{js,jsx,ts,tsx} : Run ESLint and Prettier on the codebase using `npm run lint`

Applied to files:

  • package.json
🧬 Code graph analysis (3)
src/util/types.ts (2)
src/cartesian/Area.tsx (1)
  • BaseValue (74-74)
src/synchronisation/types.ts (1)
  • SyncMethod (37-37)
src/cartesian/Area.tsx (1)
src/component/LabelList.tsx (1)
  • ImplicitLabelListType (85-85)
src/cartesian/Bar.tsx (5)
src/util/ChartUtils.ts (1)
  • StackId (443-443)
src/util/types.ts (4)
  • DataKey (60-60)
  • TooltipType (81-81)
  • LegendType (69-80)
  • ActiveShape (1058-1063)
src/index.ts (1)
  • LegendType (117-117)
src/util/BarUtils.tsx (1)
  • MinPointSize (62-62)
src/zIndex/ZIndexLayer.tsx (1)
  • ZIndexable (14-23)
⏰ 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/readProject.ts (1)

16-22: LGTM!

The new normalizedPath field is well-documented and properly integrated into the PropMeta type, enabling path-based origin tracking for props.

package.json (1)

145-145: LGTM!

The diff package is correctly added as a devDependency since it's only used in the omnidoc test infrastructure.

src/cartesian/Area.tsx (1)

159-165: LGTM!

The updated JSDoc descriptions for dataKey and id props are clear and provide helpful context for users.

Also applies to: 172-178

omnidoc/readProject.spec.ts (1)

823-854: LGTM!

The test expectations are properly updated to include normalizedPath with appropriate expect.stringContaining() matchers. This provides flexible path matching that works across different environments while still validating the essential path segments.

src/util/types.ts (1)

1091-1179: LGTM!

The JSDoc documentation updates for CartesianChartProps are comprehensive and consistent with the corresponding API documentation in www/src/docs/api/AreaChart.ts. The descriptions properly explain the purpose and behavior of each prop.

www/src/docs/api/AreaChart.ts (2)

53-61: LGTM!

The width type update to Percent | Number correctly reflects the TypeScript type definition in CartesianChartProps, and the descriptions are now consistent across the codebase.


36-51: LGTM!

The expanded syncMethod description provides valuable detail about the three synchronization modes (index, value, function) and is consistent with the JSDoc in CartesianChartProps.

omnidoc/omnidoc.spec.ts (4)

29-75: LGTM!

The Levenshtein distance implementation is correct and well-documented. The similarity score calculation properly handles edge cases (undefined inputs, empty strings) and the algorithm is appropriate for comparing short-to-medium length documentation strings.


85-113: LGTM!

The coloredDiff helper function properly handles all edge cases and produces readable diff output with clear markers for additions (+++) and deletions (---).


609-726: LGTM!

The cross-component prop consistency test is well-designed:

  • Clear documentation of the similarity threshold and its purpose
  • Proper exception handling with documented reasons
  • Pairwise comparison catches all inconsistencies
  • Helpful console output for debugging when inconsistencies are found

418-422: Verify this change to getAllPropsOf is intentional.

This changes from getRechartsPropsOf to getAllPropsOf, which now includes SVG inherited props in the comparison. This aligns with the pattern used elsewhere (lines 174, 198), but ensure this broader comparison is intended for API doc comment validation.

src/chart/types.ts (1)

8-52: LGTM with the exception of the copy-paste error noted above.

The JSDoc comments for the remaining event handlers are consistent and correctly describe their respective events.

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

33-39: LGTM for the remaining documentation updates.

The expanded descriptions for dataKey, xAxisId, yAxisId, legendType, stackId, and id provide clearer guidance for users.

Also applies to: 48-48, 58-58, 68-68, 167-167, 270-272

src/cartesian/Bar.tsx (1)

112-266: LGTM for the comprehensive JSDoc additions.

The documentation for BarProps and BarEvents is thorough and provides clear guidance on prop usage, default values, and behavior. The descriptions for complex props like activeBar, background, shape, and minPointSize are particularly well-documented with their various configuration options.

Also applies to: 274-307

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

1-375: LGTM for the remaining documentation updates.

The expanded descriptions provide clear, consistent documentation that aligns well with the JSDoc comments in Bar.tsx. The multi-option rendering explanations for label, background, activeBar, and shape are particularly helpful for users.

@codecov
Copy link

codecov bot commented Dec 2, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.03%. Comparing base (3599a10) to head (0eaa11a).
⚠️ Report is 10 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6715   +/-   ##
=======================================
  Coverage   94.03%   94.03%           
=======================================
  Files         500      500           
  Lines       42662    42666    +4     
  Branches     4901     4901           
=======================================
+ Hits        40116    40120    +4     
  Misses       2541     2541           
  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 2, 2025

Bundle Report

Bundle size has no change ✅

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: 1

🧹 Nitpick comments (2)
omnidoc/omnidoc.spec.ts (2)

10-113: LGTM! Well-structured fuzzy matching utilities.

The text normalization, Levenshtein distance calculation, and diff generation functions are correctly implemented and properly typed. These utilities provide a solid foundation for cross-component documentation consistency checking.

Optional refactor for the nested ternary (lines 107-110):

Consider refactoring for improved readability:

-  for (const part of diff) {
-    // eslint-disable-next-line no-nested-ternary
-    const colorStart = part.added ? '+++' : part.removed ? '---' : '';
-    const colorEnd = part.added || part.removed ? '<<<' : '';
-    result += colorStart + part.value + colorEnd;
-  }
+  for (const part of diff) {
+    let colorStart = '';
+    let colorEnd = '';
+    if (part.added) {
+      colorStart = '+++';
+      colorEnd = '<<<';
+    } else if (part.removed) {
+      colorStart = '---';
+      colorEnd = '<<<';
+    }
+    result += colorStart + part.value + colorEnd;
+  }

609-726: Excellent cross-component consistency test with fuzzy matching.

The new test suite intelligently identifies props shared across components and validates their JSDoc comments are similar. The implementation includes:

  • Adjustable similarity threshold with clear documentation
  • Exception mechanism for legitimately different comments
  • Comprehensive pairwise comparison with detailed failure messages
  • Proper filtering to focus on meaningful cases (multiple components with comments)

All helper functions (calculateSimilarityScore, coloredDiff) are properly defined with explicit TypeScript type signatures accepting string | undefined parameters. The code follows TypeScript best practices: no any types, explicit type annotations throughout, and safe use of non-null assertions where prior checks justify them. The test structure and logic are sound.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 087acdc and 0eaa11a.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (11)
  • omnidoc/omnidoc.spec.ts (6 hunks)
  • omnidoc/readProject.spec.ts (2 hunks)
  • omnidoc/readProject.ts (3 hunks)
  • package.json (2 hunks)
  • src/cartesian/Area.tsx (5 hunks)
  • src/cartesian/Bar.tsx (2 hunks)
  • src/chart/types.ts (1 hunks)
  • src/util/types.ts (2 hunks)
  • www/src/docs/api/Area.ts (7 hunks)
  • www/src/docs/api/AreaChart.ts (5 hunks)
  • www/src/docs/api/Bar.ts (13 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
  • omnidoc/readProject.spec.ts
  • src/chart/types.ts
  • package.json
  • www/src/docs/api/Bar.ts
  • omnidoc/readProject.ts
  • www/src/docs/api/AreaChart.ts
  • src/cartesian/Bar.tsx
  • www/src/docs/api/Area.ts
🧰 Additional context used
📓 Path-based instructions (5)
src/**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (DEVELOPING.md)

Run ESLint and Prettier on the codebase using npm run lint

Files:

  • src/util/types.ts
  • src/cartesian/Area.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (DEVELOPING.md)

Run type checking on the codebase using npm run check-types

**/*.{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/types.ts
  • omnidoc/omnidoc.spec.ts
  • src/cartesian/Area.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/types.ts
  • omnidoc/omnidoc.spec.ts
  • src/cartesian/Area.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/types.ts
  • src/cartesian/Area.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
🧠 Learnings (5)
📚 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
📚 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 : Use Storybook for smoke tests and add play functions with assertions for actual tests

Applied to files:

  • omnidoc/omnidoc.spec.ts
📚 Learning: 2025-11-25T01:22:48.289Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: DEVELOPING.md:0-0
Timestamp: 2025-11-25T01:22:48.289Z
Learning: Applies to storybook/**/*.stories.{ts,tsx} : Use Storybook stories in the `storybook` directory for high-fidelity component examples that will be published and used for visual regression testing

Applied to files:

  • omnidoc/omnidoc.spec.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:

  • omnidoc/omnidoc.spec.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 test/component/**/*.spec.tsx : Use React Testing Library for testing component interactions and behavior upon rendering

Applied to files:

  • omnidoc/omnidoc.spec.ts
🧬 Code graph analysis (2)
src/util/types.ts (2)
src/cartesian/Area.tsx (1)
  • BaseValue (74-74)
src/synchronisation/types.ts (1)
  • SyncMethod (37-37)
src/cartesian/Area.tsx (1)
src/component/LabelList.tsx (1)
  • ImplicitLabelListType (85-85)
🔇 Additional comments (9)
src/cartesian/Area.tsx (6)

160-166: Clarified dataKey documentation matches DataKey usage.

The updated bullets accurately describe the supported DataKey forms (string, number, function) and align with how getValueByDataKey is used here and elsewhere.


173-178: id prop description now matches internal usage.

The comment clearly explains that id is an optional external prop, used both as the SVG id and as an internal identifier, with auto-generation when omitted.


187-196: Label description now correctly reflects Area semantics.

Changing “bar” to “data point” and enumerating the supported forms (boolean, object props, element, function) matches ImplicitLabelListType and LabelListFromLabelProp behavior.


198-201: Legend icon behavior is documented consistently.

The legendType JSDoc (“type of icon… 'none' hides the legend item”) aligns with how legendType is passed into computeLegendPayloadFromAreaData and typical Legend handling.


249-251: stackId stacking explanation is accurate.

The note about Areas with the same axis ids and stackId being stacked matches how stackId is normalized and fed into SetCartesianGraphicalItem.


273-281: Axis id docs are clearer and include defaults.

The updated xAxisId/yAxisId comments explain correspondence to data, multi-axis requirements, and default 0, matching how these props are consumed in selectors and SetCartesianGraphicalItem.

src/util/types.ts (1)

1105-1178: Excellent documentation improvements!

The JSDoc comments added for CartesianChartProps properties are clear, concise, and will significantly improve the developer experience. The comments provide appropriate context for each prop, and the inclusion of @defaultValue annotations is particularly helpful. These documentation additions align well with the PR's goal of standardizing prop comments across components.

omnidoc/omnidoc.spec.ts (2)

345-394: Temporary exclusion list is a pragmatic approach.

The temporary exclusion list for components with known inconsistencies provides a clear path for incremental documentation fixes while preventing test failures. The comment clearly explains this will shrink over time.


396-442: LGTM! Enhanced comparison with diff output.

The test correctly broadens prop comparison to include all props (line 421) and provides clearer failure messages using coloredDiff (lines 434-438). The change from exact equality to diff-based comparison improves developer experience when inconsistencies are found.

Comment on lines +1159 to 1171
/**
* Charts with the same syncId will synchronise Tooltip and Brush events.
*/
syncId?: number | string;
/**
* Customize how the charts will synchronize tooltips and brushes.
* `index`: synchronize using the data index in the data array. Index expects that all data has the same length.
* `value`: synchronize using the data value on categorical axis (categorical: XAxis in horizontal layout, YAxis in vertical layout).
* function: a custom sync method which receives tick and data as argument and returns an index.
*
* @defaultValue index
*/
syncMethod?: SyncMethod;
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

Fix spelling inconsistency in JSDoc comments.

Line 1160 uses British spelling "synchronise" while Line 1164 uses American spelling "synchronize". For consistency, use the same spelling variant throughout the codebase.

Apply this diff to use consistent American spelling:

   /**
-   * Charts with the same syncId will synchronise Tooltip and Brush events.
+   * Charts with the same syncId will synchronize Tooltip and Brush events.
    */
📝 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
/**
* Charts with the same syncId will synchronise Tooltip and Brush events.
*/
syncId?: number | string;
/**
* Customize how the charts will synchronize tooltips and brushes.
* `index`: synchronize using the data index in the data array. Index expects that all data has the same length.
* `value`: synchronize using the data value on categorical axis (categorical: XAxis in horizontal layout, YAxis in vertical layout).
* function: a custom sync method which receives tick and data as argument and returns an index.
*
* @defaultValue index
*/
syncMethod?: SyncMethod;
/**
* Charts with the same syncId will synchronize Tooltip and Brush events.
*/
syncId?: number | string;
/**
* Customize how the charts will synchronize tooltips and brushes.
* `index`: synchronize using the data index in the data array. Index expects that all data has the same length.
* `value`: synchronize using the data value on categorical axis (categorical: XAxis in horizontal layout, YAxis in vertical layout).
* function: a custom sync method which receives tick and data as argument and returns an index.
*
* @defaultValue index
*/
syncMethod?: SyncMethod;
🤖 Prompt for AI Agents
In src/util/types.ts around lines 1159 to 1171, the JSDoc uses British
"synchronise" on line 1160 but American "synchronize" elsewhere; update the
comment to use the American spelling consistently by replacing "synchronise"
with "synchronize" (and verify any other occurrences in this comment block are
updated accordingly).

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