[@mantine/charts] Add yAxisId="left" to each <CartesianGrid>

[danielhjacobs]

Fixes #8110

According to https://github.com/recharts/recharts/wiki/3.0-migration-guide#other-breaking-changes:

• CartesianGrid has new properties x/yAxisId that correspond to x/yAxisIds on the X/YAxis components respectively. With a different ID than default, CartesianGrid lines will fail to render. This makes the grid rendering deterministic rather than just selecting the first axis it finds (Some grid lines are missing on BarChart with Recharts 3 when using yAxisId="left" recharts/recharts#6149)

They seem to tie to the <Bar>/<Line>/etc. elements too, whether or not there exists xAxisId/yAxisId on XAxis/YAxis.

From testing when all <Bar>s have "right" yAxidIds, the CartesianGrid does not show axis lines unless the <CartesianGrid> also has a "right" yAxisId, but on Discord vitaly noted "I’d rather just set it statically without calculations. It can be overridden with gridProps."

[rtivital]

Please fix errors in tests

[danielhjacobs]

So the problem is yAxisId was not one of the CartesianGridProps until recharts/recharts#4795, which means I'd need to use some heuristic to determine if recharts 3 is in use and switch to {...(isRechartsV3 && { yAxisId: 'left' })}. What heuristic do you think I should use to define isRechartsV3?

[rtivital]

If there is a reliable way of differentiating those versions, then it should be used. Otherwise, I think it is better to introduce these changes in 9.0 version planned for the next year.

[danielhjacobs]

I think this check would work:

import * as Recharts from 'recharts';

function isRechartsV3() {
  return typeof (Recharts as any).useActiveTooltipLabel === 'function';
}

          <CartesianGrid
            strokeDasharray={strokeDasharray}
            vertical={gridAxis === 'y' || gridAxis === 'xy'}
            horizontal={gridAxis === 'x' || gridAxis === 'xy'}
            {...(isRechartsV3() && { yAxisId: 'left' })}
            {...getStyles('grid')}
            {...gridProps}
          />

I don't know how you feel about adding the import * as Recharts from 'recharts';

[danielhjacobs]

Used detection of useActiveTooltipLabel, the first official recharts public hook: recharts/recharts#5452. I couldn't just import it, I had to import * as Recharts and cast Recharts as any, as recharts 2 has no exported member useActiveTooltipLabel.

[shehryar-turing]

Thanks for the quick fix and the helpful context from the Recharts 3 migration guide!

A few suggestions and potential issues to consider:

1. Default yAxisId selection may break right-only Y axis setups in v3

• In v3, CartesianGrid ties to a specific axis scale via xAxisId/yAxisId. Setting yAxisId="left" by default is fine for the common case, but if a chart uses only the right Y axis (e.g., withRightYAxis and all series have yAxisId="right"), grid lines will not render unless the grid also uses yAxisId="right".
• You mention it can be overridden via gridProps, which is good. If you want to keep defaults robust without “heavy” calculations, a minimal heuristic might help:
  • If withRightYAxis is true and every series has yAxisId === 'right', set grid yAxisId to 'right', otherwise 'left'.
  • This covers the most common failure mode and keeps logic simple.

2. Consider also setting xAxisId in v3

• The Recharts 3 breaking changes mention both xAxisId and yAxisId on CartesianGrid. We usually have a single XAxis so it likely works without being explicit, but being explicit could make the behavior more deterministic across configurations (especially if multiple X axes are introduced later).

3. Duplicate isRechartsV3 logic across files

• isRechartsV3 is duplicated in 5 components and each file adds a namespace import. Consider moving the version detection to a small shared util (e.g., packages/@mantine/charts/src/utils/is-recharts-v3.ts) and re-exporting from utils/index.ts for reuse. That reduces duplication and makes future adjustments to detection easier.

4. TypeScript compatibility across Recharts 2 and 3

• Depending on the local type versions, CartesianGridProps in v2 might not include yAxisId. The spread approach usually works at runtime, but if you ever see TS friction when building with v2 types, you can cast the conditional object to any to silence it:
  • {...(isRechartsV3() ? ({ yAxisId: 'left' } as any) : {})}
• Not necessarily required if CI builds with v3 types, just a heads up for cross-version typing.

5. Tests and docs

• It could be helpful to add a lightweight unit test that:
  • Mocks Recharts to simulate v2 (no useActiveTooltipLabel) and v3 (with it), and asserts whether yAxisId gets applied to CartesianGrid accordingly.
  • Adds a demo/doc note for Recharts 3 users who rely solely on the right Y axis to either set gridProps={{ yAxisId: 'right' }} or use whichever heuristic you prefer if implemented.

Overall, the change looks good and aligns with the v3 requirement. Addressing the right-only axis case and deduping the version check would polish it nicely.

[danielhjacobs]

1. Default yAxisId selection may break right-only Y axis setups in v3

   You're right, in fact, this is the case even if withRightYAxis is false, as long as every series has yAxisId === 'right'. I suggested on Discord using this:

   const allBarsUseRightAxis = series.every((item) => item?.yAxisId === 'right');
   
   {...(isRechartsV3() && { yAxisId: allBarsUseRightAxis ? 'right' : 'left' })}

   But @rtivital said "I’d rather just set it statically without calculations." That's why I opted for this simpler, less correct implementation.

2. Consider also setting xAxisId in v3

   This is not necessary because neither the XAxis nor any of the graphical components have an xAxisId: https://github.com/search?q=repo%3Amantinedev%2Fmantine%20%22xAxisId%22&type=code. Therefore, I don't want to over-complicate things by adding it

3. Duplicate isRechartsV3 logic across files

   Good suggestion

4. TypeScript compatibility across Recharts 2 and 3

   I can do this, doesn't seem strictly necessary but neither is it harmful

5. Tests and docs

   I can add a doc note. Not sure if any tests use recharts 3 right now, would I need to introduce an infrastructure for that? Edit: Nevermind, I now mock the recharts module for v2 and v3 based on including or not including a mock hook for useActiveTooltipLabel.

[danielhjacobs]

Ignored number 1 based on the request by @rtivital to avoid calculations. Ignored number 2 because it should be completely unnecessary. Did number 3. Did number 4. Per number 5, added docs notes anywhere changing the yAxisId of the series was mentioned in the docs.

If the lightweight unit test is deemed necessary, I'd like help writing it. Otherwise, this should be ready for re-review.

[danielhjacobs]

Made a test for this, I'm checking the return of isRechartsV3 for it though, not going all the way to checking the yAxisId of the CartesianGrid. To me, that seemed overly complex.

[danielhjacobs]

This PR should be ready for re-review.

[danielhjacobs]

Well, the test worked locally with the import, but I guess with the CI build it didn't work to mock recharts, I think the update should let the test work.

[danielhjacobs]

Came back to this with a fresh mind a week later, https://jestjs.io/docs/jest-object#jestisolatemodulesasyncfn proved to be the function I needed so the installed recharts version wouldn't interfere with my mock in the test.

Edit: I guess I still need the require in the function.

[danielhjacobs]

...I swear I'm testing my changes locally, and locally they usually work. I have no idea what the difference is in CI.

[danielhjacobs]

Created this shell script, ran it a few times with my old code, and packages/@mantine/charts/src/utils/is-recharts-v3/is-recharts-v3.test.ts failed half the time and succeeded half the time:

#!/usr/bin/env bash
set -euo pipefail

echo "🔹 Removing node_modules..."
rm -rf node_modules

echo "🔹 Clearing Yarn cache..."
rm -rf ~/.cache/yarn

echo "🔹 Cleaning build artifacts (esm, cjs, styles.css)..."
find packages -type d -name esm -exec rm -rf {} +
find packages -type d -name cjs -exec rm -rf {} +
find packages -type f -name styles.css -delete

echo "🔹 Installing dependencies..."
yarn install --frozen-lockfile

echo "🔹 Running setup..."
npm run setup

echo "🔹 Building packages..."
npm run build all

echo "🔹 Running tests..."
CI=true npm test

With my new code, I ran that script 5 times and packages/@mantine/charts/src/utils/is-recharts-v3/is-recharts-v3.test.ts succeeded every time. Hoping that means CI will succeed too. I'm 99% sure my code has been working the whole time, it's just that the proper recharts mock is not being used for the test.

[danielhjacobs]

Darn, no luck with CI...

[danielhjacobs]

Ok, I'm fairly sure there's no good way to fix the test because the tests run in parallel. As such, if a test that imports the real recharts runs concurrently with this test file, the real recharts may be taken into account instead of the mocked recharts.

https://jestjs.io/docs/cli#--runinband

[danielhjacobs]

I wouldn't trust the latest CI success, it was still flaky for me locally.

[danielhjacobs]

Ran this on the current PR:

#!/usr/bin/env bash
set -euo pipefail

SUCCESS=0
FAIL=0

for i in {1..5}; do
  echo "========================================"
  echo "▶️ Run $i of 5"
  echo "========================================"

  if ./ci-clean-test.sh; then
    echo "✅ Run $i succeeded"
    SUCCESS=$((SUCCESS + 1))
  else
    echo "❌ Run $i failed"
    FAIL=$((FAIL + 1))
  fi
done

echo "========================================"
echo "Summary:"
echo "  Successes: $SUCCESS"
echo "  Failures : $FAIL"
echo "========================================"

# Exit with failure if any run failed
if [ "$FAIL" -gt 0 ]; then
  exit 1
fi

Unfortunately:

========================================
Summary:
  Successes: 4
  Failures : 1
========================================

[rtivital]

I think I'll keep it as is for now and wait for Mantine 9.0 to introduce recharts 3 only version.

[rtivital]

Thanks for submitting the PR and your research work. Today, I've integrated these changes in 9.0 release which supports recharts 3 only. Currently, there is no plan of fixing bugs with reacts 3 + Mantine 8.x.