From f8945ea2f5fa88e50ae5671690c2a3c6814a7e22 Mon Sep 17 00:00:00 2001 From: David Winegar Date: Wed, 15 Jan 2020 11:56:27 -0800 Subject: [PATCH 1/7] Allow switching platforms Move the Chart.platform to Chart.platform.current instead, and add ways to see available platforms and set the current platform. This is necessary for adding tests that use the "basic" platform. --- samples/advanced/content-security-policy.js | 2 +- src/core/core.controller.js | 10 +- src/index.js | 2 +- src/platforms/platform.basic.js | 3 + src/platforms/platform.dom.js | 2 + src/platforms/platform.js | 126 +++++++++++++------- test/specs/global.namespace.tests.js | 1 + test/specs/platform.tests.js | 17 +++ 8 files changed, 112 insertions(+), 51 deletions(-) create mode 100644 test/specs/platform.tests.js diff --git a/samples/advanced/content-security-policy.js b/samples/advanced/content-security-policy.js index a185332f74b..d33c103309f 100644 --- a/samples/advanced/content-security-policy.js +++ b/samples/advanced/content-security-policy.js @@ -1,7 +1,7 @@ var utils = Samples.utils; // CSP: disable automatic style injection -Chart.platform.disableCSSInjection = true; +Chart.platform.current.disableCSSInjection = true; utils.srand(110); diff --git a/src/core/core.controller.js b/src/core/core.controller.js index 81aaa9ceba1..9f06f0c0ad5 100644 --- a/src/core/core.controller.js +++ b/src/core/core.controller.js @@ -154,7 +154,7 @@ class Chart { config = initConfig(config); - const context = platform.acquireContext(item, config); + const context = platform.current.acquireContext(item, config); const canvas = context && context.canvas; const height = canvas && canvas.height; const width = canvas && canvas.width; @@ -851,7 +851,7 @@ class Chart { if (canvas) { me.unbindEvents(); helpers.canvas.clear(me); - platform.releaseContext(me.ctx); + platform.current.releaseContext(me.ctx); me.canvas = null; me.ctx = null; } @@ -880,7 +880,7 @@ class Chart { }; helpers.each(me.options.events, function(type) { - platform.addEventListener(me, type, listener); + platform.current.addEventListener(me, type, listener); listeners[type] = listener; }); @@ -891,7 +891,7 @@ class Chart { me.resize(); }; - platform.addEventListener(me, 'resize', listener); + platform.current.addEventListener(me, 'resize', listener); listeners.resize = listener; } } @@ -908,7 +908,7 @@ class Chart { delete me._listeners; helpers.each(listeners, function(listener, type) { - platform.removeEventListener(me, type, listener); + platform.current.removeEventListener(me, type, listener); }); } diff --git a/src/index.js b/src/index.js index 8e8ef17e1ba..430c5df955c 100644 --- a/src/index.js +++ b/src/index.js @@ -59,7 +59,7 @@ for (var k in plugins) { } } -Chart.platform.initialize(); +Chart.platform.current.initialize(); if (typeof window !== 'undefined') { window.Chart = Chart; diff --git a/src/platforms/platform.basic.js b/src/platforms/platform.basic.js index f510b3575c7..4896aa77a2f 100644 --- a/src/platforms/platform.basic.js +++ b/src/platforms/platform.basic.js @@ -3,7 +3,10 @@ * @see https://github.com/chartjs/Chart.js/pull/4591#issuecomment-319575939 */ +'use strict'; + module.exports = { + type: 'basic', acquireContext: function(item) { if (item && item.canvas) { // Support for any object associated to a canvas (including a context2d) diff --git a/src/platforms/platform.dom.js b/src/platforms/platform.dom.js index 1579e0e15dc..22bf244bd58 100644 --- a/src/platforms/platform.dom.js +++ b/src/platforms/platform.dom.js @@ -313,6 +313,8 @@ function injectCSS(rootNode, css) { } export default { + type: 'dom', + /** * When `true`, prevents the automatic injection of the stylesheet required to * correctly detect when the chart is added to the DOM and then resized. This diff --git a/src/platforms/platform.js b/src/platforms/platform.js index f527c69cd72..b46df388e91 100644 --- a/src/platforms/platform.js +++ b/src/platforms/platform.js @@ -4,63 +4,101 @@ import helpers from '../helpers/index'; import basic from './platform.basic'; import dom from './platform.dom'; -// @TODO Make possible to select another platform at build time. -const implementation = dom._enabled ? dom : basic; +function extendPlatform(implementation) { + return helpers.extend({ + /** + * @since 3.0.0 + * @memberof Chart.platform.current + */ + type: undefined, -/** - * @namespace Chart.platform - * @see https://chartjs.gitbooks.io/proposals/content/Platform.html - * @since 2.4.0 - */ -export default helpers.extend({ - /** - * @since 2.7.0 - */ - initialize: function() {}, + /** + * @since 2.7.0 + * @memberof Chart.platform.current + */ + initialize: function() {}, - /** - * Called at chart construction time, returns a context2d instance implementing - * the [W3C Canvas 2D Context API standard]{@link https://www.w3.org/TR/2dcontext/}. - * @param {*} item - The native item from which to acquire context (platform specific) - * @param {object} options - The chart options - * @returns {CanvasRenderingContext2D} context2d instance - */ - acquireContext: function() {}, + /** + * Called at chart construction time, returns a context2d instance implementing + * the [W3C Canvas 2D Context API standard]{@link https://www.w3.org/TR/2dcontext/}. + * @param {*} item - The native item from which to acquire context (platform specific) + * @param {object} options - The chart options + * @returns {CanvasRenderingContext2D} context2d instance + * @memberof Chart.platform.current + */ + acquireContext: function() {}, - /** - * Called at chart destruction time, releases any resources associated to the context - * previously returned by the acquireContext() method. - * @param {CanvasRenderingContext2D} context - The context2d instance - * @returns {boolean} true if the method succeeded, else false - */ - releaseContext: function() {}, + /** + * Called at chart destruction time, releases any resources associated to the context + * previously returned by the acquireContext() method. + * @param {CanvasRenderingContext2D} context - The context2d instance + * @returns {boolean} true if the method succeeded, else false + * @memberof Chart.platform.current + */ + releaseContext: function() {}, + /** + * Registers the specified listener on the given chart. + * @param {Chart} chart - Chart from which to listen for event + * @param {string} type - The ({@link IEvent}) type to listen for + * @param {function} listener - Receives a notification (an object that implements + * the {@link IEvent} interface) when an event of the specified type occurs. + * @memberof Chart.platform.current + */ + addEventListener: function() {}, + + /** + * Removes the specified listener previously registered with addEventListener. + * @param {Chart} chart - Chart from which to remove the listener + * @param {string} type - The ({@link IEvent}) type to remove + * @param {function} listener - The listener function to remove from the event target. + * @memberof Chart.platform.current + */ + removeEventListener: function() {} + + }, implementation); +} + + +var defaultImplementation = dom._enabled ? dom : basic; + +/** +* @namespace Chart.platform +* Allows getting and setting the current platform +* @since 3.0.0 +*/ +var platform = { /** - * Registers the specified listener on the given chart. - * @param {Chart} chart - Chart from which to listen for event - * @param {string} type - The ({@link IEvent}) type to listen for - * @param {function} listener - Receives a notification (an object that implements - * the {@link IEvent} interface) when an event of the specified type occurs. + * @type Array.{IPlatform} + * A list of platforms that can be set */ - addEventListener: function() {}, - + availablePlatforms: [dom, basic], + /** + * @namespace Chart.platform.current + * Changed from Chart.platform to Chart.platform.current in 3.0.0 + * @see https://chartjs.gitbooks.io/proposals/content/Platform.html + * @since 2.4.0 + */ + current: extendPlatform(defaultImplementation), /** - * Removes the specified listener previously registered with addEventListener. - * @param {Chart} chart - Chart from which to remove the listener - * @param {string} type - The ({@link IEvent}) type to remove - * @param {function} listener - The listener function to remove from the event target. + * @param {IPlatform} implementation - the platform implementation to set as the current platform + * Sets the current platform. */ - removeEventListener: function() {} + setPlatform: function(implementation) { + platform.current = extendPlatform(implementation); + }, +}; -}, implementation); +export default platform; /** * @interface IPlatform * Allows abstracting platform dependencies away from the chart - * @borrows Chart.platform.acquireContext as acquireContext - * @borrows Chart.platform.releaseContext as releaseContext - * @borrows Chart.platform.addEventListener as addEventListener - * @borrows Chart.platform.removeEventListener as removeEventListener + * @borrows Chart.platform.current.type as type + * @borrows Chart.platform.current.acquireContext as acquireContext + * @borrows Chart.platform.current.releaseContext as releaseContext + * @borrows Chart.platform.current.addEventListener as addEventListener + * @borrows Chart.platform.current.removeEventListener as removeEventListener */ /** diff --git a/test/specs/global.namespace.tests.js b/test/specs/global.namespace.tests.js index 89f8cad5b60..2577e3cc650 100644 --- a/test/specs/global.namespace.tests.js +++ b/test/specs/global.namespace.tests.js @@ -13,6 +13,7 @@ describe('Chart namespace', function() { expect(Chart.layouts instanceof Object).toBeTruthy(); expect(Chart.plugins instanceof Object).toBeTruthy(); expect(Chart.platform instanceof Object).toBeTruthy(); + expect(Chart.platform.current instanceof Object).toBeTruthy(); expect(Chart.Scale instanceof Object).toBeTruthy(); expect(Chart.scaleService instanceof Object).toBeTruthy(); expect(Chart.Ticks instanceof Object).toBeTruthy(); diff --git a/test/specs/platform.tests.js b/test/specs/platform.tests.js new file mode 100644 index 00000000000..e5026360e23 --- /dev/null +++ b/test/specs/platform.tests.js @@ -0,0 +1,17 @@ +describe('Chart.platform', function() { + it('the default platform is dom', function() { + expect(Chart.platform.current.type).toEqual('dom'); + }); + + it('the platform can be changed to basic', function() { + const basicPlatform = Chart.platform.availablePlatforms.find((p) => p.type === 'basic'); + expect(Chart.platform.current.type).toEqual('dom'); + + Chart.platform.setPlatform(basicPlatform); + expect(Chart.platform.current.type).toEqual('basic'); + + // Reset to dom to clean up the test. + const domPlatform = Chart.platform.availablePlatforms.find((p) => p.type === 'dom'); + Chart.platform.setPlatform(domPlatform); + }); +}); From 533570965ce3c3a708c3268dca766afdd468e941 Mon Sep 17 00:00:00 2001 From: David Winegar Date: Thu, 16 Jan 2020 11:01:45 -0800 Subject: [PATCH 2/7] Switch to a per-chart platform --- docs/getting-started/v3-migration.md | 7 ++ samples/advanced/content-security-policy.js | 7 +- src/core/core.controller.js | 53 +++++++- src/index.js | 6 +- src/platforms/platform.basic.js | 22 ++-- src/platforms/platform.dom.js | 69 +++++------ src/platforms/platform.js | 127 ++++++-------------- src/platforms/platforms.js | 13 ++ test/specs/global.namespace.tests.js | 3 +- test/specs/platform.tests.js | 17 --- 10 files changed, 150 insertions(+), 174 deletions(-) create mode 100644 src/platforms/platforms.js delete mode 100644 test/specs/platform.tests.js diff --git a/docs/getting-started/v3-migration.md b/docs/getting-started/v3-migration.md index d11ccb1f760..9791b5025ba 100644 --- a/docs/getting-started/v3-migration.md +++ b/docs/getting-started/v3-migration.md @@ -206,3 +206,10 @@ Animation system was completely rewritten in Chart.js v3. Each property can now * The second parameter to `drawPoint` is now the full options object, so `style`, `rotation`, and `radius` are no longer passed explicitly +#### Platform + +* `Chart.platform` no longer exists. Every chart instance now has a separate platform instance, `chart.platform`. +* `Chart.platforms` is an object that contains two usable platform classes, `BasicPlatform` and `DomPlatform`. It also contains `Platform`, a class that all platforms must extend from. +* If the canvas passed in is an instance of `OffscreenCanvas`, the `BasicPlatform` is automatically used. +* T override the platform class used in a chart instance, pass `platform: PlatformClass` in the config object. Note that the class should be passed, not an instance of the class. +* To disable CSS injection (necessary in some iframe contexts), `Chart.platform.disableCSSInjection = true` is no longer supported. Pass `disableCSSInjection: true` in the config object instead. diff --git a/samples/advanced/content-security-policy.js b/samples/advanced/content-security-policy.js index d33c103309f..4c4daec2c3f 100644 --- a/samples/advanced/content-security-policy.js +++ b/samples/advanced/content-security-policy.js @@ -1,8 +1,5 @@ var utils = Samples.utils; -// CSP: disable automatic style injection -Chart.platform.current.disableCSSInjection = true; - utils.srand(110); function generateData() { @@ -48,7 +45,9 @@ window.addEventListener('load', function() { return (size / 24) * base; } } - } + }, + // CSP: disable automatic style injection + disableCSSInjection: true } }); }); diff --git a/src/core/core.controller.js b/src/core/core.controller.js index 9f06f0c0ad5..9adbc03356c 100644 --- a/src/core/core.controller.js +++ b/src/core/core.controller.js @@ -6,7 +6,7 @@ import defaults from './core.defaults'; import helpers from '../helpers/index'; import Interaction from './core.interaction'; import layouts from './core.layouts'; -import platform from '../platforms/platform'; +import {BasicPlatform, DomPlatform, Platform} from '../platforms/platforms'; import plugins from './core.plugins'; import scaleService from '../core/core.scaleService'; import Tooltip from './core.tooltip'; @@ -148,13 +148,34 @@ function onAnimationProgress(ctx) { helpers.callback(animationOptions && animationOptions.onProgress, arguments, chart); } +/** + * Chart.js can take a string id of a canvas element, a 2d context, or a canvas element itself. + * Attempt to unwrap the item passed into the chart constructor so that it is a canvas element (if possible). + */ +function unwrapItem(item) { + if (typeof document !== undefined && typeof item === 'string') { + item = document.getElementById(item); + } else if (item.length) { + // Support for array based queries (such as jQuery) + item = item[0]; + } + + if (item && item.canvas) { + // Support for any object associated to a canvas (including a context2d) + item = item.canvas; + } + return item; +} + class Chart { constructor(item, config) { const me = this; config = initConfig(config); + const unwrappedItem = unwrapItem(item); + me.initializePlatform(unwrappedItem, config); - const context = platform.current.acquireContext(item, config); + const context = me.platform.acquireContext(unwrappedItem, config); const canvas = context && context.canvas; const height = canvas && canvas.height; const width = canvas && canvas.width; @@ -226,6 +247,26 @@ class Chart { return me; } + /** + * @private + */ + initializePlatform(item, config) { + const me = this; + + if (config.platform) { + if (!(config.platform instanceof Platform)) { + throw new Error('If config.platform is used, it must be an instance of Chart.platforms.Platform or one of its descendants'); + } + me.platform = new config.platform(config); + } else if (typeof window === 'undefined' || typeof document === 'undefined' || !item) { + me.platform = new BasicPlatform(config); + } else if (window.OffscreenCanvas && item instanceof window.OffscreenCanvas) { + me.platform = new BasicPlatform(config); + } else { + me.platform = new DomPlatform(config); + } + } + clear() { helpers.canvas.clear(this); return this; @@ -851,7 +892,7 @@ class Chart { if (canvas) { me.unbindEvents(); helpers.canvas.clear(me); - platform.current.releaseContext(me.ctx); + me.platform.releaseContext(me.ctx); me.canvas = null; me.ctx = null; } @@ -880,7 +921,7 @@ class Chart { }; helpers.each(me.options.events, function(type) { - platform.current.addEventListener(me, type, listener); + me.platform.addEventListener(me, type, listener); listeners[type] = listener; }); @@ -891,7 +932,7 @@ class Chart { me.resize(); }; - platform.current.addEventListener(me, 'resize', listener); + me.platform.addEventListener(me, 'resize', listener); listeners.resize = listener; } } @@ -908,7 +949,7 @@ class Chart { delete me._listeners; helpers.each(listeners, function(listener, type) { - platform.current.removeEventListener(me, type, listener); + me.platform.removeEventListener(me, type, listener); }); } diff --git a/src/index.js b/src/index.js index 430c5df955c..467a1c339d1 100644 --- a/src/index.js +++ b/src/index.js @@ -15,7 +15,7 @@ import Element from './core/core.element'; import elements from './elements'; import Interaction from './core/core.interaction'; import layouts from './core/core.layouts'; -import platform from './platforms/platform'; +import platforms from './platforms/platforms'; import pluginsCore from './core/core.plugins'; import Scale from './core/core.scale'; import scaleService from './core/core.scaleService'; @@ -34,7 +34,7 @@ Chart.Element = Element; Chart.elements = elements; Chart.Interaction = Interaction; Chart.layouts = layouts; -Chart.platform = platform; +Chart.platforms = platforms; Chart.plugins = pluginsCore; Chart.Scale = Scale; Chart.scaleService = scaleService; @@ -59,8 +59,6 @@ for (var k in plugins) { } } -Chart.platform.current.initialize(); - if (typeof window !== 'undefined') { window.Chart = Chart; } diff --git a/src/platforms/platform.basic.js b/src/platforms/platform.basic.js index 4896aa77a2f..6957b76883d 100644 --- a/src/platforms/platform.basic.js +++ b/src/platforms/platform.basic.js @@ -5,14 +5,18 @@ 'use strict'; -module.exports = { - type: 'basic', - acquireContext: function(item) { - if (item && item.canvas) { - // Support for any object associated to a canvas (including a context2d) - item = item.canvas; - } +import Platform from './platform'; - return item && item.getContext('2d') || null; +/** + * Platform class for charts without access to the DOM or to many element properties + * This platform is used by default for any chart passed an OffscreenCanvas. + * @extends Platform + */ +export default class BasicPlatform extends Platform { + acquireContext(item) { + // To prevent canvas fingerprinting, some add-ons undefine the getContext + // method, for example: https://github.com/kkapsner/CanvasBlocker + // https://github.com/chartjs/Chart.js/issues/2807 + return item && item.getContext && item.getContext('2d') || null; } -}; +} diff --git a/src/platforms/platform.dom.js b/src/platforms/platform.dom.js index 22bf244bd58..d3a08478338 100644 --- a/src/platforms/platform.dom.js +++ b/src/platforms/platform.dom.js @@ -6,6 +6,7 @@ import helpers from '../helpers'; import stylesheet from './platform.dom.css'; +import Platform from './platform'; var EXPANDO_KEY = '$chartjs'; var CSS_PREFIX = 'chartjs-'; @@ -312,31 +313,31 @@ function injectCSS(rootNode, css) { } } -export default { - type: 'dom', - - /** - * When `true`, prevents the automatic injection of the stylesheet required to - * correctly detect when the chart is added to the DOM and then resized. This - * switch has been added to allow external stylesheet (`dist/Chart(.min)?.js`) - * to be manually imported to make this library compatible with any CSP. - * See https://github.com/chartjs/Chart.js/issues/5208 - */ - disableCSSInjection: false, - - /** - * This property holds whether this platform is enabled for the current environment. - * Currently used by platform.js to select the proper implementation. - * @private - */ - _enabled: typeof window !== 'undefined' && typeof document !== 'undefined', +/** + * Platform class for charts that can access the DOM and global window/document properties + * @constructor + * @extends Platform + */ +export default class DomPlatform extends Platform { + constructor(options) { + super(); + + /** + * When `true`, prevents the automatic injection of the stylesheet required to + * correctly detect when the chart is added to the DOM and then resized. This + * switch has been added to allow external stylesheet (`dist/Chart(.min)?.js`) + * to be manually imported to make this library compatible with any CSP. + * See https://github.com/chartjs/Chart.js/issues/5208 + */ + this.disableCSSInjection = options.disableCSSInjection || false; + } /** * Initializes resources that depend on platform options. * @param {HTMLCanvasElement} canvas - The Canvas element. * @private */ - _ensureLoaded: function(canvas) { + _ensureLoaded(canvas) { if (!this.disableCSSInjection) { // If the canvas is in a shadow DOM, then the styles must also be inserted // into the same shadow DOM. @@ -345,21 +346,9 @@ export default { var targetNode = root.host ? root : document.head; injectCSS(targetNode, stylesheet); } - }, - - acquireContext: function(item, config) { - if (typeof item === 'string') { - item = document.getElementById(item); - } else if (item.length) { - // Support for array based queries (such as jQuery) - item = item[0]; - } - - if (item && item.canvas) { - // Support for any object associated to a canvas (including a context2d) - item = item.canvas; - } + } + acquireContext(item, config) { // To prevent canvas fingerprinting, some add-ons undefine the getContext // method, for example: https://github.com/kkapsner/CanvasBlocker // https://github.com/chartjs/Chart.js/issues/2807 @@ -381,9 +370,9 @@ export default { } return null; - }, + } - releaseContext: function(context) { + releaseContext(context) { const canvas = context.canvas; if (!canvas[EXPANDO_KEY]) { return; @@ -412,9 +401,9 @@ export default { canvas.width = canvas.width; delete canvas[EXPANDO_KEY]; - }, + } - addEventListener: function(chart, type, listener) { + addEventListener(chart, type, listener) { var canvas = chart.canvas; if (type === 'resize') { // Note: the resize event is not supported on all browsers. @@ -429,9 +418,9 @@ export default { }, chart); addListener(canvas, type, proxy); - }, + } - removeEventListener: function(chart, type, listener) { + removeEventListener(chart, type, listener) { var canvas = chart.canvas; if (type === 'resize') { // Note: the resize event is not supported on all browsers. @@ -448,4 +437,4 @@ export default { removeListener(canvas, type, proxy); } -}; +} diff --git a/src/platforms/platform.js b/src/platforms/platform.js index b46df388e91..d7765334b8f 100644 --- a/src/platforms/platform.js +++ b/src/platforms/platform.js @@ -1,105 +1,48 @@ 'use strict'; -import helpers from '../helpers/index'; -import basic from './platform.basic'; -import dom from './platform.dom'; - -function extendPlatform(implementation) { - return helpers.extend({ - /** - * @since 3.0.0 - * @memberof Chart.platform.current - */ - type: undefined, - - /** - * @since 2.7.0 - * @memberof Chart.platform.current - */ - initialize: function() {}, - - /** - * Called at chart construction time, returns a context2d instance implementing - * the [W3C Canvas 2D Context API standard]{@link https://www.w3.org/TR/2dcontext/}. - * @param {*} item - The native item from which to acquire context (platform specific) - * @param {object} options - The chart options - * @returns {CanvasRenderingContext2D} context2d instance - * @memberof Chart.platform.current - */ - acquireContext: function() {}, - - /** - * Called at chart destruction time, releases any resources associated to the context - * previously returned by the acquireContext() method. - * @param {CanvasRenderingContext2D} context - The context2d instance - * @returns {boolean} true if the method succeeded, else false - * @memberof Chart.platform.current - */ - releaseContext: function() {}, - - /** - * Registers the specified listener on the given chart. - * @param {Chart} chart - Chart from which to listen for event - * @param {string} type - The ({@link IEvent}) type to listen for - * @param {function} listener - Receives a notification (an object that implements - * the {@link IEvent} interface) when an event of the specified type occurs. - * @memberof Chart.platform.current - */ - addEventListener: function() {}, - - /** - * Removes the specified listener previously registered with addEventListener. - * @param {Chart} chart - Chart from which to remove the listener - * @param {string} type - The ({@link IEvent}) type to remove - * @param {function} listener - The listener function to remove from the event target. - * @memberof Chart.platform.current - */ - removeEventListener: function() {} - - }, implementation); -} - - -var defaultImplementation = dom._enabled ? dom : basic; - /** -* @namespace Chart.platform -* Allows getting and setting the current platform -* @since 3.0.0 -*/ -var platform = { + * Abstract class that allows abstracting platform dependencies away from the chart. + */ +export default class Platform { /** - * @type Array.{IPlatform} - * A list of platforms that can be set + * @param {object} options - The chart options */ - availablePlatforms: [dom, basic], + constructor() {} + /** - * @namespace Chart.platform.current - * Changed from Chart.platform to Chart.platform.current in 3.0.0 - * @see https://chartjs.gitbooks.io/proposals/content/Platform.html - * @since 2.4.0 - */ - current: extendPlatform(defaultImplementation), + * Called at chart construction time, returns a context2d instance implementing + * the [W3C Canvas 2D Context API standard]{@link https://www.w3.org/TR/2dcontext/}. + * @param {*} item - The native item from which to acquire context (platform specific) + * @param {object} options - The chart options + * @returns {CanvasRenderingContext2D} context2d instance + */ + acquireContext() {} + /** - * @param {IPlatform} implementation - the platform implementation to set as the current platform - * Sets the current platform. + * Called at chart destruction time, releases any resources associated to the context + * previously returned by the acquireContext() method. + * @param {CanvasRenderingContext2D} context - The context2d instance + * @returns {boolean} true if the method succeeded, else false */ - setPlatform: function(implementation) { - platform.current = extendPlatform(implementation); - }, -}; + releaseContext() {} -export default platform; + /** + * Registers the specified listener on the given chart. + * @param {Chart} chart - Chart from which to listen for event + * @param {string} type - The ({@link IEvent}) type to listen for + * @param {function} listener - Receives a notification (an object that implements + * the {@link IEvent} interface) when an event of the specified type occurs. + */ + addEventListener() {} -/** - * @interface IPlatform - * Allows abstracting platform dependencies away from the chart - * @borrows Chart.platform.current.type as type - * @borrows Chart.platform.current.acquireContext as acquireContext - * @borrows Chart.platform.current.releaseContext as releaseContext - * @borrows Chart.platform.current.addEventListener as addEventListener - * @borrows Chart.platform.current.removeEventListener as removeEventListener - */ + /** + * Removes the specified listener previously registered with addEventListener. + * @param {Chart} chart - Chart from which to remove the listener + * @param {string} type - The ({@link IEvent}) type to remove + * @param {function} listener - The listener function to remove from the event target. + */ + removeEventListener() {} +} /** * @interface IEvent diff --git a/src/platforms/platforms.js b/src/platforms/platforms.js new file mode 100644 index 00000000000..2484a3056c1 --- /dev/null +++ b/src/platforms/platforms.js @@ -0,0 +1,13 @@ +'use strict'; + +import Platform from './platform'; +import BasicPlatform from './platform.basic'; +import DomPlatform from './platform.dom'; + +export {BasicPlatform, DomPlatform, Platform}; + +/** + * @namespace Chart.platforms + * @see https://chartjs.gitbooks.io/proposals/content/Platform.html +*/ +export default {BasicPlatform, DomPlatform, Platform}; diff --git a/test/specs/global.namespace.tests.js b/test/specs/global.namespace.tests.js index 2577e3cc650..255c7e685ba 100644 --- a/test/specs/global.namespace.tests.js +++ b/test/specs/global.namespace.tests.js @@ -12,8 +12,7 @@ describe('Chart namespace', function() { expect(Chart.Interaction instanceof Object).toBeTruthy(); expect(Chart.layouts instanceof Object).toBeTruthy(); expect(Chart.plugins instanceof Object).toBeTruthy(); - expect(Chart.platform instanceof Object).toBeTruthy(); - expect(Chart.platform.current instanceof Object).toBeTruthy(); + expect(Chart.platforms instanceof Object).toBeTruthy(); expect(Chart.Scale instanceof Object).toBeTruthy(); expect(Chart.scaleService instanceof Object).toBeTruthy(); expect(Chart.Ticks instanceof Object).toBeTruthy(); diff --git a/test/specs/platform.tests.js b/test/specs/platform.tests.js deleted file mode 100644 index e5026360e23..00000000000 --- a/test/specs/platform.tests.js +++ /dev/null @@ -1,17 +0,0 @@ -describe('Chart.platform', function() { - it('the default platform is dom', function() { - expect(Chart.platform.current.type).toEqual('dom'); - }); - - it('the platform can be changed to basic', function() { - const basicPlatform = Chart.platform.availablePlatforms.find((p) => p.type === 'basic'); - expect(Chart.platform.current.type).toEqual('dom'); - - Chart.platform.setPlatform(basicPlatform); - expect(Chart.platform.current.type).toEqual('basic'); - - // Reset to dom to clean up the test. - const domPlatform = Chart.platform.availablePlatforms.find((p) => p.type === 'dom'); - Chart.platform.setPlatform(domPlatform); - }); -}); From c005cad8c35a03f7bf6224b75a606b057142676c Mon Sep 17 00:00:00 2001 From: David Winegar Date: Thu, 16 Jan 2020 15:38:26 -0800 Subject: [PATCH 3/7] Address comments --- docs/getting-started/v3-migration.md | 5 +++-- src/core/core.controller.js | 21 +++++++++------------ src/platforms/platform.dom.js | 21 ++++++++++++--------- src/platforms/platform.js | 6 ++++-- 4 files changed, 28 insertions(+), 25 deletions(-) diff --git a/docs/getting-started/v3-migration.md b/docs/getting-started/v3-migration.md index 9791b5025ba..47d0c741fac 100644 --- a/docs/getting-started/v3-migration.md +++ b/docs/getting-started/v3-migration.md @@ -63,6 +63,8 @@ Chart.js 3.0 introduces a number of breaking changes. Chart.js 2.0 was released * `scales.[x/y]Axes.time.max` was renamed to `scales[id].max` * `scales.[x/y]Axes.time.min` was renamed to `scales[id].min` * The dataset option `tension` was renamed to `lineTension` +* To override the platform class used in a chart instance, pass `platform: PlatformClass` in the config object. Note that the class should be passed, not an instance of the class. +* To disable CSS injection (necessary in some iframe contexts), `Chart.platform.disableCSSInjection = true` is no longer supported. Pass `disableCSSInjection: true` in the config object instead. ### Animations @@ -161,6 +163,7 @@ Animation system was completely rewritten in Chart.js v3. Each property can now * `helpers._alignPixel` was renamed to `helpers.canvas._alignPixel` * `helpers._decimalPlaces` was renamed to `helpers.math._decimalPlaces` +* `chart.initialize` was renamed to `chart._initialize` (labeled as private but not named as such) ### Changed @@ -211,5 +214,3 @@ Animation system was completely rewritten in Chart.js v3. Each property can now * `Chart.platform` no longer exists. Every chart instance now has a separate platform instance, `chart.platform`. * `Chart.platforms` is an object that contains two usable platform classes, `BasicPlatform` and `DomPlatform`. It also contains `Platform`, a class that all platforms must extend from. * If the canvas passed in is an instance of `OffscreenCanvas`, the `BasicPlatform` is automatically used. -* T override the platform class used in a chart instance, pass `platform: PlatformClass` in the config object. Note that the class should be passed, not an instance of the class. -* To disable CSS injection (necessary in some iframe contexts), `Chart.platform.disableCSSInjection = true` is no longer supported. Pass `disableCSSInjection: true` in the config object instead. diff --git a/src/core/core.controller.js b/src/core/core.controller.js index 9adbc03356c..6a274bd6899 100644 --- a/src/core/core.controller.js +++ b/src/core/core.controller.js @@ -152,7 +152,7 @@ function onAnimationProgress(ctx) { * Chart.js can take a string id of a canvas element, a 2d context, or a canvas element itself. * Attempt to unwrap the item passed into the chart constructor so that it is a canvas element (if possible). */ -function unwrapItem(item) { +function getCanvas(item) { if (typeof document !== undefined && typeof item === 'string') { item = document.getElementById(item); } else if (item.length) { @@ -172,10 +172,10 @@ class Chart { const me = this; config = initConfig(config); - const unwrappedItem = unwrapItem(item); - me.initializePlatform(unwrappedItem, config); + const initialCanvas = getCanvas(item); + me._initializePlatform(initialCanvas, config); - const context = me.platform.acquireContext(unwrappedItem, config); + const context = me.platform.acquireContext(initialCanvas, config); const canvas = context && context.canvas; const height = canvas && canvas.height; const width = canvas && canvas.width; @@ -217,14 +217,14 @@ class Chart { Animator.listen(me, 'complete', onAnimationsComplete); Animator.listen(me, 'progress', onAnimationProgress); - me.initialize(); + me._initialize(); me.update(); } /** * @private */ - initialize() { + _initialize() { const me = this; // Before init plugin notification @@ -250,17 +250,14 @@ class Chart { /** * @private */ - initializePlatform(item, config) { + _initializePlatform(canvas, config) { const me = this; if (config.platform) { - if (!(config.platform instanceof Platform)) { - throw new Error('If config.platform is used, it must be an instance of Chart.platforms.Platform or one of its descendants'); - } me.platform = new config.platform(config); - } else if (typeof window === 'undefined' || typeof document === 'undefined' || !item) { + } else if (typeof window === 'undefined' || typeof document === 'undefined') { me.platform = new BasicPlatform(config); - } else if (window.OffscreenCanvas && item instanceof window.OffscreenCanvas) { + } else if (window.OffscreenCanvas && canvas instanceof window.OffscreenCanvas) { me.platform = new BasicPlatform(config); } else { me.platform = new DomPlatform(config); diff --git a/src/platforms/platform.dom.js b/src/platforms/platform.dom.js index d3a08478338..e11a4d4fdf1 100644 --- a/src/platforms/platform.dom.js +++ b/src/platforms/platform.dom.js @@ -315,10 +315,13 @@ function injectCSS(rootNode, css) { /** * Platform class for charts that can access the DOM and global window/document properties - * @constructor * @extends Platform */ export default class DomPlatform extends Platform { + /** + * @constructor + * @param {object} options - The chart options + */ constructor(options) { super(); @@ -329,7 +332,7 @@ export default class DomPlatform extends Platform { * to be manually imported to make this library compatible with any CSP. * See https://github.com/chartjs/Chart.js/issues/5208 */ - this.disableCSSInjection = options.disableCSSInjection || false; + this.disableCSSInjection = options.disableCSSInjection; } /** @@ -348,24 +351,24 @@ export default class DomPlatform extends Platform { } } - acquireContext(item, config) { + acquireContext(canvas, config) { // To prevent canvas fingerprinting, some add-ons undefine the getContext // method, for example: https://github.com/kkapsner/CanvasBlocker // https://github.com/chartjs/Chart.js/issues/2807 - var context = item && item.getContext && item.getContext('2d'); + var context = canvas && canvas.getContext && canvas.getContext('2d'); - // `instanceof HTMLCanvasElement/CanvasRenderingContext2D` fails when the item is + // `instanceof HTMLCanvasElement/CanvasRenderingContext2D` fails when the canvas is // inside an iframe or when running in a protected environment. We could guess the // types from their toString() value but let's keep things flexible and assume it's - // a sufficient condition if the item has a context2D which has item as `canvas`. + // a sufficient condition if the canvas has a context2D which has canvas as `canvas`. // https://github.com/chartjs/Chart.js/issues/3887 // https://github.com/chartjs/Chart.js/issues/4102 // https://github.com/chartjs/Chart.js/issues/4152 - if (context && context.canvas === item) { + if (context && context.canvas === canvas) { // Load platform resources on first chart creation, to make it possible to // import the library before setting platform options. - this._ensureLoaded(item); - initCanvas(item, config); + this._ensureLoaded(canvas); + initCanvas(canvas, config); return context; } diff --git a/src/platforms/platform.js b/src/platforms/platform.js index d7765334b8f..2200221c93a 100644 --- a/src/platforms/platform.js +++ b/src/platforms/platform.js @@ -5,14 +5,16 @@ */ export default class Platform { /** - * @param {object} options - The chart options + * @constructor + * The Platform constructor has no required parameters, but chart options are passed in as + * the first and only parameter to all Platform constructors. */ constructor() {} /** * Called at chart construction time, returns a context2d instance implementing * the [W3C Canvas 2D Context API standard]{@link https://www.w3.org/TR/2dcontext/}. - * @param {*} item - The native item from which to acquire context (platform specific) + * @param {canvas} canvas - The canvas from which to acquire context (platform specific) * @param {object} options - The chart options * @returns {CanvasRenderingContext2D} context2d instance */ From f9fd4bda8d6c1e96e6162b396bffd6c7658fbb85 Mon Sep 17 00:00:00 2001 From: David Winegar Date: Fri, 17 Jan 2020 11:38:02 -0800 Subject: [PATCH 4/7] Fix tests --- src/core/core.controller.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/core/core.controller.js b/src/core/core.controller.js index 6a274bd6899..755b8fb857c 100644 --- a/src/core/core.controller.js +++ b/src/core/core.controller.js @@ -6,7 +6,7 @@ import defaults from './core.defaults'; import helpers from '../helpers/index'; import Interaction from './core.interaction'; import layouts from './core.layouts'; -import {BasicPlatform, DomPlatform, Platform} from '../platforms/platforms'; +import {BasicPlatform, DomPlatform} from '../platforms/platforms'; import plugins from './core.plugins'; import scaleService from '../core/core.scaleService'; import Tooltip from './core.tooltip'; @@ -148,12 +148,16 @@ function onAnimationProgress(ctx) { helpers.callback(animationOptions && animationOptions.onProgress, arguments, chart); } +function isDomSupported() { + return typeof window !== undefined && typeof document !== undefined; +} + /** * Chart.js can take a string id of a canvas element, a 2d context, or a canvas element itself. * Attempt to unwrap the item passed into the chart constructor so that it is a canvas element (if possible). */ function getCanvas(item) { - if (typeof document !== undefined && typeof item === 'string') { + if (isDomSupported() && typeof item === 'string') { item = document.getElementById(item); } else if (item.length) { // Support for array based queries (such as jQuery) @@ -255,7 +259,7 @@ class Chart { if (config.platform) { me.platform = new config.platform(config); - } else if (typeof window === 'undefined' || typeof document === 'undefined') { + } else if (!isDomSupported()) { me.platform = new BasicPlatform(config); } else if (window.OffscreenCanvas && canvas instanceof window.OffscreenCanvas) { me.platform = new BasicPlatform(config); From 1c297ab253c81c8e31d8895b5d428487f8114654 Mon Sep 17 00:00:00 2001 From: David Winegar Date: Tue, 21 Jan 2020 14:37:51 -0800 Subject: [PATCH 5/7] Make disableCSSInjection a global again --- samples/advanced/content-security-policy.js | 4 ++-- src/core/core.controller.js | 10 +++++----- src/index.js | 4 +++- .../platform.js => platform/platform.base.js} | 4 +--- src/{platforms => platform}/platform.basic.js | 6 +++--- src/{platforms => platform}/platform.dom.css | 0 src/{platforms => platform}/platform.dom.js | 12 ++++++------ src/platform/platform.js | 3 +++ src/{platforms => platform}/platforms.js | 6 +++--- 9 files changed, 26 insertions(+), 23 deletions(-) rename src/{platforms/platform.js => platform/platform.base.js} (92%) rename src/{platforms => platform}/platform.basic.js (82%) rename src/{platforms => platform}/platform.dom.css (100%) rename src/{platforms => platform}/platform.dom.js (98%) create mode 100644 src/platform/platform.js rename src/{platforms => platform}/platforms.js (57%) diff --git a/samples/advanced/content-security-policy.js b/samples/advanced/content-security-policy.js index 4c4daec2c3f..c77d43193b8 100644 --- a/samples/advanced/content-security-policy.js +++ b/samples/advanced/content-security-policy.js @@ -1,6 +1,8 @@ var utils = Samples.utils; utils.srand(110); +// CSP: disable automatic style injection +Chart.platform.disableCSSInjection = true; function generateData() { var DATA_COUNT = 16; @@ -46,8 +48,6 @@ window.addEventListener('load', function() { } } }, - // CSP: disable automatic style injection - disableCSSInjection: true } }); }); diff --git a/src/core/core.controller.js b/src/core/core.controller.js index c8889513e25..2da5705ebf4 100644 --- a/src/core/core.controller.js +++ b/src/core/core.controller.js @@ -6,7 +6,7 @@ import defaults from './core.defaults'; import helpers from '../helpers/index'; import Interaction from './core.interaction'; import layouts from './core.layouts'; -import {BasicPlatform, DomPlatform} from '../platforms/platforms'; +import {BasicPlatform, DomPlatform} from '../platform/platforms'; import plugins from './core.plugins'; import scaleService from '../core/core.scaleService'; @@ -253,13 +253,13 @@ class Chart { const me = this; if (config.platform) { - me.platform = new config.platform(config); + me.platform = new config.platform(); } else if (!isDomSupported()) { - me.platform = new BasicPlatform(config); + me.platform = new BasicPlatform(); } else if (window.OffscreenCanvas && canvas instanceof window.OffscreenCanvas) { - me.platform = new BasicPlatform(config); + me.platform = new BasicPlatform(); } else { - me.platform = new DomPlatform(config); + me.platform = new DomPlatform(); } } diff --git a/src/index.js b/src/index.js index d7adf4c3aee..b32770a0972 100644 --- a/src/index.js +++ b/src/index.js @@ -15,7 +15,8 @@ import Element from './core/core.element'; import elements from './elements'; import Interaction from './core/core.interaction'; import layouts from './core/core.layouts'; -import platforms from './platforms/platforms'; +import platforms from './platform/platforms'; +import platform from './platform/platform'; import pluginsCore from './core/core.plugins'; import Scale from './core/core.scale'; import scaleService from './core/core.scaleService'; @@ -34,6 +35,7 @@ Chart.elements = elements; Chart.Interaction = Interaction; Chart.layouts = layouts; Chart.platforms = platforms; +Chart.platform = platform; Chart.plugins = pluginsCore; Chart.Scale = Scale; Chart.scaleService = scaleService; diff --git a/src/platforms/platform.js b/src/platform/platform.base.js similarity index 92% rename from src/platforms/platform.js rename to src/platform/platform.base.js index 4bca682e4d4..5ef81f74498 100644 --- a/src/platforms/platform.js +++ b/src/platform/platform.base.js @@ -3,11 +3,9 @@ /** * Abstract class that allows abstracting platform dependencies away from the chart. */ -export default class Platform { +export default class BasePlatform { /** * @constructor - * The Platform constructor has no required parameters, but chart options are passed in as - * the first and only parameter to all Platform constructors. */ constructor() {} diff --git a/src/platforms/platform.basic.js b/src/platform/platform.basic.js similarity index 82% rename from src/platforms/platform.basic.js rename to src/platform/platform.basic.js index 6957b76883d..2f2d52c3bc3 100644 --- a/src/platforms/platform.basic.js +++ b/src/platform/platform.basic.js @@ -5,14 +5,14 @@ 'use strict'; -import Platform from './platform'; +import BasePlatform from './platform.base'; /** * Platform class for charts without access to the DOM or to many element properties * This platform is used by default for any chart passed an OffscreenCanvas. - * @extends Platform + * @extends BasePlatform */ -export default class BasicPlatform extends Platform { +export default class BasicPlatform extends BasePlatform { acquireContext(item) { // To prevent canvas fingerprinting, some add-ons undefine the getContext // method, for example: https://github.com/kkapsner/CanvasBlocker diff --git a/src/platforms/platform.dom.css b/src/platform/platform.dom.css similarity index 100% rename from src/platforms/platform.dom.css rename to src/platform/platform.dom.css diff --git a/src/platforms/platform.dom.js b/src/platform/platform.dom.js similarity index 98% rename from src/platforms/platform.dom.js rename to src/platform/platform.dom.js index d049af9b6ec..4b1cd2b7d59 100644 --- a/src/platforms/platform.dom.js +++ b/src/platform/platform.dom.js @@ -6,7 +6,8 @@ import helpers from '../helpers'; import stylesheet from './platform.dom.css'; -import Platform from './platform'; +import BasePlatform from './platform.base'; +import platform from './platform'; var EXPANDO_KEY = '$chartjs'; var CSS_PREFIX = 'chartjs-'; @@ -315,14 +316,13 @@ function injectCSS(rootNode, css) { /** * Platform class for charts that can access the DOM and global window/document properties - * @extends Platform + * @extends BasePlatform */ -export default class DomPlatform extends Platform { +export default class DomPlatform extends BasePlatform { /** * @constructor - * @param {object} options - The chart options */ - constructor(options) { + constructor() { super(); /** @@ -332,7 +332,7 @@ export default class DomPlatform extends Platform { * to be manually imported to make this library compatible with any CSP. * See https://github.com/chartjs/Chart.js/issues/5208 */ - this.disableCSSInjection = options.disableCSSInjection; + this.disableCSSInjection = platform.disableCSSInjection; } /** diff --git a/src/platform/platform.js b/src/platform/platform.js new file mode 100644 index 00000000000..ec81caef8b5 --- /dev/null +++ b/src/platform/platform.js @@ -0,0 +1,3 @@ +'use strict'; + +export default {disableCSSInjection: false}; diff --git a/src/platforms/platforms.js b/src/platform/platforms.js similarity index 57% rename from src/platforms/platforms.js rename to src/platform/platforms.js index 2484a3056c1..a8701f21354 100644 --- a/src/platforms/platforms.js +++ b/src/platform/platforms.js @@ -1,13 +1,13 @@ 'use strict'; -import Platform from './platform'; +import BasePlatform from './platform.base'; import BasicPlatform from './platform.basic'; import DomPlatform from './platform.dom'; -export {BasicPlatform, DomPlatform, Platform}; +export {BasicPlatform, DomPlatform, BasePlatform}; /** * @namespace Chart.platforms * @see https://chartjs.gitbooks.io/proposals/content/Platform.html */ -export default {BasicPlatform, DomPlatform, Platform}; +export default {BasicPlatform, DomPlatform, BasePlatform}; From dc81c40764343b0e012839f48f2cb17967737d35 Mon Sep 17 00:00:00 2001 From: David Winegar Date: Tue, 21 Jan 2020 15:06:52 -0800 Subject: [PATCH 6/7] Update documentation --- docs/getting-started/v3-migration.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/docs/getting-started/v3-migration.md b/docs/getting-started/v3-migration.md index 789210a1545..5f1decb3177 100644 --- a/docs/getting-started/v3-migration.md +++ b/docs/getting-started/v3-migration.md @@ -64,7 +64,6 @@ Chart.js 3.0 introduces a number of breaking changes. Chart.js 2.0 was released * `scales.[x/y]Axes.time.min` was renamed to `scales[id].min` * The dataset option `tension` was renamed to `lineTension` * To override the platform class used in a chart instance, pass `platform: PlatformClass` in the config object. Note that the class should be passed, not an instance of the class. -* To disable CSS injection (necessary in some iframe contexts), `Chart.platform.disableCSSInjection = true` is no longer supported. Pass `disableCSSInjection: true` in the config object instead. ### Animations @@ -212,6 +211,6 @@ Animation system was completely rewritten in Chart.js v3. Each property can now #### Platform -* `Chart.platform` no longer exists. Every chart instance now has a separate platform instance, `chart.platform`. -* `Chart.platforms` is an object that contains two usable platform classes, `BasicPlatform` and `DomPlatform`. It also contains `Platform`, a class that all platforms must extend from. +* `Chart.platform` is no longer the platform object used by charts. It contains only a single configuration option, `disableCSSInjection`. Every chart instance now has a separate platform instance. +* `Chart.platforms` is an object that contains two usable platform classes, `BasicPlatform` and `DomPlatform`. It also contains `BasePlatform`, a class that all platforms must extend from. * If the canvas passed in is an instance of `OffscreenCanvas`, the `BasicPlatform` is automatically used. From b2561d7bdf00a4008d58f0fe0c820a94608aa9c7 Mon Sep 17 00:00:00 2001 From: David Winegar Date: Wed, 22 Jan 2020 10:53:37 -0800 Subject: [PATCH 7/7] Minor fix --- samples/advanced/content-security-policy.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/samples/advanced/content-security-policy.js b/samples/advanced/content-security-policy.js index c77d43193b8..a974fc6f349 100644 --- a/samples/advanced/content-security-policy.js +++ b/samples/advanced/content-security-policy.js @@ -47,7 +47,7 @@ window.addEventListener('load', function() { return (size / 24) * base; } } - }, + } } }); });