Refactor barSelectors so that they read axis IDs from state instead of props by PavelVanecek · Pull Request #6723 · recharts/recharts

PavelVanecek · 2025-12-03T00:53:03Z

Description

Select data from state instead of props to keep the API simpler.

Summary by CodeRabbit

Refactor
- Simplified internal data retrieval architecture for bar chart components by consolidating identifier resolution pathways.
Tests
- Updated test cases to align with refactored data retrieval mechanisms, including explicit identifier assignments and corresponding selector invocations.

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

…f props

coderabbitai · 2025-12-03T00:53:16Z

Walkthrough

This pull request refactors bar selector signatures to accept a single GraphicalItemId (bar identifier) instead of separate xAxisId and yAxisId parameters. Selectors now derive axis associations internally from per-bar state settings, reducing argument count and centralizing bar-to-axis mapping logic.

Changes

Cohort / File(s)	Summary
Selector refactoring `src/state/selectors/barSelectors.ts`	Refactors 13+ selector signatures to accept GraphicalItemId instead of separate axis ids. Introduces `selectSynchronisedBarSettings`, `selectXAxisIdFromBar`, `selectYAxisIdFromBar` helpers to derive axis associations from bar state. Updates computation pipelines (sizing, stacking, rectangles) to route through per-bar state resolution with null/undefined guards.
Consumer update `src/cartesian/Bar.tsx`	Updates `selectBarRectangles` invocation from `(state, xAxisId, yAxisId, isPanorama, id, cells)` to `(state, id, isPanorama, cells)` to match new selector signature.
Test updates `test/cartesian/Bar.truncateByDomain.spec.tsx`, `test/cartesian/XAxis/XAxis.barSize.spec.tsx`, `test/cartesian/XAxis/XAxis.padding.spec.tsx`, `test/chart/BarChart.spec.tsx`	Update selector calls across multiple test files to use new bar-id-first signatures. Replace numeric axis indices with explicit bar/test identifiers. Add explicit `id` attributes to Bar components and update expectations to match new identifier scheme.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Primary concerns:
- Verify axis derivation logic in new helpers (selectXAxisIdFromBar, selectYAxisIdFromBar) correctly maps bar ids to their corresponding axes
- Check null/undefined guards throughout the selector refactor to ensure no unhandled edge cases when bar-to-axis mapping is missing
- Confirm all 13+ public selector signature changes are coordinated across call sites
- Validate that test identifier changes (explicit ids vs. regex patterns) reflect real component behavior and don't mask issues

Suggested reviewers

ckifer

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description is minimal but related to the changes. However, it lacks required template sections like Related Issue, Motivation and Context, How Has This Been Tested, and Types of changes.	Add missing template sections including Related Issue (link to #6723 or associated issue), detailed Motivation and Context explaining why this simplifies the API, and descriptions of testing performed and the type of change (breaking change, feature, etc.).
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately captures the main refactoring: bar selectors now derive axis IDs from state instead of accepting them as parameters.

✨ 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 barselector-refactor

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 492be75 and ed57c1e.

📒 Files selected for processing (6)

src/cartesian/Bar.tsx (1 hunks)
src/state/selectors/barSelectors.ts (8 hunks)
test/cartesian/Bar.truncateByDomain.spec.tsx (1 hunks)
test/cartesian/XAxis/XAxis.barSize.spec.tsx (1 hunks)
test/cartesian/XAxis/XAxis.padding.spec.tsx (1 hunks)
test/chart/BarChart.spec.tsx (33 hunks)

🧰 Additional context used

📓 Path-based instructions (8)

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

test/cartesian/XAxis/XAxis.padding.spec.tsx
test/cartesian/Bar.truncateByDomain.spec.tsx
test/cartesian/XAxis/XAxis.barSize.spec.tsx
test/chart/BarChart.spec.tsx
src/cartesian/Bar.tsx
src/state/selectors/barSelectors.ts

{test,www/test}/**/*.spec.{ts,tsx}

📄 CodeRabbit inference engine (DEVELOPING.md)

Write unit tests in the test or www/test directories with .spec.tsx file extension

Files:

test/cartesian/XAxis/XAxis.padding.spec.tsx
test/cartesian/Bar.truncateByDomain.spec.tsx
test/cartesian/XAxis/XAxis.barSize.spec.tsx
test/chart/BarChart.spec.tsx

test/**/*.spec.{ts,tsx}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Aim for 100% unit test code coverage when writing new code

Files:

test/cartesian/XAxis/XAxis.padding.spec.tsx
test/cartesian/Bar.truncateByDomain.spec.tsx
test/cartesian/XAxis/XAxis.barSize.spec.tsx
test/chart/BarChart.spec.tsx

**/*.{js,ts,tsx}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

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

Files:

test/cartesian/XAxis/XAxis.padding.spec.tsx
test/cartesian/Bar.truncateByDomain.spec.tsx
test/cartesian/XAxis/XAxis.barSize.spec.tsx
test/chart/BarChart.spec.tsx
src/cartesian/Bar.tsx
src/state/selectors/barSelectors.ts

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 the createSelectorTestCase helper function when writing or modifying tests
Use the expectLastCalledWith helper function instead of expect(spy).toHaveBeenLastCalledWith(...) for better typing and autocompletion
Verify the number of selector calls using the spy object from createSelectorTestCase to spot unnecessary re-renders and improve performance
Mock getBoundingClientRect in tests using the helper function provided in test/helper/MockGetBoundingClientRect.ts
Use vi.useFakeTimers() in all tests due to Redux autoBatchEnhancer dependency on timers and requestAnimationFrame
Call vi.runOnlyPendingTimers() to advance timers after renders when not using createSelectorTestCase helper, and avoid vi.runAllTimers() to prevent infinite loops
Use userEvent.setup({ advanceTimers: vi.runOnlyPendingTimers }) or the userEventSetup helper function from test/helper/userEventSetup.ts when creating userEvent instances
When testing tooltips on hover, use vi.runOnlyPendingTimers() after each userEvent.hover() call or use the showTooltip helper function from tooltipTestHelpers.ts to account for requestAnimationFrame delays

Files:

test/cartesian/XAxis/XAxis.padding.spec.tsx
test/cartesian/Bar.truncateByDomain.spec.tsx
test/cartesian/XAxis/XAxis.barSize.spec.tsx
test/chart/BarChart.spec.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:

test/cartesian/XAxis/XAxis.padding.spec.tsx
test/cartesian/Bar.truncateByDomain.spec.tsx
test/cartesian/XAxis/XAxis.barSize.spec.tsx
test/chart/BarChart.spec.tsx

src/**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (DEVELOPING.md)

Run ESLint and Prettier on the codebase using npm run lint

Files:

src/cartesian/Bar.tsx
src/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/cartesian/Bar.tsx
src/state/selectors/barSelectors.ts

🧠 Learnings (9)