Can you add some unit tests

Before looking into code, is it possible to have the API as width="auto"? If we add a new prop then we have to define what happens when people pass in both width and isWidthResponsive. Discoverability suffers.

Before looking into code, is it possible to have the API as width="auto"? If we add a new prop then we have to define what happens when people pass in both width and isWidthResponsive. Discoverability suffers.

You're right. The width="auto" property makes more sense. I will use that.

Can you add some unit tests

I will work on it @ckifer.

Run npm run test-build-output -- --update to get new bundle file snapshots, that will fix the build

Also, the YAxis label can be defined two different ways, 1. with label prop or 2. with <YAxis><Label /> child component, do your changes work with both?

Run npm run test-build-output -- --update to get new bundle file snapshots, that will fix the build

The command runs successfully in local but produces no change. Do I need to do something else?

If it's not working you can also add to the snapshots manually if you look at the failing build

Run npm run test-build-output -- --update to get new bundle file snapshots, that will fix the build

The command runs successfully in local but produces no change. Do I need to do something else?

Ah sorry I forgot. First npm run build that will generate the bundles, then npm run test-build-output -- --update that will update the bundle snapshot. Then you can run npm run build again to verify that it worked, commit and push.

Codecov Report

Attention: Patch coverage is 74.30168% with 46 lines in your changes missing coverage. Please review.

Project coverage is 95.38%. Comparing base (3d2b625) to head (19fd24f).
Report is 16 commits behind head on 3.x.

@@            Coverage Diff             @@
##              3.x    #5880      +/-   ##
==========================================
- Coverage   95.54%   95.38%   -0.16%     
==========================================
  Files         184      187       +3     
  Lines       18776    19076     +300     
  Branches     3790     3831      +41     
==========================================
+ Hits        17939    18196     +257     
- Misses        831      874      +43     
  Partials        6        6              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Can you add some unit tests

I am done with all other changes and was trying to add some test cases but I don't know why, but the render method is not rendering the y-axis properly when width='auto' is passed. It gives null when I try to get it using querySelector.

This is the test code I am using:

 it('should render y-axis with dynamically calculated width', async () => {
    const ticks = [0, 400, 1200, 1600];

    const { container } = render(
      <BarChart width={100} height={100}>
        <YAxis width="auto" ticks={ticks} />
        <CartesianGrid />
      </BarChart>,
    );

    const yAxis = container.querySelector('.yAxis');
    const yAxisWidth = yAxis.getBoundingClientRect().width;
    const yAxisLine = yAxis.querySelector('line');

    expect(yAxis).toBeVisible();
    expect(yAxisLine).toHaveAttribute('width', String(Math.round(yAxisWidth)));
  });

You're going to need to mock getBoundingClientRect. We should have a utility to do that somewhere

Bundle Report

Changes will increase total bundle size by 17.33kB (0.77%) ⬆️. This is within the configured threshold ✅

Affected Assets, Files, and Routes:

getBoundingClientRect

But YAxis itself is not being rendered when width="auto" is passed. I wanted to get the width of the axis that is being rendered and then use the util function get the width and compare them. In my current code, I get yAxis = null.

I also used debug. I found that YAxis doesn't even exist in the DOM.

Am I missing something or is it some limitation of the testing library?

Do you have any insight as of why is it not being rendered? If you add console.log({updatedYAxisWidth}) what do you see? If you place a debugger what do you see?

We have tests that show that YAxis does in fact render, for example in test/cartesian/YAxis.spec.tsx, so the testing library can do that.

Coltin was recommending to use mockGetBoundingClientRect({ width: 80, height: 30 });, have you tried that? Look at test/component/Legend.spec.tsx for example. jsdom by default returns all zeroes from getBoundingClientRect() (because it doesn't layout) so we have to mock the results in jsdom environment.