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

Draft proposal and open for discussion: timeseries chart crashing when too many ticks #160

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

RafaelSzmarowski
Copy link
Contributor

It's a draft proposition of a solution to solve an old but 'niche' crash of chartjs when setting a time unit on a too big range. For example range from 1600 to 2020 and time unit set to day which resolves in a crash.

The problem is mainly that we allow to force and set a time unit that is not compatible with the data range. We could believe that it should be handled by the user but in our case data are not static and can evolve so it appears that the check should be done on the component level and prevent the crash here.

We have few options that I see:

  • (solution in this draft PR) Write a function to check when building scales that the option of time unit is compatible with the range, fallback to no time unit option instead to avoid error. The problem is that we have to base our function on a constant dataFrame object with a specific format which can be painful to maintain and a huge constraint to our SDK users.
    Another solution would be to make this check optional and use it only in Studio where we control dataFrame format, only maintain it with API changes ?
  • Wrap initialisations or updates of our instance of chartjs with a try and catch, display caught errors instead of component. (Probably an easy win solution but very global and a bit lazy)
  • Write a custom plugin that will be made afterDataLimits are computed by chartjs that will check and change options (but I haven't found a way to cancel the options passed in Chart.svelte so this solution ended up in too many updates and a crash as well.)
  • Design a global Svelte Error Boundary to prevent uncaught errors through React Error Boundary (it was already explored a few months ago here: First draft implementation of global Svelte Error boundary #62 but maybe some better solution are now available in Svelte)

I’m not really happy with any of the solutions that’s why I haven’t finished to really write the function and this PR and I’m curious of your opinions here, anyone that has some free time and want to contribute with his opinion or a proposition on this subject is more than welcome.

@etienneburdet
Copy link
Contributor

Made a slight comment about the implem itself, but my bigger point: I like the function, but I'm for putting that check in the {#if data.error} …, rather than providing a fallback. It would make the error explicit. Then if it's the app job (for us the Studio) to decide if we want to provide a fallback or not. We could export that function as a util if need be.

Chart is the only to have that kind of error wrapper and I think it's a good mechanism. I don't think we should do to much for the React error boundary, if the Svelte component catches some error we want it's good enough already imo.

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.

None yet

2 participants