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

Fix beforeDestory when renderChart was not called #139

Merged

Conversation

ZhangYiJiang
Copy link
Contributor

Fix

If renderChart() was somehow not called before the component was destroyed, the component will raise an exception because this._chart is undefined. This patch ensures that this._chart.destroy() will only be called if this._chart has actually been initialized.

  • All tests passed

@codecov
Copy link

codecov bot commented Jul 4, 2017

Codecov Report

Merging #139 into develop will increase coverage by 0.62%.
The diff coverage is 100%.

@@             Coverage Diff             @@
##           develop     #139      +/-   ##
===========================================
+ Coverage    92.23%   92.85%   +0.62%     
===========================================
  Files           11       11              
  Lines          103      112       +9     
===========================================
+ Hits            95      104       +9     
  Misses           8        8
Impacted Files Coverage Δ
src/BaseCharts/Scatter.js 100% <100%> (ø) ⬆️
src/BaseCharts/HorizontalBar.js 90.9% <100%> (+0.9%) ⬆️
src/BaseCharts/Pie.js 90.9% <100%> (+0.9%) ⬆️
src/BaseCharts/Line.js 90.9% <100%> (+0.9%) ⬆️
src/BaseCharts/PolarArea.js 90.9% <100%> (+0.9%) ⬆️
src/BaseCharts/Bar.js 90.9% <100%> (+0.9%) ⬆️
src/BaseCharts/Bubble.js 90.9% <100%> (+0.9%) ⬆️
src/BaseCharts/Radar.js 90.9% <100%> (+0.9%) ⬆️
src/BaseCharts/Doughnut.js 90.9% <100%> (+0.9%) ⬆️

@apertureless
Copy link
Owner

Awesome! Thanks! 🙏

@apertureless apertureless merged commit 677c53d into apertureless:develop Jul 4, 2017
@apertureless apertureless added this to the 2.7.0 milestone Jul 4, 2017
@denislapi
Copy link

denislapi commented Jul 20, 2017

@ZhangYiJiang @apertureless Hey guys! Is this updated in version for VueJS 1 ?
I get this error Uncaught TypeError: Cannot read property 'destroy' of undefined when I change my route (from component with chart componenets to some other)

This is the line where I get error:

beforeDestroy: function() {
    this._chart.destroy()
}     

Versions:

"vue": "^1.0.7",
"vue-chartjs": "^1.1.5",

@apertureless
Copy link
Owner

Hey!
No this got only applied to the vue 2 version.

Can you create an issue? I will then implement it in the vue 1.x version, too.

@denislapi
Copy link

@apertureless Of course! Give me a minut

@ZhangYiJiang ZhangYiJiang deleted the patch-beforeDestroy branch August 11, 2017 01:38
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

3 participants