-
-
Notifications
You must be signed in to change notification settings - Fork 75
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
Add chart-chartjs-financial
dependency resolution fix
#139
base: master
Are you sure you want to change the base?
Add chart-chartjs-financial
dependency resolution fix
#139
Conversation
`chart-chartjs-financial` is unreleased and breaking as a dependency, swap in `temp-chartjs-candlestick` released package. Eventually when official release has happened via an npm package, use that instead. This fixes the dependency issue and tests pass.
@walter When i try and use this I get an error that says
|
I see it called in the dummy app application template, but without actually data. So hard to tell if it's functional. Can you give some code context of how it's being called and some example data, please? |
Oh, one other note. When I was testing this add-on from within another app at first it wasn't picking up the changes as it was based on the same repo/branch as previously. I had to blow away The upshot, if you are testing from an app confirm that the changes have been pulled in by looking in |
@devotox - never my request for context and code, I managed to recreate error in the test dummy app by passing in datasets in |
@walter I was thinking maybe it was different versions of chart.js being used in ember-cli-chart and temp-chartjs-candlestick. I am pretty sure I used the same versions at 3.5.1 but I may be wrong One thing I havent tried that crossed my mind maybe using resolutions in package.json |
@devotox Yeah, it's weird. It looks like they both are using chart.js I wonder if has to do with |
@walter could it be that we are importing the index.esm.js file which does not actually register the controllers Yup just tested it out. What we need to do is
|
Also in
Since we are no longer including any files |
From temp-chartjs-candlestick Co-authored-by: Devonte <[email protected]>
@devotox thanks for getting that working (I think). I now have the candlestick chart rendering without errors, but I don't have test candlestick or OHLC chart data to verify they work with. Can you try them out locally and let me know if they work for you? |
@walter yup tested with |
Closes #127
Supersedes PR #136
Relies on #138
This fixes the
chart-chartjs-financial
import by replacing the unreleased as a packagechart-chartjs-financial
dependency withtemp-chartjs-candlestick
which is simply a released version of the same code.Normally I wouldn't advise using a fork exactly likely this, but currently
ember-cli-chartjs
is broken without fixing this dependency.Thanks @devotox for his work on these two PRs and his patience!