Skip to content

[TypeScript] add BarShapeProps type to fix Bar.shape type#6900

Merged
ckifer merged 2 commits intomainfrom
barshape-type
Jan 18, 2026
Merged

[TypeScript] add BarShapeProps type to fix Bar.shape type#6900
ckifer merged 2 commits intomainfrom
barshape-type

Conversation

@PavelVanecek
Copy link
Collaborator

@PavelVanecek PavelVanecek commented Jan 17, 2026

Description

New type to accurately describe what Bar.shape is doing.

Related Issue

Motivation and Context

Nobody has complained yet but I found it embarrassing that our own examples have to use @ts-expect-error to compile.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • I have added a storybook story or VR test, or extended an existing story or VR test to show my changes

Summary by CodeRabbit

  • New Features

    • Introduced BarShapeProps for richer, more specific bar-shape customization in Bar components.
  • Refactor

    • Unified and simplified shape callback typings to improve consistency and reliability across the Bar API.
    • Updated examples, docs, and demos to use the new shape type so custom shapes behave consistently.

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 17, 2026

Walkthrough

Introduces a new public type BarShapeProps and replaces BarProps with BarShapeProps for shape-related callbacks (shape, activeBar, background) across the Bar component, utilities, tests, and example components. Also narrows ActiveShape function signature and re-exports BarShapeProps from src/index.ts.

Changes

Cohort / File(s) Summary
Core Bar implementation & types
src/cartesian/Bar.tsx, src/util/types.ts, src/util/BarUtils.tsx
Adds public BarShapeProps; updates BarProps's shape, activeBar, and background typings to use ActiveShape<BarShapeProps, SVGPathElement>; narrows ActiveShape function signature to return `ReactElement
Public exports & verification
src/index.ts, omnidoc/verifyExamples.spec.ts
Re-exports BarShapeProps from src/index.ts; updates example verification to include the relevant export(s).
Tests
test/chart/BarChart.spec.tsx
Replaces BarProps with BarShapeProps in test callbacks and imports; adjusts local stacked-bar typings and TS expectations accordingly.
Example & site components
www/src/components/.../CustomizeBarShape.tsx, www/src/docs/exampleComponents/BarChart/CandlestickExample.tsx, .../CustomShapeBarChart.tsx, .../TimelineExample.tsx
Update example/custom shape component signatures and imports from BarProps to BarShapeProps; minor cleanup (remove ts-expect-error, adjust returns).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested labels

typescript

Suggested reviewers

  • ckifer
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: introducing a new BarShapeProps type to fix Bar.shape type issues.
Description check ✅ Passed The description covers key sections including motivation (removing @ts-expect-error in examples) and type of change, though some template sections are incomplete.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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

🤖 Fix all issues with AI agents
In `@src/cartesian/Bar.tsx`:
- Around line 115-116: BarShapeProps currently uses `option?: any`; replace this
with the specific option type used by BarRectangleProps to avoid `any` — e.g.
change `option?: any` to `option?: BarRectangleProps['option']` (referencing the
existing BarRectangleProps type used in BarUtils) so the prop uses the
established precise type instead of `any`.

In `@src/util/BarUtils.tsx`:
- Around line 5-10: Change the option field on the BarShapeProps type in
src/cartesian/Bar.tsx from option?: any to option?: unknown and then update any
consumer code to explicitly narrow or validate that option before using it
(e.g., add type guards or runtime checks where BarShapeProps or
ActiveShape<BarShapeProps, ...> is consumed). Specifically modify the
BarShapeProps type declaration (the option property), and then search for usages
such as in ActiveShape<BarShapeProps, SVGPathElement>, BarRectangleProps, and
any functions that access option to add proper type refinement or safe casts so
no code assumes properties on option without checking. Ensure exports and
imports remain consistent so BarUtils and other modules compile with the new
unknown type.
🧹 Nitpick comments (1)
www/src/docs/exampleComponents/BarChart/CandlestickExample.tsx (1)

5-5: Prefer a typed intersection to remove @ts-expect-error.

Since the shape props include the data payload, model it explicitly as BarShapeProps & MarketCandle and drop the ts-expect-error.

♻️ Suggested change
-const Candlestick = (props: BarShapeProps) => {
-  // `@ts-expect-error` Recharts does spread MarketCandle on the props but the types don't reflect that
-  const color = props.open < props.close ? 'green' : 'red';
+const Candlestick = (props: BarShapeProps & MarketCandle) => {
+  const color = props.open < props.close ? 'green' : 'red';

Also applies to: 37-40

@codecov
Copy link

codecov bot commented Jan 17, 2026

Codecov Report

❌ Patch coverage is 90.90909% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 94.27%. Comparing base (a2c1e23) to head (45e5425).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
...mponents/GuideView/Customize/CustomizeBarShape.tsx 66.66% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6900   +/-   ##
=======================================
  Coverage   94.27%   94.27%           
=======================================
  Files         565      565           
  Lines       53858    53859    +1     
  Branches     5178     5178           
=======================================
+ Hits        50777    50778    +1     
  Misses       3072     3072           
  Partials        9        9           

☔ 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.

@github-actions
Copy link
Contributor

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.

@codecov
Copy link

codecov bot commented Jan 18, 2026

Bundle Report

Bundle size has no change ✅

@ckifer ckifer merged commit a78f991 into main Jan 18, 2026
47 of 48 checks passed
@PavelVanecek PavelVanecek deleted the barshape-type branch January 18, 2026 23:00
@coderabbitai coderabbitai bot mentioned this pull request Jan 28, 2026
@coderabbitai coderabbitai bot mentioned this pull request Feb 16, 2026
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