-
-
Notifications
You must be signed in to change notification settings - Fork 836
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(legacy-charts): tune chartoptions type check #821
Conversation
size-limit report 📦
|
@@ -74,6 +74,7 @@ export function generateChart(chartId, chartType, chartController) { | |||
computed: { | |||
hasAnnotationPlugin() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hasAnnotationPlugin() { | |
hasAnnotationPlugin() { | |
return this.chartOptions?.plugins?.hasOwnProperty(ANNOTATION_PLUGIN_KEY) |
maybe better like that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dangreen Do not access Object.prototype method 'hasOwnProperty' from target object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reflect.hasOwnProperty(this.chartOptions?.plugins, ANNOTATION_PLUGIN_KEY)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dangreen has the same error((
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const plugins = this.chartOptions?.plugins
return plugins && Reflect.has(plugins, ANNOTATION_PLUGIN_KEY)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dangreen has Vue error in build version[Vue warn]: Property or method "chartOptions" is not defined on the instance but referenced during render. Make sure that this property is reactive, either in the data option, or for class-based components, by initializing the property. See: https://vuejs.org/v2/guide/reactivity.html#Declaring-Reactive-Properties.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@thabarbados ok. please find short variant of current code, cause current variant is too long
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dangreen fixed
a88e6c2
to
e766051
Compare
e766051
to
855f0ba
Compare
legacy/src/Charts.js
Outdated
const pluginSettings = | ||
this.chartOptions?.plugins?.[ANNOTATION_PLUGIN_KEY] | ||
|
||
return pluginSettings !== undefined |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return pluginSettings !== undefined | |
return typeof pluginSettings !== 'undefined' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dangreen Used this option. I don't like Boolean)
legacy/src/Charts.js
Outdated
const pluginSettings = | ||
this.chartOptions?.plugins?.[ANNOTATION_PLUGIN_KEY] | ||
|
||
return pluginSettings !== undefined |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or just
const pluginSettings = | |
this.chartOptions?.plugins?.[ANNOTATION_PLUGIN_KEY] | |
return pluginSettings !== undefined | |
return Boolen(this.chartOptions?.plugins?.[ANNOTATION_PLUGIN_KEY]) |
855f0ba
to
22a5f4a
Compare
Fix or Enhancement?
tune legacy charts options type check
Environment