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

[Best Practices❓] Usage of Private Data #182

Closed
alexsasharegan opened this issue Aug 24, 2017 · 1 comment
Closed

[Best Practices❓] Usage of Private Data #182

alexsasharegan opened this issue Aug 24, 2017 · 1 comment

Comments

@alexsasharegan
Copy link

Expected Behavior

Rather than dynamically setting a private data property _chart on the chart instance after creation, use standard data method to define property.

Actual Behavior

Currently the Chartjs _chart is set at the time renderChart is called. Ideally, this would be stubbed out in the data method. This will keep Vue from ever overwriting _chart, however unlikely. The gotcha is that Vue's data proxy goes away for props beginning with $ & _. They must be referenced instead like so: this.$data._chart.

Of course, using the private notation "the right way" for Vue means Vue will now reactively watch those props. This may not be the desired effect. In any case, this is just a question. It is not currently affecting the library in any negative way that I can see.

Environment

  • vue.js version: 2.4.2
  • vue-chart.js version: 2.8.2
  • npm version: 5.3.0
@apertureless
Copy link
Owner

Hm thats interesting.

I will check if it would have any side effects. In the beginning I did not think about exposing the chart.js instance, however it was proven to be a good idea, as people could modify a lot of stuff this way.

It way actually only introduced to properly cleanup the chartjs object and destroy the instance :D

But this seems to be a good idea to make it more future proof. ✌️

@apertureless apertureless added this to the 3.0 milestone Sep 16, 2017
apertureless added a commit that referenced this issue Oct 14, 2017
Implements #182. The private chart instance is now in the vue.js data model. And can be accessed over `this.$data._chart`
Updated unit tests
apertureless added a commit that referenced this issue Oct 14, 2017
* Remove Vue dependency and change extends

Signed-off-by: Jakub Juszczak <netghost03@gmail.com>

* 💎 Release new version 3.0.0-rc0

* ⬆️ Update examples

* 📝 Update README.md

* ⬆️ Update examples

* ⬆️ Update englishd docs

* ⬆️ Update transalted docs with current code examples

* 🔥 Remove dist files from gitignore

* ⬆️ Update dependencies vue and chartjs

* Change private data

Implements #182. The private chart instance is now in the vue.js data model. And can be accessed over `this.$data._chart`
Updated unit tests

* 📝 Update docs with private data

* ✨ Add codeclimate ignore

* ⬆️ Update codeclimate

* ⬆️ Update codeclimate

* ⬆️ Update codeclimate

Add build and config folders to ignore
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants