Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Brush Error: Cannot read property 'range' of undefined #2093

Closed
avindra opened this issue Apr 12, 2020 · 6 comments
Closed

Brush Error: Cannot read property 'range' of undefined #2093

avindra opened this issue Apr 12, 2020 · 6 comments

Comments

@avindra
Copy link
Contributor

avindra commented Apr 12, 2020

This is a continuation of Issue #1187 . This issue was re-filed as requested by @xile611 . Thanks to @ZephD for creating the error repro case.

Reproduction link

Edit on CodeSandbox

Steps to reproduce

Open the code sandbox, then resize the preview window pane

What is expected?

It should continue rendering normally

What is actually happening?

The code crashes, throwing this error:

Cannot read property 'range' of undefined
Brush.UNSAFE_componentWillReceiveProps
https://gqjm4.csb.app/node_modules/recharts/lib/cartesian/Brush.js:158:20
callComponentWillReceiveProps
https://gqjm4.csb.app/node_modules/react-dom/cjs/react-dom.development.js:12976:14
updateClassInstance
https://gqjm4.csb.app/node_modules/react-dom/cjs/react-dom.development.js:13178:7
updateClassComponent
https://gqjm4.csb.app/node_modules/react-dom/cjs/react-dom.development.js:17107:20
beginWork
https://gqjm4.csb.app/node_modules/react-dom/cjs/react-dom.development.js:18620:16
HTMLUnknownElement.callCallback
https://gqjm4.csb.app/node_modules/react-dom/cjs/react-dom.development.js:188:14
Object.invokeGuardedCallbackDev
https://gqjm4.csb.app/node_modules/react-dom/cjs/react-dom.development.js:237:16
invokeGuardedCallback
https://gqjm4.csb.app/node_modules/react-dom/cjs/react-dom.development.js:292:31
beginWork$1
https://gqjm4.csb.app/node_modules/react-dom/cjs/react-dom.development.js:23203:7
performUnitOfWork
Environment Info
Recharts v2.0.0-beta.5
React 16
System Linux
Browser Chrome 80

@avindra
Copy link
Contributor Author

avindra commented Jun 4, 2020

Thanks for the fix @xile611 , can confirm it works great in v2 beta 6

@NereonNeo
Copy link

NereonNeo commented May 7, 2024

Error: Cannot read properties of undefined (reading 'name')

Hi everyone! I'm not sure if this will be useful to anyone, but I wanted to share how I solved a similar error.

The issue was that when the data in the Brush component changed, it would completely break and throw an error (Cannot read properties of undefined (reading 'name')). I understood that this error was occurring because the new data might have had fewer elements, causing it to fail when trying to access properties using the old indices. However, I couldn't figure out how to resolve it! Out of desperation, I dove into the Brush component's code and found an interesting snippet:

var ariaLabelBrush = ariaLabel || "Min value: ".concat(data[startIndex].name, ", Max value: ").concat(data[endIndex].name);
it seemed to be expecting ariaLabel, which was odd because there's no mention of ariaLabel in the Recharts Brush documentation (https://recharts.org/en-US/api/Brush).

In the end, I provided ariaLabel to the Brush component, and everything started working!

ariaLabel="Brush"

Here's the full code:

<Brush
  data={data}
  className={cn("brush-custom translate-y-2")}
  padding={{
    left: 5,
    right: 5,
    top: 5,
    bottom: 5,
  }}
  tickFormatter={() => ""}
  height={47}
  alwaysShowText={false}
  traveller={customTraveller}
  travellerWidth={0}
  ariaLabel="Brush"
  onChange={updateBrushIndex}
  startIndex={brushIndex.startIndex}
  endIndex={brushIndex.endIndex}
>
  <BarChart barCategoryGap={0} barGap={0} data={data} barSize={20}>
    {barChartConfig.map(renderBarChartSecond)}
  </BarChart>
</Brush>

I will be glad if this helps someone =) Good luck!

@ckifer
Copy link
Member

ckifer commented May 7, 2024

The reason for it not being in the docs is the same as the other implicit props - recharts passes all applicable SVG properties down to the element. It's a hard thing to document, but we'll try to make it better somehow.

Anyways, that needs guarded against! It shouldn't matter if you pass your own label or not. And if you update data, the indices should update too. It's just trying to generate a reasonable label from the data when there isn't one passed

@ckifer
Copy link
Member

ckifer commented May 8, 2024

@NereonNeo may I ask why you pass data directly to Brush and don't rely on data to be passed down from the parent chart?

I ask this because in 3.x we're removing data as a public property from Brush - ideally you are brushing a chart and that chart has the same data as the Brush.

Or was that just for the example?

@ckifer
Copy link
Member

ckifer commented May 8, 2024

Added optional chaining to that property access in v2.12.7, shouldn't be an issue anymore

ckifer added a commit that referenced this issue May 8, 2024
<!--- Provide a general summary of your changes in the Title above -->

## Description

<!--- Describe your changes in detail -->
Fix brush bug experienced in current release, guard against it in 3.x
too

## Related Issue

<!--- This project only accepts pull requests related to open issues -->
<!--- If suggesting a new feature or change, please discuss it in an
issue first -->
<!--- If fixing a bug, there should be an issue describing it with steps
to reproduce -->
<!--- Please link to the issue here: -->
This is interesting... not sure why people use Brush like this? Asked
for a reason in the comment
#2093 (comment)

## Motivation and Context

<!--- Why is this change required? What problem does it solve? -->
bug prevention

## How Has This Been Tested?

<!--- Please describe in detail how you tested your changes. -->
<!--- Include details of your testing environment, and the tests you ran
to -->
<!--- see how your change affects other areas of the code, etc. -->
it hasn't really, but it doesn't hurt anything

## Screenshots (if appropriate):

## Types of changes

<!--- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->

- [X] 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:

<!--- Go over all the following points, and put an `x` in all the boxes
that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->

- [ ] 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 extended an existing story to
show my changes

Co-authored-by: Coltin Kifer <ckifer@amazon.com>
@NereonNeo
Copy link

@NereonNeo may I ask why you pass data directly to Brush and don't rely on data to be passed down from the parent chart?

I ask this because in 3.x we're removing data as a public property from Brush - ideally you are brushing a chart and that chart has the same data as the Brush.

Or was that just for the example?

Hey @ckifer I didn’t notice this, thanks for the warning 🙏, I’ll fix it

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

No branches or pull requests

3 participants