-
Notifications
You must be signed in to change notification settings - Fork 22
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
Passing Zone object to options.adapters.date causes invalid dates. #18
Comments
Oh, so the issue is that Chart.js expects all options to be object literals and if you pass in an instance of some other type then Chart.js clones it and you lose the class instance. Is that it? If I've correctly described the issue, I think the best thing to do would be to report it in the main Chart.js repo and see if we can get a real fix for the underlying issue. We're currently working on Chart.js 3, so if we need to change the behavior of options in a breaking way to fix it this would be the time to do it |
Linking the upstream issue: chartjs/Chart.js#7340 |
@iddings thanks for filing the issue upstream I just took a look at the Luxon docs and didn't see any examples of using a |
Sure, Without a workaround, passing an instance of This example has a relatively easy workaround (i.e. using a I'll add this example to the upstream issue as well. |
This should be resolved in chart.js v3.0.0-beta.12 since options are no longer merged |
This isn't really a bug with this adapter, but it doesn't seem like this warrants an issue in the Chart.js main repo.
Because of the way Chart.js merges options, things like a luxon FixedOffsetZone get broken. I.e. if you pass a FixedOffsetZone object as the "zone" option to this adapter, Chart.js will convert it to an object like this:
{ fixed: -300 }
. Luxon will then see that as an invalid zone.One fix for this could be catching that edge case converting the zone from (e.g.)
{ fixed: -300 }
to simply-300
, which Luxon will handle properly.Locally I've worked this out by adding:
and replacing references to
this.options
withthis._normalizedOptions()
elsewhere.If this is deemed not applicable to this repo, then I would suggest updating the documentation to indicate that only certain types of Zones are allowed.
I can make sure all types of Zones are supported and submit a PR if that's preferred.
Update: changed FixedOffset => FixedOffsetZone
The text was updated successfully, but these errors were encountered: