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

chart.js 3.7.1: Error thrown after updating data and subsequent user interaction #10467

Closed
bdorninger opened this issue Jul 6, 2022 · 9 comments · Fixed by #10523 or #10667
Closed
Milestone

Comments

@bdorninger
Copy link

bdorninger commented Jul 6, 2022

Expected behavior

After an update to the data (regardless if data points in the update were considered to be out of the chart area), interaction with the chart should be possible as usual

Current behavior

Context: Simple line chart with point objects, fixed scales (min, max)

  • An update of dataset's data with additional points causes chart.js to synchronously create empty PointElement instances (buildOrUpdateElements)

  • Subsequently, LineController checks for visible points (getStartAndCountOfVisiblePoints). If added points lie on or beyond an axis' bounds, previously created PointElement instances remain uninitialized.

  • If then the user interacts with the chart (hover / click), an Error is thrown, as evaluation functions (e.g. for displaying tooltips) try to access a PointElement's options, which are undefined.

Error in /turbo_modules/[email protected]/dist/chart.js (9319:84)
Cannot read properties of undefined (reading 'hitRadius')

This is causing the chart / app to crash. Happens on any browser, not just the one noted below

Reproducible sample

https://stackblitz.com/edit/typescript-w1ahqk

Optional extra steps/info to reproduce

The provided code (https://github.com/bdorninger/chartjs-update-bug) can be edited on stackblitz: (https://stackblitz.com/edit/typescript-w1ahqk)

  • Start the app
  • Wait for 2 secs until data is updated
  • Interact with the chart --> Error is thrown

Possible solution

If points are considered invisible (due to being out of bounds), please make sure they are properly initialized or are being removed from the internal dataset representation.

Workaround:

  • enable animations, set duration value to zero, if no animation wanted seems to help

Context

In our app, the chart is embedded in an Angular app. Chartjs is used to display live data being sent from a server. The app does not crash, but the chart is not usable anymore in such cases. Page has to be reloaded. Any time "unfavorable" data arrives from the server this happens again. No more interaction possible and the console is filled with error messages

chart.js version

v3.7.1 but also on 3.8.0

Browser name and version

Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/103.0.0.0 Safari/537.36'

Link to your project

https://github.com/bdorninger/chartjs-update-bug

@etimberg
Copy link
Member

This still occurs in 3.8.2.

Debugging

  1. The problem goes away if the last 3 points in the new data array have different X coordinates. I tested with 48, 49, 50 instead of 50, 50, 50
  2. The problem occurs inside of getStartAndCountOfVisiblePoints that only thinks the first 4 points are visible rather than the first 6

Possible Causes

  1. getStartAndCountOfVisiblePoints searches for the first point in the dataset that is at the max, but discounts multiple points at the same X coordinate
  2. Drawing renders all of the points at the final X coordinate

Solutions

It seems like the correct behaviour here is to have getStartAndCountOfVisiblePoints consider all of the points at the last data value rather than the first. I think this could happen by switching out _lookupByKey for _rlookupByKey in https://github.com/chartjs/Chart.js/blob/master/src/controllers/controller.line.js#L155-L158

CC @kurkle @LeeLenaleee thoughts?

@etimberg etimberg added this to the Version 3.9.0 milestone Jul 24, 2022
@etimberg
Copy link
Member

Did some testing, rlookupByKey is the wrong solution, but I think it's on the right track. We need to search the sorted array from right to left to find the point rather than left to right

@kurkle
Copy link
Member

kurkle commented Jul 25, 2022

Without looking at the code, doesn't the lookup util return .lo and .hi?

@LeeLenaleee
Copy link
Collaborator

It seems like the correct behaviour here is to have getStartAndCountOfVisiblePoints consider all of the points at the last data value rather than the first. I think this could happen by switching out _lookupByKey for _rlookupByKey

Won't you get the same issue as you have now with this is you have multiple points with the same x value at the start of the chart?

Without looking at the code, doesn't the lookup util return .lo and .hi?

Yes

@etimberg
Copy link
Member

I don't think the issue will happen at the start because we'd look left-to-right for the start and right-to-left for the end.

re .lo and .hi we already use .hi for the max value

@LeeLenaleee
Copy link
Collaborator

mb, I thought you wanted to search rtl for both points, than it makes more sense.

@temaivanoff
Copy link

Hi, problem is not solved 3.9.1. I updating data and drag viewpoint

chart.mjs:8866 Uncaught TypeError: Cannot read properties of undefined (reading 'hitRadius')
    at PointElement.inRange (chart.mjs:8866:1)
    at evaluationFunc (chart.mjs:5509:1)
    at evaluateInteractionItems (chart.mjs:5434:1)
    at getNearestCartesianItems (chart.mjs:5540:1)
    at getNearestItems (chart.mjs:5549:1)
    at nearest (chart.mjs:5637:1)
    at Chart.getElementsAtEventForMode (chart.mjs:7668:1)
    at Chart._getActiveElements (chart.mjs:8049:1)
    at Chart._handleEvent (chart.mjs:8012:1)
    at Chart._eventHandler (chart.mjs:7993:1)
    at listener (chart.mjs:7832:1)
    at Chart.event (chart.mjs:6320:1)
    at helpers.segment.mjs:565:1

@LeeLenaleee
Copy link
Collaborator

@fast0490f do you have a reproducable because it seems to be solved: https://stackblitz.com/edit/typescript-kzlsgp?file=package.json

@ggcannard
Copy link

@LeeLenaleee I am still running into this as well. Here is a sample, I've only changed the data slightly from the original.

https://stackblitz.com/edit/typescript-tgji1b?file=index.ts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants