fix: inflate functions passed to __callXXXFunction methods (#5080) (#…

…5086)

Co-authored-by: Diego Cardoso <[email protected]>

packages/charts/src/helpers.js

@@ -0,0 +1,24 @@
+export function inflateFunctions(config) {
+  if (
+    // Check if param is a primitive/null/undefined value
+    !(config instanceof Object) ||
+    // Check if param is a plain object (not an array or HC object)
+    config.constructor !== Object
+  ) {
+    return;
+  }
+  Object.entries(config).forEach(([attr, targetProperty]) => {
+    if (attr.startsWith('_fn_') && (typeof targetProperty === 'string' || targetProperty instanceof String)) {
+      try {
+        // eslint-disable-next-line no-eval
+        config[attr.substr(4)] = eval(`(${targetProperty})`);
+      } catch (e) {
+        // eslint-disable-next-line no-eval
+        config[attr.substr(4)] = eval(`(function(){${targetProperty}})`);
+      }
+      delete config[attr];
+    } else if (targetProperty instanceof Object) {
+      inflateFunctions(targetProperty);
+    }
+  });
+}

packages/charts/src/vaadin-chart.js

@@ -27,6 +27,7 @@ import Highcharts from 'highcharts/es-modules/masters/highstock.src.js';
 import { ElementMixin } from '@vaadin/component-base/src/element-mixin.js';
 import { ResizeMixin } from '@vaadin/component-base/src/resize-mixin.js';
 import { ThemableMixin } from '@vaadin/vaadin-themable-mixin/vaadin-themable-mixin.js';
+import { inflateFunctions } from './helpers.js';
 import { ChartSeries } from './vaadin-chart-series.js';
 
 /** @private */
@@ -272,6 +273,7 @@ class Chart extends ResizeMixin(ElementMixin(ThemableMixin(PolymerElement))) {
   static __callHighchartsFunction(functionName, redrawCharts, ...args) {
     const functionToCall = Highcharts[functionName];
     if (functionToCall && typeof functionToCall === 'function') {
+      args.forEach((arg) => inflateFunctions(arg));
       functionToCall.apply(this.configuration, args);
       if (redrawCharts) {
         Highcharts.charts.forEach((c) => c.redraw());
@@ -1148,7 +1150,7 @@ class Chart extends ResizeMixin(ElementMixin(ThemableMixin(PolymerElement))) {
     }
 
     const configCopy = deepMerge({}, jsonConfiguration);
-    this.__inflateFunctions(configCopy);
+    inflateFunctions(configCopy);
     this._jsonConfigurationBuffer = this.__makeConfigurationBuffer(this._jsonConfigurationBuffer, configCopy);
 
     beforeNextRender(this, () => {
@@ -1214,24 +1216,6 @@ class Chart extends ResizeMixin(ElementMixin(ThemableMixin(PolymerElement))) {
     delete configuration[entry];
   }
 
-  /** @private */
-  __inflateFunctions(jsonConfiguration) {
-    Object.entries(jsonConfiguration).forEach(([attr, targetProperty]) => {
-      if (attr.startsWith('_fn_') && (typeof targetProperty === 'string' || targetProperty instanceof String)) {
-        try {
-          // eslint-disable-next-line no-eval
-          jsonConfiguration[attr.substr(4)] = eval(`(${targetProperty})`);
-        } catch (e) {
-          // eslint-disable-next-line no-eval
-          jsonConfiguration[attr.substr(4)] = eval(`(function(){${targetProperty}})`);
-        }
-        delete jsonConfiguration[attr];
-      } else if (targetProperty instanceof Object) {
-        this.__inflateFunctions(targetProperty);
-      }
-    });
-  }
-
   /** @private */
   __initEventsListeners(configuration) {
     this.__initChartEventsListeners(configuration);
@@ -1684,6 +1668,7 @@ class Chart extends ResizeMixin(ElementMixin(ThemableMixin(PolymerElement))) {
     if (this.configuration) {
       const functionToCall = this.configuration[functionName];
       if (functionToCall && typeof functionToCall === 'function') {
+        args.forEach((arg) => inflateFunctions(arg));
         functionToCall.apply(this.configuration, args);
       }
     }
@@ -1695,6 +1680,7 @@ class Chart extends ResizeMixin(ElementMixin(ThemableMixin(PolymerElement))) {
       const series = this.configuration.series[seriesIndex];
       const functionToCall = series[functionName];
       if (functionToCall && typeof functionToCall === 'function') {
+        args.forEach((arg) => inflateFunctions(arg));
         functionToCall.apply(series, args);
       }
     }
@@ -1731,6 +1717,7 @@ class Chart extends ResizeMixin(ElementMixin(ThemableMixin(PolymerElement))) {
         const axis = axes[axisIndex];
         const functionToCall = axis[functionName];
         if (functionToCall && typeof functionToCall === 'function') {
+          args.forEach((arg) => inflateFunctions(arg));
           functionToCall.apply(axis, args);
         }
       }

packages/charts/test/private-api.test.js

@@ -1,38 +1,41 @@
 import { expect } from '@esm-bundle/chai';
 import { fixtureSync, oneEvent } from '@vaadin/testing-helpers';
 import '../vaadin-chart.js';
+import { inflateFunctions } from '../src/helpers.js';
 
 describe('vaadin-chart private API', () => {
   describe('__inflateFunctions', () => {
-    let chart;
-
-    beforeEach(() => {
-      chart = fixtureSync('<vaadin-chart></vaadin-chart>');
-    });
-
     it('should inflate whole function strings', () => {
       // eslint-disable-next-line camelcase
       const config = { tooltip: { _fn_formatter: 'function() {return "awesome chart"}' } };
-      chart.__inflateFunctions(config);
+      inflateFunctions(config);
       expect(config.tooltip.formatter).to.be.a('function');
       expect(config.tooltip).to.not.have.property('_fn_formatter');
     });
 
     it('should inflate function body strings', () => {
       // eslint-disable-next-line camelcase
       const config = { tooltip: { _fn_formatter: 'return "awesome chart"' } };
-      chart.__inflateFunctions(config);
+      inflateFunctions(config);
       expect(config.tooltip.formatter).to.be.a('function');
       expect(config.tooltip).to.not.have.property('_fn_formatter');
     });
 
     it('should inflate empty function strings', () => {
       // eslint-disable-next-line camelcase
       const config = { tooltip: { _fn_formatter: '' } };
-      chart.__inflateFunctions(config);
+      inflateFunctions(config);
       expect(config.tooltip.formatter).to.be.a('function');
       expect(config.tooltip).to.not.have.property('_fn_formatter');
     });
+
+    it('should not try to inflate if a non-object value is passed', () => {
+      // Check for no errors being thrown
+      inflateFunctions(null);
+      inflateFunctions(undefined);
+      inflateFunctions(1);
+      inflateFunctions([1]);
+    });
   });
 
   describe('__callChartFunction', () => {
@@ -53,6 +56,14 @@ describe('vaadin-chart private API', () => {
       chart.__callChartFunction('addSeries', { data: [30, 1, 3, 5, 2] });
       expect(series).to.have.lengthOf(2);
     });
+
+    it('should inflate functions passed in config', () => {
+      // eslint-disable-next-line camelcase
+      const seriesConfig = { data: [30, 1, 3, 5, 2], dataLabels: { enabled: true, _fn_formatter: 'return `val`;' } };
+      chart.__callChartFunction('addSeries', seriesConfig);
+      const { dataLabels } = chart.configuration.series[1].userOptions;
+      expect(dataLabels.formatter).to.be.a('function');
+    });
   });
 
   describe('__callSeriesFunction', () => {
@@ -74,6 +85,13 @@ describe('vaadin-chart private API', () => {
       expect(data).to.have.lengthOf(6);
       expect(data[5].y).to.equal(30);
     });
+
+    it('should inflate functions passed as string', () => {
+      // eslint-disable-next-line camelcase
+      chart.__callSeriesFunction('update', 0, { dataLabels: { _fn_formatter: 'return `foo`;' } });
+      const { dataLabels } = chart.configuration.series[0].userOptions;
+      expect(dataLabels.formatter).to.be.a('function');
+    });
   });
 
   describe('__callAxisFunction', () => {
@@ -95,6 +113,13 @@ describe('vaadin-chart private API', () => {
       chart.__callAxisFunction('setExtremes', 1, 0, 5);
       expect(yAxis[0].min).to.equal(5);
     });
+
+    it('should inflate functions passed as string', () => {
+      // eslint-disable-next-line camelcase
+      chart.__callAxisFunction('update', 1, 0, { dataLabels: { _fn_formatter: 'return `foo`;' } });
+      const { dataLabels } = chart.configuration.yAxis[0].userOptions;
+      expect(dataLabels.formatter).to.be.a('function');
+    });
   });
 
   describe('__callPointFunction', () => {
@@ -127,6 +152,12 @@ describe('vaadin-chart private API', () => {
     }
 
     beforeEach(async () => {
+      customElements.get('vaadin-chart').__callHighchartsFunction('setOptions', false, {
+        legend: {
+          // eslint-disable-next-line camelcase
+          _fn_labelFormatter: 'return `value`',
+        },
+      });
       chart = fixtureSync(`
         <vaadin-chart additional-options='{ "xAxis": { "type": "datetime", "labels": { "format": "{value:%a}" } } }'>
           <vaadin-chart-series values="[5, 6, 4, 7, 6, 2, 1]" additional-options='{ "pointIntervalUnit": "day" }'>
@@ -147,5 +178,10 @@ describe('vaadin-chart private API', () => {
       });
       expect(getLabels()).to.be.deep.equal(['to', 'pe', 'la', 'su', 'ma', 'ti', 'ke']);
     });
+
+    it('should inflate functions passed as string', () => {
+      const legend = chart.$.chart.querySelector('.highcharts-legend-item > text').textContent;
+      expect(legend).to.be.equal('value');
+    });
   });
 });