From 8887663f6cb6a96cecc1c88d46720438cbab9db0 Mon Sep 17 00:00:00 2001 From: Dan Vanderkam Date: Wed, 19 Nov 2014 00:52:28 -0500 Subject: [PATCH] Log warnings when using non-existent options --- Makefile | 1 + auto_tests/tests/per_series.js | 4 +- auto_tests/tests/range_selector.js | 1 - auto_tests/tests/simple_drawing.js | 12 +-- auto_tests/tests/update_options.js | 23 +++--- auto_tests/tests/utils_test.js | 6 ++ dygraph-options-reference.js | 13 +++ dygraph-options.js | 67 ++++++++++++++++ dygraph-utils.js | 159 +++++++++++++++++++------------------ dygraph.js | 6 +- 10 files changed, 195 insertions(+), 97 deletions(-) diff --git a/Makefile b/Makefile index 2047c7f..6829ebf 100644 --- a/Makefile +++ b/Makefile @@ -38,6 +38,7 @@ move-combined: generate-combined clean-combined-test: clean @echo restoring combined git checkout dygraph-dev.js + rm dygraph-combined.js.map lint: @./lint.sh diff --git a/auto_tests/tests/per_series.js b/auto_tests/tests/per_series.js index 7289e0c..8a7e081 100644 --- a/auto_tests/tests/per_series.js +++ b/auto_tests/tests/per_series.js @@ -20,7 +20,9 @@ perSeriesTestCase.prototype.testPerSeriesFill = function() { drawYGrid: false, drawXAxis: false, drawYAxis: false, - Y: { fillGraph: true }, + series: { + Y: { fillGraph: true }, + }, colors: [ '#FF0000', '#0000FF' ], fillAlpha: 0.15 }; diff --git a/auto_tests/tests/range_selector.js b/auto_tests/tests/range_selector.js index aed2b3e..c2b829e 100644 --- a/auto_tests/tests/range_selector.js +++ b/auto_tests/tests/range_selector.js @@ -115,7 +115,6 @@ RangeSelectorTestCase.prototype.testRangeSelectorOptions = function() { showRangeSelector: true, rangeSelectorHeight: 30, rangeSelectorPlotFillColor: 'lightyellow', - rangeSelectorPlotStyleColor: 'yellow', labels: ['X', 'Y'] }; var data = [ diff --git a/auto_tests/tests/simple_drawing.js b/auto_tests/tests/simple_drawing.js index 27e9a1a..051fd74 100644 --- a/auto_tests/tests/simple_drawing.js +++ b/auto_tests/tests/simple_drawing.js @@ -96,12 +96,14 @@ SimpleDrawingTestCase.prototype.testDrawWithAxis = function() { */ SimpleDrawingTestCase.prototype.testDrawSimpleDash = function() { var opts = { - drawXGrid: false, - drawYGrid: false, - drawXAxis: false, - drawYAxis: false, + drawXGrid: false, + drawYGrid: false, + drawXAxis: false, + drawYAxis: false, + series: { 'Y1': {strokePattern: [25, 7, 7, 7]}, - colors: ['#ff0000'] + }, + colors: ['#ff0000'] }; var graph = document.getElementById("graph"); diff --git a/auto_tests/tests/update_options.js b/auto_tests/tests/update_options.js index ac28e99..faf8f25 100644 --- a/auto_tests/tests/update_options.js +++ b/auto_tests/tests/update_options.js @@ -67,7 +67,7 @@ UpdateOptionsTestCase.prototype.testStrokeSingleSeries = function() { var optionsForY1 = { }; optionsForY1['strokeWidth'] = 3; - updatedOptions['Y1'] = optionsForY1; + updatedOptions['series'] = {'Y1': optionsForY1}; // These options will allow us to jump to renderGraph_() // drawGraph_() will be skipped. @@ -80,17 +80,16 @@ UpdateOptionsTestCase.prototype.testStrokeSingleSeries = function() { UpdateOptionsTestCase.prototype.testSingleSeriesRequiresNewPoints = function() { var graphDiv = document.getElementById("graph"); var graph = new Dygraph(graphDiv, this.data, this.opts); - var updatedOptions = { }; - var optionsForY1 = { }; - var optionsForY2 = { }; - - // This will not require new points. - optionsForY1['strokeWidth'] = 2; - updatedOptions['Y1'] = optionsForY1; - - // This will require new points. - optionsForY2['stepPlot'] = true; - updatedOptions['Y2'] = optionsForY2; + var updatedOptions = { + series: { + Y1: { + strokeWidth: 2 + }, + Y2: { + stepPlot: true + } + } + }; // These options will not allow us to jump to renderGraph_() // drawGraph_() must be called diff --git a/auto_tests/tests/utils_test.js b/auto_tests/tests/utils_test.js index 0aee0d5..5bd7f8d 100644 --- a/auto_tests/tests/utils_test.js +++ b/auto_tests/tests/utils_test.js @@ -177,6 +177,12 @@ UtilsTestCase.prototype.testToRGB = function() { assertEquals({r: 255, g: 0, b: 0}, Dygraph.toRGB_('red')); }; +UtilsTestCase.prototype.testIsPixelChangingOptionList = function() { + var isPx = Dygraph.isPixelChangingOptionList; + assertTrue(isPx([], { axes: { y: { digitsAfterDecimal: 3 }}})); + assertFalse(isPx([], { axes: { y: { axisLineColor: 'blue' }}})); +}; + /* UtilsTestCase.prototype.testDateSet = function() { var base = new Date(1383455100000); diff --git a/dygraph-options-reference.js b/dygraph-options-reference.js index 8520c10..ddd490f 100644 --- a/dygraph-options-reference.js +++ b/dygraph-options-reference.js @@ -425,6 +425,13 @@ Dygraph.OPTIONS_REFERENCE = // "type": "red, blue", "description": "The color of the gridlines. This may be set on a per-axis basis to define each axis' grid separately." }, + "gridLinePattern": { + "default": "null", + "labels": ["Grid"], + "type": "array", + "example": "[10, 2, 5, 2]", + "description": "A custom pattern array where the even index is a draw and odd is a space in pixels. If null then it draws a solid line. The array should have a even length as any odd lengthed array could be expressed as a smaller even length array. This is used to create dashed gridlines." + }, "visibility": { "default": "[true, true, ...]", "labels": ["Data Line display"], @@ -817,6 +824,12 @@ Dygraph.OPTIONS_REFERENCE = // "type": "array or function", "description": "A function (or array of functions) which plot each data series on the chart. TODO(danvk): more details! May be set per-series." }, + "axes": { + "default": "null", + "labels": ["Configuration"], + "type": "Object", + "description": "Defines per-axis options. Valid keys are 'x', 'y' and 'y2'. Only some options may be set on a per-axis basis. If an option may be set in this way, it will be noted on this page. See also documentation on per-series and per-axis options." + }, "series": { "default": "null", "labels": ["Series"], diff --git a/dygraph-options.js b/dygraph-options.js index b0fad61..2fbf656 100644 --- a/dygraph-options.js +++ b/dygraph-options.js @@ -12,6 +12,12 @@ var DygraphOptions = (function() { +// For "production" code, this gets set to false by uglifyjs. +// Need to define it outside of "use strict", hence the nested IIFEs. +if (typeof(DEBUG) === 'undefined') DEBUG=true; + +return (function() { + /*jshint sub:true */ /*global Dygraph:false */ "use strict"; @@ -217,6 +223,8 @@ DygraphOptions.prototype.reparseSeries = function() { Dygraph.update(this.yAxes_[1].options, axis_opts["y2"] || {}); } Dygraph.update(this.xAxis_.options, axis_opts["x"] || {}); + + if (DEBUG) this.validateOptions_(); }; /** @@ -372,6 +380,65 @@ DygraphOptions.prototype.seriesNames = function() { return this.labels_; }; +if (DEBUG) { + +/** + * Validate all options. + * This requires Dygraph.OPTIONS_REFERENCE, which is only available in debug builds. + * @private + */ +DygraphOptions.prototype.validateOptions_ = function() { + if (typeof Dygraph.OPTIONS_REFERENCE === 'undefined') { + throw 'Called validateOptions_ in prod build.'; + } + + var that = this; + var validateOption = function(optionName) { + if (!Dygraph.OPTIONS_REFERENCE[optionName]) { + that.warnInvalidOption_(optionName); + } + }; + + var optionsDicts = [this.xAxis_.options, + this.yAxes_[0].options, + this.yAxes_[1] && this.yAxes_[1].options, + this.global_, + this.user_, + this.highlightSeries_]; + for (var i = 0; i < optionsDicts.length; i++) { + var dict = optionsDicts[i]; + if (!dict) continue; + for (var optionName in dict) { + if (dict.hasOwnProperty(optionName)) { + validateOption(optionName); + } + } + } +}; + +var WARNINGS = {}; // Only show any particular warning once. + +/** + * Logs a warning about invalid options. + * TODO: make this throw for testing + * @private + */ +DygraphOptions.prototype.warnInvalidOption_ = function(optionName) { + if (!WARNINGS[optionName]) { + var isSeries = (this.labels_.indexOf(optionName) >= 0); + if (isSeries) { + console.warn('Use new-style per-series options (saw ' + optionName + ' as top-level options key). See http://bit.ly/1tceaJs'); + } else { + console.warn('Unknown option ' + optionName + ' (full list of options at dygraphs.com/options.html'); + // throw "invalid option " + optionName; + } + WARNINGS[optionName] = true; + } +}; + +} + return DygraphOptions; })(); +})(); diff --git a/dygraph-utils.js b/dygraph-utils.js index 73777b9..f4bb2ff 100644 --- a/dygraph-utils.js +++ b/dygraph-utils.js @@ -11,7 +11,8 @@ * search) and generic DOM-manipulation functions. */ -/*jshint globalstrict: true */ +(function() { + /*global Dygraph:false, G_vmlCanvasManager:false, Node:false */ "use strict"; @@ -894,72 +895,64 @@ Dygraph.repeatAndCleanup = function(repeatFn, maxFrames, framePeriodInMillis, })(); }; +// A whitelist of options that do not change pixel positions. +var pixelSafeOptions = { + 'annotationClickHandler': true, + 'annotationDblClickHandler': true, + 'annotationMouseOutHandler': true, + 'annotationMouseOverHandler': true, + 'axisLabelColor': true, + 'axisLineColor': true, + 'axisLineWidth': true, + 'clickCallback': true, + 'drawCallback': true, + 'drawHighlightPointCallback': true, + 'drawPoints': true, + 'drawPointCallback': true, + 'drawXGrid': true, + 'drawYGrid': true, + 'fillAlpha': true, + 'gridLineColor': true, + 'gridLineWidth': true, + 'hideOverlayOnMouseOut': true, + 'highlightCallback': true, + 'highlightCircleSize': true, + 'interactionModel': true, + 'isZoomedIgnoreProgrammaticZoom': true, + 'labelsDiv': true, + 'labelsDivStyles': true, + 'labelsDivWidth': true, + 'labelsKMB': true, + 'labelsKMG2': true, + 'labelsSeparateLines': true, + 'labelsShowZeroValues': true, + 'legend': true, + 'panEdgeFraction': true, + 'pixelsPerYLabel': true, + 'pointClickCallback': true, + 'pointSize': true, + 'rangeSelectorPlotFillColor': true, + 'rangeSelectorPlotStrokeColor': true, + 'showLabelsOnHighlight': true, + 'showRoller': true, + 'strokeWidth': true, + 'underlayCallback': true, + 'unhighlightCallback': true, + 'zoomCallback': true +}; + /** * This function will scan the option list and determine if they * require us to recalculate the pixel positions of each point. + * TODO: move this into dygraph-options.js * @param {!Array.} labels a list of options to check. * @param {!Object} attrs * @return {boolean} true if the graph needs new points else false. * @private */ Dygraph.isPixelChangingOptionList = function(labels, attrs) { - // A whitelist of options that do not change pixel positions. - var pixelSafeOptions = { - 'annotationClickHandler': true, - 'annotationDblClickHandler': true, - 'annotationMouseOutHandler': true, - 'annotationMouseOverHandler': true, - 'axisLabelColor': true, - 'axisLineColor': true, - 'axisLineWidth': true, - 'clickCallback': true, - 'digitsAfterDecimal': true, - 'drawCallback': true, - 'drawHighlightPointCallback': true, - 'drawPoints': true, - 'drawPointCallback': true, - 'drawXGrid': true, - 'drawYGrid': true, - 'fillAlpha': true, - 'gridLineColor': true, - 'gridLineWidth': true, - 'hideOverlayOnMouseOut': true, - 'highlightCallback': true, - 'highlightCircleSize': true, - 'interactionModel': true, - 'isZoomedIgnoreProgrammaticZoom': true, - 'labelsDiv': true, - 'labelsDivStyles': true, - 'labelsDivWidth': true, - 'labelsKMB': true, - 'labelsKMG2': true, - 'labelsSeparateLines': true, - 'labelsShowZeroValues': true, - 'legend': true, - 'maxNumberWidth': true, - 'panEdgeFraction': true, - 'pixelsPerYLabel': true, - 'pointClickCallback': true, - 'pointSize': true, - 'rangeSelectorPlotFillColor': true, - 'rangeSelectorPlotStrokeColor': true, - 'showLabelsOnHighlight': true, - 'showRoller': true, - 'sigFigs': true, - 'strokeWidth': true, - 'underlayCallback': true, - 'unhighlightCallback': true, - 'xAxisLabelFormatter': true, - 'xTicker': true, - 'xValueFormatter': true, - 'yAxisLabelFormatter': true, - 'yValueFormatter': true, - 'zoomCallback': true - }; - // Assume that we do not require new points. // This will change to true if we actually do need new points. - var requiresNewPoints = false; // Create a dictionary of series names for faster lookup. // If there are no labels, then the dictionary stays empty. @@ -970,34 +963,44 @@ Dygraph.isPixelChangingOptionList = function(labels, attrs) { } } + // Scan through a flat (i.e. non-nested) object of options. + // Returns true/false depending on whether new points are needed. + var scanFlatOptions = function(options) { + for (var property in options) { + if (options.hasOwnProperty(property) && + !pixelSafeOptions[property]) { + return true; + } + } + return false; + }; + // Iterate through the list of updated options. for (var property in attrs) { - // Break early if we already know we need new points from a previous option. - if (requiresNewPoints) { - break; - } - if (attrs.hasOwnProperty(property)) { - // Find out of this field is actually a series specific options list. - if (seriesNamesDictionary[property]) { - // This property value is a list of options for this series. - // If any of these sub properties are not pixel safe, set the flag. - for (var subProperty in attrs[property]) { - // Break early if we already know we need new points from a previous option. - if (requiresNewPoints) { - break; - } - if (attrs[property].hasOwnProperty(subProperty) && !pixelSafeOptions[subProperty]) { - requiresNewPoints = true; - } + if (!attrs.hasOwnProperty(property)) continue; + + // Find out of this field is actually a series specific options list. + if (property == 'highlightSeriesOpts' || + (seriesNamesDictionary[property] && !attrs.series)) { + // This property value is a list of options for this series. + if (scanFlatOptions(attrs[property])) return true; + } else if (property == 'series' || property == 'axes') { + // This is twice-nested options list. + var perSeries = attrs[property]; + for (var series in perSeries) { + if (perSeries.hasOwnProperty(series) && + scanFlatOptions(perSeries[series])) { + return true; } - // If this was not a series specific option list, check if its a pixel changing property. - } else if (!pixelSafeOptions[property]) { - requiresNewPoints = true; } + } else { + // If this was not a series specific option list, check if it's a pixel + // changing property. + if (!pixelSafeOptions[property]) return true; } } - return requiresNewPoints; + return false; }; Dygraph.Circles = { @@ -1207,3 +1210,5 @@ Dygraph.parseFloat_ = function(x, opt_line_no, opt_line) { return null; }; + +})(); diff --git a/dygraph.js b/dygraph.js index 7dbf29f..49ecc3e 100644 --- a/dygraph.js +++ b/dygraph.js @@ -46,6 +46,7 @@ // For "production" code, this gets set to false by uglifyjs. if (typeof(DEBUG) === 'undefined') DEBUG=true; +var Dygraph = (function() { /*jshint globalstrict: true */ /*global DygraphLayout:false, DygraphCanvasRenderer:false, DygraphOptions:false, G_vmlCanvasManager:false,ActiveXObject:false */ "use strict"; @@ -334,7 +335,6 @@ Dygraph.DEFAULT_ATTRS = { axisLineWidth: 0.3, gridLineWidth: 0.3, axisLabelColor: "black", - axisLabelFont: "Arial", // TODO(danvk): is this implemented? axisLabelWidth: 50, drawYGrid: true, drawXGrid: true, @@ -3794,3 +3794,7 @@ Dygraph.addAnnotationRule = function() { console.warn("Unable to add default annotation CSS rule; display may be off."); }; + +return Dygraph; + +})(); -- 2.7.4