upgrade elastic/charts from v57 to v66#226
Merged
marjisound merged 1 commit intomainfrom Jul 17, 2024
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this change?
Upgrade elastic/charts from
v57.0.0tov66.1.0. The only usage of this dependancy is inTimeHistogram.jsand the upgrade does not have any breaking changes in our code base apart from the chart colour which I fixed.I updated this as an attempt to fix the
d3-colorvulnerability in https://github.com/guardian/giant/security/dependabot/2 that's suggesting the vulnerable version is introduced by elastic/charts. But it seems that this vulnerebility is also introduced byd3-scalewhich is also a sub dependancy of the latest version of elastic/charts. So this change doesn't seem to be fixing the main issue of d3-color vulnerability. But it's still good to have the most recent versionHow to test
tested locally
The only change in the UI is that the default number of desired ticks in the Y-Axis from 10 to 5. I left it to be default since it's mentioned in the actual elastic chart PR that this helps a bit more when rendering short charts and will help reduce the clutter.
elastic/elastic-charts#2005
before
after