search

chartjs / Chart.js

Simple HTML5 Charts using the <canvas> tag
https://www.chartjs.org/
MIT License
63.9k stars 11.88k forks source link

ensure `getComputedStyle` always has a valid canvas provided #11809

Open DAcodedBEAT opened 2 weeks ago

DAcodedBEAT commented 2 weeks ago

Expected behavior

getComputedStyle() from helpers.dom.js should be called with valid canvas values provided.

Current behavior

TypeError: Cannot read properties of null (reading 'ownerDocument')
  at getComputedStyle(../../node_modules/chart.js/dist/chunks/helpers.segment.js:1841:47)
  at getMaximumSize(../../node_modules/chart.js/dist/chunks/helpers.segment.js:1933:17)
  at DomPlatform.getMaximumSize(../../node_modules/chart.js/dist/chart.js:3435:12)
  at _a._resize(../../node_modules/chart.js/dist/chart.js:5700:35)
  at detached(../../node_modules/chart.js/dist/chart.js:6267:12)
  at _a.bindResponsiveEvents(../../node_modules/chart.js/dist/chart.js:6273:16)
  at _a.bindEvents(../../node_modules/chart.js/dist/chart.js:6215:12)
  at _a._checkEventBindings(../../node_modules/chart.js/dist/chart.js:5911:12)
  at _a.update(../../node_modules/chart.js/dist/chart.js:5860:10)
  at this._doResize(../../node_modules/chart.js/dist/chart.js:5628:46)
  at sentryWrapped(../../node_modules/@sentry/browser/esm/helpers.js:36:17)

From sentry:

image

Reproducible sample

.

Optional extra steps/info to reproduce

No response

chart.js version

4.4.3

Browser name and version

Experienced on Chrome >=124.0.0 according to my project's Sentry, but probably impacts others since this is an issue with the provided parameter.

Link to your project

No response

DAcodedBEAT commented 2 weeks ago

@etimberg / @LeeLenaleee I used optional chaining and null coalescing because of this discussion - https://github.com/chartjs/Chart.js/pull/11651#issuecomment-1925881486, which is why the CI pipeline failed. Give the word and I'll remove the rule so this pipeline can pass.

DAcodedBEAT commented 1 week ago

@etimberg / @LeeLenaleee can you re-review this?

LeeLenaleee commented 1 week ago

It feels kind of hacky to just return semi default values if the canvas does not exist.

Can you reproduce the errors because it seems to me that if you remove the parents and keep chart.js alive. But if you destroy the chart first these errors also should not happen