Skip to content

fix(#11315): don't call getLabelAndValue on null controller#11576

Closed
mihai-peteu wants to merge 1 commit into
chartjs:masterfrom
mihai-peteu:fix/11315-getLabelAndValue-null-controller
Closed

fix(#11315): don't call getLabelAndValue on null controller#11576
mihai-peteu wants to merge 1 commit into
chartjs:masterfrom
mihai-peteu:fix/11315-getLabelAndValue-null-controller

Conversation

@mihai-peteu

Copy link
Copy Markdown

Suggested fix for #11315

@mihai-peteu

Copy link
Copy Markdown
Author

@etimberg | @LeeLenaleee
CI got snagged on a codeclimate check. I'm open to solutions on how to trim the function I touched down to 25 lines 😬

@LeeLenaleee LeeLenaleee left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to have a test that shows this will fix it. Because I think you might shift the problem to other parts where the code expects values to be present which get set to null/undefined right now

@joshkel

joshkel commented Nov 28, 2023

Copy link
Copy Markdown
Contributor

This change may have unintended effects. Currently, the TooltipItem type is defined as always providing a dataset. I experimented with this fix locally, any my code threw errors because my callbacks expected a dataset that was now not necessarily present.

I have an alternative candidate fix at #11596; I'm not sure if that approach is better or not.

@mihai-peteu

Copy link
Copy Markdown
Author

Closing in favor of #11596

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

Successfully merging this pull request may close these issues.

3 participants