Conversation
WalkthroughAdds a new BarStack feature (component, context, hooks, clip-path rendering) to enable rounded stacked bars, refactors bar layout selectors into combiners, updates Bar to consume BarStack context, adds omnidoc tooling and generated API docs, website examples/guides, and accompanying tests. Changes
Sequence Diagram(s)sequenceDiagram
participant App as App / BarChart
participant BarStack as BarStack
participant State as Redux State / Selectors
participant Bar as Bar
participant Clip as BarStackClipPath/Layer
App->>BarStack: render BarStack(stackId?, radius)
BarStack->>State: selectStackRects(state, resolvedStackId, isPanorama)
State-->>BarStack: stacked rectangles (positions)
BarStack->>Clip: render clipPath(s) per stacked rect
App->>Bar: render Bar children
Bar->>BarStack: useStackId(props.stackId?)
BarStack-->>Bar: resolved stackId (context or props)
Bar->>Clip: wrap bar rectangles with BarStackClipLayer (uses clipPath URL)
Clip-->>App: bars rendered with stack-aware rounded corners
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (10)
src/util/propsAreEqual.ts (1)
23-24: Clarify comment to reflect that radius can be a number or an array.The comment states that radius can be an array of 4 numbers, but according to the type information (RectRadius), radius can also be a plain number. Consider updating the comment to reflect both possibilities for completeness.
- // radius can be an array of 4 numbers, easy to compare shallowly + // radius can be a number or an array of 4 numbers, easy to compare shallowly 'radius',src/cartesian/Bar.tsx (1)
346-346: Consider usingRectRadiustype for consistency.
InternalBarProps.radiusstill uses the inline typenumber | [number, number, number, number]whileBarProps.radius(line 211) now uses theRectRadiustype alias. Consider updating this toradius?: RectRadiusfor consistency and to avoid type duplication.- radius?: number | [number, number, number, number]; + radius?: RectRadius;test/cartesian/BarStack.spec.tsx (2)
1-8: Missingvi.useFakeTimers()setup.Per coding guidelines, tests should use
vi.useFakeTimers()due to Redux autoBatchEnhancer dependency on timers andrequestAnimationFrame. Consider adding timer setup:+import { beforeEach, afterEach, describe, it, expect, vi } from 'vitest'; -import { describe, it, expect } from 'vitest'; import { createSelectorTestCase } from '../helper/createSelectorTestCase'; import { Bar, BarChart, BarStack } from '../../src'; import { PageData } from '../_data'; import { useBarStackClipPathUrl, useStackId } from '../../src/cartesian/BarStack'; import { expectLastCalledWith } from '../helper/expectLastCalledWith'; +beforeEach(() => { + vi.useFakeTimers(); +}); + +afterEach(() => { + vi.useRealTimers(); +});However, if
createSelectorTestCasealready handles timer setup internally, this may not be needed. Please verify.
21-34: Consider verifying selector call counts.Per coding guidelines, tests should verify the number of selector calls using the spy object to spot unnecessary re-renders. Consider adding assertions like
expect(spy).toHaveBeenCalledTimes(1)to ensure the hooks aren't called more than expected.test/state/selectors/barStackSelectors.spec.tsx (1)
28-46: Consider verifying selector call counts.Per coding guidelines, selector tests should verify the number of selector calls to spot unnecessary re-renders. Consider adding
expect(spy).toHaveBeenCalledTimes(1)assertions.test('selectAllBarsInStack', () => { const { spy } = renderTestCase(state => selectAllBarsInStack(state, 'mystackid', false)); + expect(spy).toHaveBeenCalledTimes(1); expectLastCalledWith(spy, [omnidoc/generateApiDoc.ts (1)
127-136: Avoidastype assertion.Per coding guidelines,
astype assertions should not be used except foras const. Consider using a type guard or helper function 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; + // defaultValue.value is typed as unknown from the reader + // ApiProps.defaultVal accepts these types, so we assign directly + prop.defaultVal = defaultValue.value; }If TypeScript still complains, consider updating the
DefaultValuetype inreadProject.tsto be more specific, or create a type guard function.src/state/selectors/combiners/combineAllBarPositions.ts (2)
42-42: Consider usingMath.floorfor clarity.The bitwise
>> 0trick floors the number but reduces readability.Math.floor()would be clearer and has negligible performance difference for this use case.- const offset = ((bandSize - sum) / 2) >> 0; + const offset = Math.floor((bandSize - sum) / 2);
70-73: Same readability suggestion for floor operation.Consider using
Math.floorinstead of bitwise shift for consistency and clarity.let originalSize = (bandSize - 2 * offset - (len - 1) * realBarGap) / len; if (originalSize > 1) { - originalSize >>= 0; + originalSize = Math.floor(originalSize); }src/state/selectors/combiners/combineBarSizeList.ts (1)
52-67: Consider placing theBarCategorytype definition before its usage.The
BarCategorytype is used on lines 38 and 44 but defined at line 52. While TypeScript hoists type definitions, placing the type before thecombineBarSizeListfunction improves readability.Regarding
DataKey<any>on line 62: this appears to follow the existing pattern in the codebase whereDataKeyis a generic type. Per coding guidelines, verify this is the accepted convention for this type.+export type BarCategory = { + stackId: StackId | undefined; + /** + * List of dataKeys of items stacked at this position. + * All of these Bars are either sharing the same stackId, + * or this is an array with one Bar because it has no stackId defined. + * + * This structure limits us to having one dataKey only once per stack which I think is reasonable. + * People who want to have the same data twice can duplicate their data to have two distinct dataKeys. + */ + dataKeys: ReadonlyArray<DataKey<any>>; + /** + * Width (in horizontal chart) or height (in vertical chart) of this stack of items + */ + barSize: number | undefined; +}; + export const combineBarSizeList = (omnidoc/readProject.ts (1)
442-454: Consider makinggetTextOfTagprivate.This method appears to be a helper utility used internally. Adding the
privatemodifier would be consistent with similar helper methods in this class.- getTextOfTag(tag: JSDocTagInfo | undefined): string | undefined { + private getTextOfTag(tag: JSDocTagInfo | undefined): string | undefined {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
package-lock.jsonis excluded by!**/package-lock.jsontest-vr/__snapshots__/tests/www/BarChartApiExamples.spec-vr.tsx-snapshots/RangedStackedBarChart-1-chromium-linux.pngis excluded by!**/*.pngtest-vr/__snapshots__/tests/www/BarChartApiExamples.spec-vr.tsx-snapshots/RangedStackedBarChart-1-firefox-linux.pngis excluded by!**/*.pngtest-vr/__snapshots__/tests/www/BarChartApiExamples.spec-vr.tsx-snapshots/RangedStackedBarChart-1-webkit-linux.pngis excluded by!**/*.pngwww/test/__snapshots__/navigation.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (38)
omnidoc/README.md(1 hunks)omnidoc/componentsAndDefaultPropsMap.ts(2 hunks)omnidoc/cross-component-prop-comments.spec.ts(1 hunks)omnidoc/generateApiDoc.ts(1 hunks)omnidoc/readApiDoc.spec.ts(2 hunks)omnidoc/readProject.spec.ts(3 hunks)omnidoc/readProject.ts(6 hunks)package.json(1 hunks)scripts/snapshots/es6Files.txt(2 hunks)scripts/snapshots/libFiles.txt(2 hunks)scripts/snapshots/typesFiles.txt(2 hunks)src/cartesian/Bar.tsx(7 hunks)src/cartesian/BarStack.tsx(1 hunks)src/index.ts(1 hunks)src/shape/Rectangle.tsx(2 hunks)src/state/selectors/barSelectors.ts(2 hunks)src/state/selectors/barStackSelectors.ts(1 hunks)src/state/selectors/combiners/combineAllBarPositions.ts(1 hunks)src/state/selectors/combiners/combineBarSizeList.ts(1 hunks)src/state/selectors/combiners/combineStackedData.ts(1 hunks)src/state/selectors/radialBarSelectors.ts(2 hunks)src/util/Global.ts(1 hunks)src/util/propsAreEqual.ts(1 hunks)test/cartesian/BarStack.spec.tsx(1 hunks)test/shape/Rectangle.spec.tsx(1 hunks)test/state/selectors/barStackSelectors.spec.tsx(1 hunks)www/src/components/GuideView/RoundedBars/index.tsx(1 hunks)www/src/docs/api/BarStack.ts(1 hunks)www/src/docs/api/index.ts(2 hunks)www/src/docs/apiCates.ts(1 hunks)www/src/docs/apiExamples/BarStack/index.tsx(1 hunks)www/src/docs/apiExamples/index.tsx(1 hunks)www/src/docs/exampleComponents/BarChart/RangedStackedBarChart.tsx(2 hunks)www/src/docs/exampleComponents/BarChart/index.tsx(1 hunks)www/src/locale/en-US.ts(1 hunks)www/src/locale/zh-CN.ts(1 hunks)www/src/navigation.data.ts(1 hunks)www/src/views/GuideView.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{ts,tsx}: Never useanytype (implicit or explicit) in TypeScript code
Preferunknownoveranyand 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 useastype assertions in TypeScript; the only exception isas const
Files:
src/util/propsAreEqual.tswww/src/views/GuideView.tsxwww/src/docs/apiCates.tswww/src/locale/en-US.tswww/src/components/GuideView/RoundedBars/index.tsxwww/src/locale/zh-CN.tsomnidoc/cross-component-prop-comments.spec.tstest/shape/Rectangle.spec.tsxwww/src/docs/api/index.tswww/src/docs/api/BarStack.tswww/src/docs/apiExamples/BarStack/index.tsxsrc/util/Global.tsomnidoc/componentsAndDefaultPropsMap.tssrc/shape/Rectangle.tsxsrc/index.tssrc/state/selectors/combiners/combineStackedData.tsomnidoc/readProject.spec.tswww/src/navigation.data.tssrc/cartesian/BarStack.tsxsrc/cartesian/Bar.tsxsrc/state/selectors/barStackSelectors.tswww/src/docs/apiExamples/index.tsxsrc/state/selectors/combiners/combineBarSizeList.tstest/cartesian/BarStack.spec.tsxomnidoc/readApiDoc.spec.tssrc/state/selectors/combiners/combineAllBarPositions.tswww/src/docs/exampleComponents/BarChart/index.tsxomnidoc/readProject.tssrc/state/selectors/radialBarSelectors.tstest/state/selectors/barStackSelectors.spec.tsxomnidoc/generateApiDoc.tswww/src/docs/exampleComponents/BarChart/RangedStackedBarChart.tsxsrc/state/selectors/barSelectors.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Ensure code lints by running
npm run lintand follows Airbnb's Style Guide
Files:
src/util/propsAreEqual.tswww/src/views/GuideView.tsxwww/src/docs/apiCates.tswww/src/locale/en-US.tswww/src/components/GuideView/RoundedBars/index.tsxwww/src/locale/zh-CN.tsomnidoc/cross-component-prop-comments.spec.tstest/shape/Rectangle.spec.tsxwww/src/docs/api/index.tswww/src/docs/api/BarStack.tswww/src/docs/apiExamples/BarStack/index.tsxsrc/util/Global.tsomnidoc/componentsAndDefaultPropsMap.tssrc/shape/Rectangle.tsxsrc/index.tssrc/state/selectors/combiners/combineStackedData.tsomnidoc/readProject.spec.tswww/src/navigation.data.tssrc/cartesian/BarStack.tsxsrc/cartesian/Bar.tsxsrc/state/selectors/barStackSelectors.tswww/src/docs/apiExamples/index.tsxsrc/state/selectors/combiners/combineBarSizeList.tstest/cartesian/BarStack.spec.tsxomnidoc/readApiDoc.spec.tssrc/state/selectors/combiners/combineAllBarPositions.tswww/src/docs/exampleComponents/BarChart/index.tsxomnidoc/readProject.tssrc/state/selectors/radialBarSelectors.tstest/state/selectors/barStackSelectors.spec.tsxomnidoc/generateApiDoc.tswww/src/docs/exampleComponents/BarChart/RangedStackedBarChart.tsxsrc/state/selectors/barSelectors.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/util/propsAreEqual.tssrc/util/Global.tssrc/shape/Rectangle.tsxsrc/index.tssrc/state/selectors/combiners/combineStackedData.tssrc/cartesian/BarStack.tsxsrc/cartesian/Bar.tsxsrc/state/selectors/barStackSelectors.tssrc/state/selectors/combiners/combineBarSizeList.tssrc/state/selectors/combiners/combineAllBarPositions.tssrc/state/selectors/radialBarSelectors.tssrc/state/selectors/barSelectors.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (DEVELOPING.md)
All imports from
rechartsmust use the public API entry point; imports from internal paths likerecharts/types/*orrecharts/src/*are not allowed
Files:
src/util/propsAreEqual.tswww/src/views/GuideView.tsxwww/src/docs/apiCates.tswww/src/locale/en-US.tswww/src/components/GuideView/RoundedBars/index.tsxwww/src/locale/zh-CN.tsomnidoc/cross-component-prop-comments.spec.tstest/shape/Rectangle.spec.tsxwww/src/docs/api/index.tswww/src/docs/api/BarStack.tswww/src/docs/apiExamples/BarStack/index.tsxsrc/util/Global.tsomnidoc/componentsAndDefaultPropsMap.tssrc/shape/Rectangle.tsxsrc/index.tssrc/state/selectors/combiners/combineStackedData.tsomnidoc/readProject.spec.tswww/src/navigation.data.tssrc/cartesian/BarStack.tsxsrc/cartesian/Bar.tsxsrc/state/selectors/barStackSelectors.tswww/src/docs/apiExamples/index.tsxsrc/state/selectors/combiners/combineBarSizeList.tstest/cartesian/BarStack.spec.tsxomnidoc/readApiDoc.spec.tssrc/state/selectors/combiners/combineAllBarPositions.tswww/src/docs/exampleComponents/BarChart/index.tsxomnidoc/readProject.tssrc/state/selectors/radialBarSelectors.tstest/state/selectors/barStackSelectors.spec.tsxomnidoc/generateApiDoc.tswww/src/docs/exampleComponents/BarChart/RangedStackedBarChart.tsxsrc/state/selectors/barSelectors.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.tsxrather than running all tests withnpm test
Files:
omnidoc/cross-component-prop-comments.spec.tstest/shape/Rectangle.spec.tsxomnidoc/readProject.spec.tstest/cartesian/BarStack.spec.tsxomnidoc/readApiDoc.spec.tstest/state/selectors/barStackSelectors.spec.tsx
test/**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Aim for 100% unit test code coverage when writing new code
Files:
test/shape/Rectangle.spec.tsxtest/cartesian/BarStack.spec.tsxtest/state/selectors/barStackSelectors.spec.tsx
test/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (test/README.md)
test/**/*.{test,spec}.{ts,tsx}: Aim for 100% unit test code coverage when writing new code
Prefer to use thecreateSelectorTestCasehelper function when writing or modifying tests
Use theexpectLastCalledWithhelper function instead ofexpect(spy).toHaveBeenLastCalledWith(...)for better typing and autocompletion
Verify the number of selector calls using the spy object fromcreateSelectorTestCaseto spot unnecessary re-renders and improve performance
MockgetBoundingClientRectin tests using the helper function provided intest/helper/MockGetBoundingClientRect.ts
Usevi.useFakeTimers()in all tests due to Redux autoBatchEnhancer dependency on timers andrequestAnimationFrame
Callvi.runOnlyPendingTimers()to advance timers after renders when not usingcreateSelectorTestCasehelper, and avoidvi.runAllTimers()to prevent infinite loops
UseuserEvent.setup({ advanceTimers: vi.runOnlyPendingTimers })or theuserEventSetuphelper function fromtest/helper/userEventSetup.tswhen creating userEvent instances
When testing tooltips on hover, usevi.runOnlyPendingTimers()after eachuserEvent.hover()call or use theshowTooltiphelper function fromtooltipTestHelpers.tsto account for requestAnimationFrame delays
Files:
test/shape/Rectangle.spec.tsxtest/cartesian/BarStack.spec.tsxtest/state/selectors/barStackSelectors.spec.tsx
🧠 Learnings (13)
📚 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} : Mock `getBoundingClientRect` in tests using the helper function provided in `test/helper/MockGetBoundingClientRect.ts`
Applied to files:
test/shape/Rectangle.spec.tsxtest/cartesian/BarStack.spec.tsxtest/state/selectors/barStackSelectors.spec.tsx
📚 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:
test/shape/Rectangle.spec.tsxscripts/snapshots/es6Files.txtsrc/state/selectors/barStackSelectors.tsscripts/snapshots/libFiles.txtscripts/snapshots/typesFiles.txttest/cartesian/BarStack.spec.tsxtest/state/selectors/barStackSelectors.spec.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 test/component/**/*.spec.tsx : Use React Testing Library for testing component interactions and behavior upon rendering
Applied to files:
test/shape/Rectangle.spec.tsxtest/cartesian/BarStack.spec.tsxtest/state/selectors/barStackSelectors.spec.tsx
📚 Learning: 2025-12-06T03:36:59.360Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: DEVELOPING.md:0-0
Timestamp: 2025-12-06T03:36:59.360Z
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:
www/src/docs/api/index.tsomnidoc/componentsAndDefaultPropsMap.tssrc/index.tsomnidoc/readProject.spec.tswww/src/docs/apiExamples/index.tsxscripts/snapshots/typesFiles.txtwww/src/docs/exampleComponents/BarChart/index.tsxsrc/state/selectors/radialBarSelectors.tsomnidoc/generateApiDoc.tswww/src/docs/exampleComponents/BarChart/RangedStackedBarChart.tsxsrc/state/selectors/barSelectors.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: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 **/*.spec.{ts,tsx} : 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`
Applied to files:
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} : Prefer to use the `createSelectorTestCase` helper function when writing or modifying tests
Applied to files:
scripts/snapshots/es6Files.txttest/cartesian/BarStack.spec.tsxtest/state/selectors/barStackSelectors.spec.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/index.tssrc/state/selectors/barStackSelectors.tsscripts/snapshots/typesFiles.txtwww/src/docs/exampleComponents/BarChart/index.tsxwww/src/docs/exampleComponents/BarChart/RangedStackedBarChart.tsx
📚 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} : Use the `expectLastCalledWith` helper function instead of `expect(spy).toHaveBeenLastCalledWith(...)` for better typing and autocompletion
Applied to files:
test/cartesian/BarStack.spec.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 storybook/stories/**/*.stories.tsx : Use Storybook for smoke tests and add play functions with assertions for actual tests
Applied to files:
test/cartesian/BarStack.spec.tsx
📚 Learning: 2025-11-19T14:08:01.728Z
Learnt from: PavelVanecek
Repo: recharts/recharts PR: 6659
File: www/src/components/GuideView/Performance/index.tsx:232-250
Timestamp: 2025-11-19T14:08:01.728Z
Learning: In Recharts version 3.4.2, object-as-prop optimizations were introduced to reduce unnecessary re-renders when new object references are passed as props. This affects the recommendation for the `react-perf/jsx-no-new-object-as-prop` ESLint rule.
Applied to files:
www/src/docs/exampleComponents/BarChart/index.tsx
📚 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} : Call `vi.runOnlyPendingTimers()` to advance timers after renders when not using `createSelectorTestCase` helper, and avoid `vi.runAllTimers()` to prevent infinite loops
Applied to files:
test/state/selectors/barStackSelectors.spec.tsx
📚 Learning: 2025-12-06T03:36:59.360Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: DEVELOPING.md:0-0
Timestamp: 2025-12-06T03:36:59.360Z
Learning: Run type checking with `npm run check-types` to verify TypeScript types
Applied to files:
omnidoc/generateApiDoc.ts
🧬 Code graph analysis (16)
www/src/views/GuideView.tsx (1)
www/src/components/GuideView/RoundedBars/index.tsx (1)
RoundedBars(9-51)
test/shape/Rectangle.spec.tsx (1)
src/shape/Rectangle.tsx (1)
RectRadius(20-20)
www/src/docs/api/index.ts (1)
www/src/docs/api/BarStack.ts (1)
BarStackAPI(3-32)
www/src/docs/api/BarStack.ts (1)
www/src/docs/api/types.ts (1)
ApiDoc(20-26)
www/src/docs/apiExamples/BarStack/index.tsx (1)
www/src/docs/exampleComponents/types.ts (1)
ChartExample(3-23)
omnidoc/componentsAndDefaultPropsMap.ts (1)
src/cartesian/BarStack.tsx (1)
defaultBarStackProps(70-72)
src/shape/Rectangle.tsx (1)
storybook/stories/API/props/RectangleProps.ts (1)
radius(8-21)
src/state/selectors/combiners/combineStackedData.ts (3)
src/util/stacks/stackTypes.ts (3)
AllStackGroups(26-26)StackSeries(43-43)StackGroup(32-35)src/state/types/StackedGraphicalItem.ts (1)
MaybeStackedGraphicalItem(9-15)src/util/stacks/getStackSeriesIdentifier.ts (1)
getStackSeriesIdentifier(14-18)
src/cartesian/BarStack.tsx (8)
src/index.ts (4)
BarStackProps(88-88)Layer(8-8)Rectangle(37-37)BarStack(87-87)src/util/ChartUtils.ts (3)
StackId(458-458)NormalizedStackId(464-464)getNormalizedStackId(466-468)src/shape/Rectangle.tsx (2)
RectRadius(20-20)Rectangle(141-252)src/context/PanoramaContext.tsx (1)
useIsPanorama(6-6)src/state/selectors/barStackSelectors.ts (1)
selectStackRects(86-93)src/util/useUniqueId.ts (1)
useUniqueId(13-27)src/util/resolveDefaultProps.tsx (1)
resolveDefaultProps(16-53)src/util/propsAreEqual.ts (1)
propsAreEqual(67-93)
src/cartesian/Bar.tsx (2)
src/shape/Rectangle.tsx (1)
RectRadius(20-20)src/cartesian/BarStack.tsx (2)
BarStackClipLayer(87-90)useStackId(59-68)
src/state/selectors/barStackSelectors.ts (6)
src/state/store.ts (1)
RechartsRootState(23-38)src/util/ChartUtils.ts (1)
NormalizedStackId(464-464)src/state/types/BarSettings.ts (1)
BarSettings(5-10)src/state/selectors/axisSelectors.ts (1)
selectUnfilteredCartesianItems(316-318)src/state/graphicalItemsSlice.ts (1)
CartesianGraphicalItemSettings(59-59)src/state/selectors/barSelectors.ts (1)
selectBarRectangles(268-347)
src/state/selectors/combiners/combineBarSizeList.ts (5)
src/util/DataUtils.ts (2)
isNullish(180-182)getPercentValue(51-82)src/state/types/StackedGraphicalItem.ts (3)
MaybeStackedGraphicalItem(9-15)DefinitelyStackedGraphicalItem(22-25)isStacked(27-31)src/state/selectors/barSelectors.ts (1)
SizeList(80-80)src/util/ChartUtils.ts (1)
StackId(458-458)src/index.ts (1)
DataKey(120-120)
test/cartesian/BarStack.spec.tsx (1)
src/cartesian/BarStack.tsx (3)
BarStack(150-150)useStackId(59-68)useBarStackClipPathUrl(78-85)
test/state/selectors/barStackSelectors.spec.tsx (5)
test/_data.ts (1)
PageData(4-11)src/state/selectors/barStackSelectors.ts (3)
selectAllBarsInStack(14-31)selectStackRects(86-93)expandRectangle(51-68)test/helper/expectLastCalledWith.ts (1)
expectLastCalledWith(14-34)src/state/cartesianAxisSlice.ts (1)
defaultAxisId(9-9)src/state/selectors/barSelectors.ts (1)
selectBarRectangles(268-347)
omnidoc/generateApiDoc.ts (1)
www/src/docs/api/types.ts (2)
ApiDoc(20-26)ApiProps(9-18)
www/src/docs/exampleComponents/BarChart/RangedStackedBarChart.tsx (4)
src/cartesian/BarStack.tsx (1)
BarStack(150-150)src/index.ts (2)
BarStack(87-87)Bar(85-85)src/cartesian/Bar.tsx (1)
Bar(1071-1071)storybook/stories/API/props/AnimationProps.ts (1)
isAnimationActive(29-36)
⏰ 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 (69)
src/shape/Rectangle.tsx (3)
17-20: LGTM!The
RectRadiustype consolidation is clean. The@inlineJSDoc annotation will help with documentation generation, and the union typenumber | [number, number, number, number]correctly captures all valid radius configurations.
22-22: LGTM!The function signature correctly uses the unified
RectRadiustype, which simplifies the parameter type from the previousnumber | RectRadiuspattern.
100-100: Verify Storybook documentation consistency.The
radiusprop now usesRectRadiustype. The Storybook documentation instorybook/stories/API/props/RectangleProps.ts(lines 7-20) showstype: { summary: 'number | number[]' }, which is slightly inconsistent with the actual typenumber | [number, number, number, number](a fixed 4-tuple, not a general array).Consider updating the Storybook summary to
'number | [number, number, number, number]'for accuracy.test/shape/Rectangle.spec.tsx (1)
7-7: LGTM!The test fixture type correctly uses
RectRadiusdirectly. SinceRectRadius = number | [number, number, number, number], both test cases ({ radius: 5 }and{ radius: [5, 10, 8, 15] }) are valid and provide good coverage of the radius prop variants.omnidoc/componentsAndDefaultPropsMap.ts (2)
33-33: LGTM!The import follows the established pattern used for other components in this file.
44-44: LGTM!The
BarStackentry is correctly added to thecomponentMetaMapin alphabetical order, following the established pattern for component metadata registration.src/state/selectors/combiners/combineStackedData.ts (1)
5-26: LGTM!The combiner function is well-structured with:
- Explicit parameter and return types
- Defensive null checks at each step
- Clear separation of concerns (identifier resolution, stack group lookup, series matching)
The multiple early returns make the function easy to follow and maintain.
scripts/snapshots/libFiles.txt (2)
15-15: LGTM!The
BarStack.jsentry is correctly added in alphabetical order within the cartesian directory.
135-145: LGTM!All new BarStack-related selector and combiner entries are correctly added to the library manifest in alphabetical order:
barStackSelectors.jscombineAllBarPositions.jscombineBarSizeList.jscombineStackedData.jswww/src/navigation.data.ts (1)
11-21: Guide slug wiring forroundedBarslooks correctNew
roundedBarsentry cleanly plugs into existingguidePagesrouting without behavior changes.www/src/locale/en-US.ts (1)
36-47: NewroundedBarsEnglish label is consistentKey and label align with existing guide entries and the new route.
www/src/locale/zh-CN.ts (1)
36-47: NewroundedBarszh-CN label is wired correctlyKey mirrors en-US and keeps the guide map in sync across locales.
www/src/docs/apiExamples/index.tsx (1)
23-29: BarStack API examples are integrated consistentlyImport and
allApiExamples.BarStackentry follow the established structure for other components.scripts/snapshots/es6Files.txt (1)
12-30: Snapshot updates for BarStack and selectors look alignedNew ES6 module paths for
BarStackand related bar stack selectors/combiners match the project’s directory and naming conventions.Also applies to: 130-147
www/src/docs/apiCates.ts (1)
23-41: BarStack added to Cartesian API category appropriately
BarStackis correctly listed among other cartesian components for API navigation.src/index.ts (1)
85-90: Public API export of BarStack is consistent
BarStackandBarStackPropsare exported in line with other cartesian components, keeping the main entry point coherent.www/src/docs/exampleComponents/BarChart/index.tsx (1)
104-118: Ranged stacked bar description now highlights BarStack usage clearlyThe added paragraphs succinctly explain BarStack’s role in rounded stacks and fit well with the existing example description.
www/src/docs/api/index.ts (2)
53-53: LGTM! BarStack import added correctly.The import follows the established pattern and uses the explicit
.tsextension consistently with other imports in the file.
70-70: LGTM! BarStack properly integrated into API documentation.The BarStack entry is correctly placed in the allExamples object, making it available in the public API documentation registry.
omnidoc/readApiDoc.spec.ts (2)
22-22: LGTM! Test expectations updated for BarStack.The addition of "BarStack" to the expected public symbol names correctly reflects the new component in the public API.
101-104: LGTM! New test coverage for Area's onAnimationStart comment.The test correctly verifies that the Area component's onAnimationStart prop comment can be read and matches the expected value.
src/state/selectors/radialBarSelectors.ts (1)
21-21: LGTM! Import reorganization improves code modularity.The combiner functions (
combineBarSizeList,combineAllBarPositions,combineStackedData) are now imported from dedicated combiner modules, improving code organization and separation of concerns. The public API remains unchanged.Also applies to: 40-42
omnidoc/cross-component-prop-comments.spec.ts (1)
75-79: LGTM! Exception justified for BarStack's stackId prop.The exception is appropriate because BarStack's
stackIdprop has unique semantics—it sets the stackId for all contained Bar components, which differs from how other components use stackId. This warrants a distinct description.package.json (1)
44-44: LGTM! New omnidoc script automates API documentation generation.The script correctly chains API doc generation with Prettier formatting. The
&&operator ensures Prettier only runs if the generation succeeds.www/src/docs/exampleComponents/BarChart/RangedStackedBarChart.tsx (2)
1-1: LGTM! Import uses public API entry point.The BarStack import correctly uses the public
rechartsAPI entry point as required by coding guidelines.
28-32: LGTM! BarStack usage demonstrates the new feature correctly.The example properly demonstrates BarStack functionality:
- BarStack wraps stacked bars and applies
radius={25}to the entire stack- Individual Bar components have
maxBarSize={50}for width controlstackIdis removed from individual Bars (as expected, since BarStack manages the stacking context)This effectively showcases how BarStack provides stack-wide rounded corners for stacked bar charts.
www/src/components/GuideView/RoundedBars/index.tsx (1)
1-51: LGTM! Well-structured guide component for rounded bars.The guide effectively explains rounded bar corner functionality through three clear sections with interactive examples:
- Uniform radius for all corners
- Per-corner radius control via arrays
- BarStack usage for stacked bar scenarios
The component structure is clean, educational content is well-organized, and code examples are appropriately demonstrated through CodeEditorWithPreview.
www/src/views/GuideView.tsx (1)
11-11: LGTM! RoundedBars guide properly integrated.The new guide is correctly wired into the guide system:
- Import added for the RoundedBars component
- Registered in guideMap with the
roundedBarskey for routing and localizationThe integration follows the established pattern for guide components.
Also applies to: 22-22
www/src/docs/apiExamples/BarStack/index.tsx (1)
1-11: LGTM!The API examples structure is correct and follows the
ChartExampletype pattern. The readonly array export is properly typed and contains the required fields.scripts/snapshots/typesFiles.txt (1)
15-15: LGTM!The new type definition entries are properly added in alphabetical order, correctly extending the public API surface for the BarStack feature and related selectors/combiners.
Also applies to: 135-145
src/cartesian/Bar.tsx (3)
211-211: LGTM!Good refactoring to use the centralized
RectRadiustype from Rectangle.tsx for the publicBarPropsinterface.
627-661: LGTM!The integration with
BarStackClipLayeris well-designed. It correctly wraps each bar rectangle and applies the clip-path URL from context when inside aBarStack, while gracefully returningundefinedwhen outside of one.
1027-1055: LGTM!The
useStackIdhook integration correctly resolves the stackId from the BarStack context or falls back to the prop value. The comment on line 1027 clearly documents the source of the stackId.www/src/docs/api/BarStack.ts (1)
1-32: LGTM!The API documentation is well-structured and provides comprehensive descriptions for the BarStack props. The explanations for
radiusandstackIdbehavior are particularly helpful for understanding the interaction between BarStack and individual Bar components.test/cartesian/BarStack.spec.tsx (1)
67-90: LGTM!Good coverage of the fallback behavior when BarStack is not present. The tests correctly verify that:
useStackId(undefined)returnsundefineduseStackId('child-stack')returns the child's stackIduseBarStackClipPathUrlreturnsundefinedtest/state/selectors/barStackSelectors.spec.tsx (3)
1-14: Imports and setup look good.The test file correctly imports from the public API (
recharts) and uses the recommended test helpers (createSelectorTestCase,expectLastCalledWith). Internal selector imports are appropriately from src paths for testing purposes.
15-26: Good test setup with stacked and non-stacked bars.The test fixture appropriately includes both bars within a BarStack (
red,green) and one outside (blue), enabling comprehensive selector testing.
145-232: Thorough utility function tests with clear documentation.The
expandRectangletests cover all edge cases (both undefined, one undefined, touching, gap, overlapping) with helpful comments explaining the expected calculations.omnidoc/readProject.spec.ts (4)
33-33: BarStack correctly added to exported symbols.The snapshot update reflects that BarStack is now part of the public API as a Variable symbol.
833-868: Enhanced prop metadata structure is well-tested.The updated assertions properly verify the new
jsDocstructure (withtagsMap andtext) andisRequiredfields for both DOM-originating and recharts-originating props.
907-923: BarStack.radius prop test validates new component documentation.The test properly verifies that BarStack's radius prop has the expected default value, JSDoc text, and metadata structure.
925-932: Good use of.failsto document known limitation.Using
it.failsappropriately documents the current type inference limitation while setting expectations for future improvement.omnidoc/README.md (1)
1-74: Comprehensive documentation for omnidoc tooling.The README provides clear documentation of the omnidoc tools, including usage examples, post-generation checklist, and architecture overview. This will help maintainers understand and use the tooling effectively.
omnidoc/generateApiDoc.ts (3)
37-94: Type simplification handles common patterns well.The
simplifyTypefunction covers import paths, unions, arrays, functions, React types, and objects. The logic is clear and handles edge cases appropriately.
150-160: Verify import extension compatibility.The generated import uses
.tsextension ('./types.ts'). Depending on the project's TypeScript/bundler configuration, this may cause issues. Consider using'./types'without extension for broader compatibility.- const fileContent = `import { ApiDoc } from './types.ts'; + const fileContent = `import { ApiDoc } from './types';
165-200: Clean CLI implementation with good error handling.The main function properly isolates errors per component, provides helpful console output, and the exports enable reuse in tests or other scripts.
src/state/selectors/combiners/combineAllBarPositions.ts (2)
24-26: Existing TODO/concern about first element check.The comment raises a valid question about why only the first element's
barSizeis checked. If the design intent is that either all bars have explicit sizes or none do, this should be documented. Otherwise, mixed scenarios (some bars with sizes, some without) may produce unexpected layouts.Is it intentional that only the first bar's
barSizedetermines the layout branch? If bars can have mixed explicit/implicit sizes, this logic may need adjustment.
94-121: Well-structured combiner function.The
combineAllBarPositionsfunction cleanly orchestrates bar position calculation with proper handling ofmaxBarSizeselection and band size offset adjustments.src/state/selectors/combiners/combineBarSizeList.ts (1)
7-18: LGTM!The
getBarSizehelper correctly prioritizes self-size over global size and properly handles nullish values before delegating togetPercentValue.omnidoc/readProject.ts (6)
18-37: LGTM!The
JSDocMetaand extendedPropMetatypes are well-structured, enabling richer documentation metadata extraction for API documentation generation.
202-233: LGTM!The property collection logic correctly derives
isRequiredfrom the declaration's question token presence, andjsDocis properly extracted via the new helper method.
401-432: LGTM!The refactored
getCommentOfmethod correctly prioritizes Recharts props for documentation, with proper fallback to DOM/other props.
456-478: LGTM!The
getTypeOfmethod safely extracts type information from the first declaration with proper null checks and error handling.
480-489: LGTM!The
isOptionalPropmethod correctly determines optionality by checking if any declaration marks the prop as required. The conservative fallback (returningfalseon error) is appropriate.
510-525: LGTM!The
getJsDocMetahelper correctly extracts both JSDoc text and tags, with proper null safety via optional chaining.src/cartesian/BarStack.tsx (7)
1-12: LGTM!Imports are well-organized. The import from
'../index'is appropriate for internal library code.
14-43: LGTM!The
BarStackPropstype is well-documented with comprehensive JSDoc explaining the behavior ofstackIdandradiusprops, including edge cases.
52-68: LGTM!The
useStackIdhook correctly implements the precedence logic where BarStack context takes priority over individual Bar's stackId, matching the documented behavior.
74-90: LGTM!The clip path utilities correctly generate unique IDs and handle the absence of BarStack context gracefully by returning
undefined.
92-129: LGTM!The
BarStackClipPathcomponent correctly renders SVG clip paths for each stack item, with proper null handling and unique keys.
131-145: LGTM!The
BarStackImplcomponent properly resolves unique IDs, applies default props, and memoizes the context value to prevent unnecessary re-renders.
147-150: LGTM!The
BarStackcomponent is correctly memoized withpropsAreEqualfor optimized re-renders, and the@since 3.6annotation is helpful for API documentation.src/state/selectors/barStackSelectors.ts (2)
14-31: LGTM!The
selectAllBarsInStackselector correctly filters bars by stack ID, panorama mode, and visibility. The chained.filter()calls prioritize readability.
44-68: LGTM!The
expandRectangleutility correctly computes the bounding rectangle that encompasses both input rectangles, with proper handling of undefined inputs.src/state/selectors/barSelectors.ts (5)
26-28: LGTM!The imports are correctly updated to use the new combiner modules, maintaining clean separation of concerns.
112-119: LGTM!The
selectBarSizeListselector correctly delegates to the newcombineBarSizeListcombiner with appropriate input selectors.
189-204: LGTM!The
selectAllBarPositionsselector correctly delegates tocombineAllBarPositionswith the necessary input selectors for bar position calculation.
259-266: LGTM!The
selectStackedDataOfItemselector correctly delegates tocombineStackedDatafor computing stacked data series.
268-347: LGTM!The
selectBarRectanglesselector remains well-structured with comprehensive null checks before computing bar rectangles.
| const combineStackRects: ( | ||
| state: RechartsRootState, | ||
| stackId: NormalizedStackId, | ||
| isPanorama: boolean, | ||
| ) => ReadonlyArray<BarStackItem | undefined> = (state, stackId, isPanorama) => { | ||
| const allBarIds = selectAllBarIdsInStack(state, stackId, isPanorama); | ||
| const stackRects: Array<BarStackItem | undefined> = []; | ||
| allBarIds.forEach(barId => { | ||
| const rectangles = selectBarRectangles(state, barId, isPanorama, undefined); | ||
| rectangles?.forEach((rect, index) => { | ||
| stackRects[index] = expandRectangle(stackRects[index], rect); | ||
| }); | ||
| }); | ||
| return stackRects; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "barStackSelectors|combineStackRects" test/ -A 5 -B 2Repository: recharts/recharts
Length of output: 833
🏁 Script executed:
# Also search for the entire barStackSelectors file to understand the context
cat -n src/state/selectors/barStackSelectors.tsRepository: recharts/recharts
Length of output: 3772
🏁 Script executed:
# Search for selectStackRects to see how combineStackRects is used
rg -n "selectStackRects" src/ -A 3 -B 2Repository: recharts/recharts
Length of output: 1455
🏁 Script executed:
cat -n test/state/selectors/barStackSelectors.spec.tsxRepository: recharts/recharts
Length of output: 9465
Consider adding selector call count verification to the test suite.
The combineStackRects function calls selectBarRectangles once per bar in the loop (line 78). While selectBarRectangles is memoized and will cache results for repeated barId values, the current test at line 132-142 in barStackSelectors.spec.tsx only validates the final output—it doesn't verify the number of selector calls or detect unnecessary re-renders.
To align with test best practices and the learning about selector performance, add a test that uses the spy object from createSelectorTestCase to count how many times selectBarRectangles is invoked and confirm this matches the expected number of bars. This would catch any future performance regressions if the logic changes.
🤖 Prompt for AI Agents
In src/state/selectors/barStackSelectors.ts around lines 70-84 the
combineStackRects function calls selectBarRectangles once per bar; add a unit
test (adjacent to the existing test in
src/state/selectors/barStackSelectors.spec.tsx around lines 132-142) that uses
the spy from createSelectorTestCase to count invocations of selectBarRectangles,
invoke combineStackRects with the same inputs used in the existing test, and
assert the spy call count equals the expected number of bars (i.e.,
allBarIds.length) to detect unnecessary selector calls or regressions.
| export const selectStackRects: ( | ||
| state: RechartsRootState, | ||
| stackId: NormalizedStackId, | ||
| isPanorama: boolean, | ||
| ) => ReadonlyArray<BarStackItem | undefined> = createSelector( | ||
| [state => state, pickStackId, pickIsPanorama], | ||
| combineStackRects, | ||
| ); |
There was a problem hiding this comment.
Memoization inefficiency: state => state defeats selector caching.
Using state => state as an input selector (line 91) means selectStackRects will recompute combineStackRects on every state change, not just when relevant state changes. This negates the performance benefits of createSelector.
Consider extracting specific state slices that combineStackRects actually depends on, or ensure that the combiner's internal selector calls provide sufficient caching.
export const selectStackRects: (
state: RechartsRootState,
stackId: NormalizedStackId,
isPanorama: boolean,
) => ReadonlyArray<BarStackItem | undefined> = createSelector(
- [state => state, pickStackId, pickIsPanorama],
+ [selectUnfilteredCartesianItems, pickStackId, pickIsPanorama],
combineStackRects,
);Note: This would require refactoring combineStackRects to accept the items array instead of the full state. Alternatively, if the current approach is intentional, add a comment explaining why.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/state/selectors/barStackSelectors.ts around lines 86 to 93, the input
selector `state => state` causes the selector to recompute on every state
change; replace it with one or more selectors that return only the specific
slice(s) `combineStackRects` needs (for example the stacks/items portion and any
other dependencies), and refactor `combineStackRects` to accept those slices
(e.g., an items array and stack metadata) instead of the whole state so
memoization works; if you intentionally need the whole state, add a clear
comment explaining why and why memoization cannot be narrowed.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
Tests are too slow again. I will fix |
Not bad Gemini 3, not bad. Let's see if that helps. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
omnidoc/readProject.ts (2)
18-21: JSDoc metadata extraction and Recharts‑first comment selection are well structuredStoring
JSDocMetaonPropMetaand usinggetCommentOfPropMetaingetCommentOfgives you a single source of truth and correctly prefers Recharts-origin declarations before falling back to DOM/other. ThegetJsDocMetahelper also centralizes tag parsing so you can extend it later (e.g.,@deprecated,@see) without changing call sites.One small tidy-up: unless you intend
getTextOfTagto be a public API, consider marking itprivateto avoid accidentally expanding the exposed surface ofProjectDocReader.Also applies to: 35-36, 221-231, 235-252, 428-459, 461-467, 536-552
143-155: Consider preferring Recharts declarations when computing prop types
getTypeOfcurrently takes the first declaration returned byproperty.getDeclarations(). For props that exist both in Recharts and in DOM types, that order is library-dependent and might pick the DOM declaration even though you’re documenting Recharts props (unlikegetCommentOf, which explicitly prefers Recharts origins).You can make
getTypeOfconsistent withgetCommentOfby preferring declarations whose origin is'recharts'and only falling back to the first declaration when none match:- const declarations = property.getDeclarations(); - if (declarations.length === 0) { - return undefined; - } - - const declaration = declarations[0]; - - const type = declaration.getType(); - return type.getText(); + const declarations = property.getDeclarations(); + if (declarations.length === 0) { + return undefined; + } + + const declaration = + declarations.find(d => this.getDeclarationOrigin(d) === 'recharts') ?? declarations[0]; + + const type = declaration.getType(); + return type.getText();This should give more accurate types in the generated API docs for props that overlap with DOM/SVG props.
Also applies to: 483-505
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
omnidoc/readProject.ts(11 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{ts,tsx}: Never useanytype (implicit or explicit) in TypeScript code
Preferunknownoveranyand 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 useastype assertions in TypeScript; the only exception isas const
Files:
omnidoc/readProject.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Ensure code lints by running
npm run lintand follows Airbnb's Style Guide
Files:
omnidoc/readProject.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (DEVELOPING.md)
All imports from
rechartsmust use the public API entry point; imports from internal paths likerecharts/types/*orrecharts/src/*are not allowed
Files:
omnidoc/readProject.ts
🧬 Code graph analysis (1)
omnidoc/readProject.ts (1)
omnidoc/DocReader.ts (1)
DocReader(3-42)
⏰ 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 (2)
omnidoc/readProject.ts (2)
42-47: Caching of symbols, components, and props looks correct and matches perf goalsThe new
symbolNamesCache,componentNamesCache, andpropCacheare keyed appropriately (by kind/absence and component name), only cache fully-computed, sorted arrays, and avoid recomputing expensive AST traversals. Given the project structure is static for the life ofProjectDocReader, the lack of invalidation is fine and explains the omnidoc test speedups without risking stale data.Also applies to: 55-57, 87-92, 95-111, 258-266, 415-421
247-249:isOptionalPropbehavior is coherent withisRequiredmetadata, fallback is conservativeUsing
Node.isPropertySignature(declaration) && !declaration.hasQuestionToken()to setisRequiredand then treating a prop as required if any declaration is required (some(p => p.isRequired)) is a reasonable heuristic for merged/intersection types. Returningfalseon exceptions (e.g., unknown component) is a conservative default for tooling, since it avoids falsely marking undocumented props as optional.No changes strictly needed here; just noting that this behavior will consider “partially optional” declarations as required overall, which is consistent with your inline comment.
Also applies to: 507-516
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #6746 +/- ##
==========================================
- Coverage 93.96% 93.92% -0.05%
==========================================
Files 504 512 +8
Lines 42177 42428 +251
Branches 4925 4960 +35
==========================================
+ Hits 39633 39849 +216
- Misses 2539 2574 +35
Partials 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Bundle ReportChanges will increase total bundle size by 18.64kB (0.7%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: recharts/bundle-es6Assets Changed:
view changes for bundle: recharts/bundle-umdAssets Changed:
view changes for bundle: recharts/bundle-cjsAssets Changed:
|
Description
Adds a new component: BarStack, that allows configuring settings for the whole stack of bars.
Also includes: omnidoc is now able to generate documentation too.
Related Issue
Fixes #6698
Fixes #4477
Fixes #3887
There's #759 which I am not sure if it's still active or solved? But we could add mouse handlers to BarStack too, if we need to.
Then there is #1888 but I think that is asking for something slightly different?
Motivation and Context
Bar.radiusprop is still valuable and makes sense in combination with the newBarStack.radius, some charts will use both.I was considering adding new prop to the main chart but then I decided for a component instead, looks more fun this way and allows having multiple stacks in one chart.
Types of changes
Checklist:
Summary by CodeRabbit
New Features
Documentation
Refactor
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.