From f314d25b96cd4528aadae00ab881d50667606ca4 Mon Sep 17 00:00:00 2001 From: Evert Harmeling Date: Wed, 14 Dec 2022 11:06:35 +0100 Subject: [PATCH 1/8] Add support for chart.js:^4.0 --- src/Chartjs/assets/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Chartjs/assets/package.json b/src/Chartjs/assets/package.json index c278b0f5b0f..84b4f6d8e6a 100644 --- a/src/Chartjs/assets/package.json +++ b/src/Chartjs/assets/package.json @@ -17,7 +17,7 @@ }, "peerDependencies": { "@hotwired/stimulus": "^3.0.0", - "chart.js": "^3.4.1" + "chart.js": "^3.4.1 || ^4.0" }, "devDependencies": { "@hotwired/stimulus": "^3.0.0", From 768507e41f3bd283abf871dff7dfdf0fdeb6f8d6 Mon Sep 17 00:00:00 2001 From: Evert Harmeling Date: Wed, 14 Dec 2022 11:09:50 +0100 Subject: [PATCH 2/8] Update CHANGELOG.md --- src/Chartjs/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Chartjs/CHANGELOG.md b/src/Chartjs/CHANGELOG.md index aafa4aad040..1643eb70848 100644 --- a/src/Chartjs/CHANGELOG.md +++ b/src/Chartjs/CHANGELOG.md @@ -3,6 +3,7 @@ ## Unreleased - Add `assets/src` to `.gitattributes` to exclude them from the installation +- Support added for Chart.js version 4 ## 2.6.0 From 7a36a4f4ff8fcc2172ceb64795adb4f91c8cb823 Mon Sep 17 00:00:00 2001 From: Evert Harmeling Date: Wed, 14 Dec 2022 16:26:49 +0100 Subject: [PATCH 3/8] Update devDependency for `chart.js` --- src/Chartjs/assets/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Chartjs/assets/package.json b/src/Chartjs/assets/package.json index 84b4f6d8e6a..dfc2fdd83bd 100644 --- a/src/Chartjs/assets/package.json +++ b/src/Chartjs/assets/package.json @@ -22,7 +22,7 @@ "devDependencies": { "@hotwired/stimulus": "^3.0.0", "@types/chart.js": "^2.9.34", - "chart.js": "^3.4.1 <3.9", + "chart.js": "^4.0", "jest-canvas-mock": "^2.3.0", "resize-observer-polyfill": "^1.5.1" } From 5cb0ee99d45f12441a875c0a8405191885c23b9d Mon Sep 17 00:00:00 2001 From: Evert Harmeling Date: Thu, 15 Dec 2022 22:41:56 +0100 Subject: [PATCH 4/8] Exclude v3.9 --- src/Chartjs/assets/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Chartjs/assets/package.json b/src/Chartjs/assets/package.json index dfc2fdd83bd..9e16b21e025 100644 --- a/src/Chartjs/assets/package.json +++ b/src/Chartjs/assets/package.json @@ -17,7 +17,7 @@ }, "peerDependencies": { "@hotwired/stimulus": "^3.0.0", - "chart.js": "^3.4.1 || ^4.0" + "chart.js": "^3.4.1 <3.9 || ^4.0" }, "devDependencies": { "@hotwired/stimulus": "^3.0.0", From 42fb4192c746f58549eeca9267dd22cb3f37c37e Mon Sep 17 00:00:00 2001 From: Evert Harmeling Date: Thu, 15 Dec 2022 22:54:41 +0100 Subject: [PATCH 5/8] Create controller per version --- src/Chartjs/assets/package.json | 12 +++++++++-- .../{controller.ts => abstract_controller.ts} | 10 +++++++--- src/Chartjs/assets/src/chart_v3_controller.ts | 20 +++++++++++++++++++ src/Chartjs/assets/src/chart_v4_controller.ts | 20 +++++++++++++++++++ 4 files changed, 57 insertions(+), 5 deletions(-) rename src/Chartjs/assets/src/{controller.ts => abstract_controller.ts} (76%) create mode 100644 src/Chartjs/assets/src/chart_v3_controller.ts create mode 100644 src/Chartjs/assets/src/chart_v4_controller.ts diff --git a/src/Chartjs/assets/package.json b/src/Chartjs/assets/package.json index 9e16b21e025..6a97f437cb0 100644 --- a/src/Chartjs/assets/package.json +++ b/src/Chartjs/assets/package.json @@ -7,11 +7,19 @@ "types": "dist/controller.d.ts", "symfony": { "controllers": { - "chart": { - "main": "dist/controller.js", + "chart_v3": { + "main": "dist/chart_v3_controller.js", + "name": "symfony--ux-chartjs--chart", "webpackMode": "eager", "fetch": "eager", "enabled": true + }, + "chart_v4": { + "main": "dist/chart_v4_controller.js", + "name": "symfony--ux-chartjs--chart", + "webpackMode": "eager", + "fetch": "eager", + "enabled": false } } }, diff --git a/src/Chartjs/assets/src/controller.ts b/src/Chartjs/assets/src/abstract_controller.ts similarity index 76% rename from src/Chartjs/assets/src/controller.ts rename to src/Chartjs/assets/src/abstract_controller.ts index 612cf6a42f6..4d340ca1b89 100644 --- a/src/Chartjs/assets/src/controller.ts +++ b/src/Chartjs/assets/src/abstract_controller.ts @@ -10,9 +10,8 @@ 'use strict'; import { Controller } from '@hotwired/stimulus'; -import Chart from 'chart.js/auto'; -export default class extends Controller { +export default abstract class AbstractChartController extends Controller { declare readonly viewValue: any; static values = { @@ -35,7 +34,7 @@ export default class extends Controller { if (!canvasContext) { throw new Error('Could not getContext() from Element'); } - const chart = new Chart(canvasContext, payload); + const chart = this.createChart(canvasContext, payload); this._dispatchEvent('chartjs:connect', { chart }); } @@ -43,4 +42,9 @@ export default class extends Controller { _dispatchEvent(name: string, payload: any) { this.element.dispatchEvent(new CustomEvent(name, { detail: payload })); } + + /** + * To support v3 and v4 of chart.js this help function is added, could be refactored when support for v3 is dropped + */ + abstract createChart(canvasContext: any, payload: any): any; } diff --git a/src/Chartjs/assets/src/chart_v3_controller.ts b/src/Chartjs/assets/src/chart_v3_controller.ts new file mode 100644 index 00000000000..ea7e0e8b32f --- /dev/null +++ b/src/Chartjs/assets/src/chart_v3_controller.ts @@ -0,0 +1,20 @@ +/* + * This file is part of the Symfony package. + * + * (c) Fabien Potencier + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +'use strict'; + +import AbstractChartController from './abstract_controller'; +import Chart from 'chart.js/auto'; +import { ChartConfiguration, ChartItem } from 'chart.js'; + +export default class extends AbstractChartController { + createChart(canvasContext: ChartItem, payload: ChartConfiguration): any { + return new Chart(canvasContext, payload); + } +} diff --git a/src/Chartjs/assets/src/chart_v4_controller.ts b/src/Chartjs/assets/src/chart_v4_controller.ts new file mode 100644 index 00000000000..ea7e0e8b32f --- /dev/null +++ b/src/Chartjs/assets/src/chart_v4_controller.ts @@ -0,0 +1,20 @@ +/* + * This file is part of the Symfony package. + * + * (c) Fabien Potencier + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +'use strict'; + +import AbstractChartController from './abstract_controller'; +import Chart from 'chart.js/auto'; +import { ChartConfiguration, ChartItem } from 'chart.js'; + +export default class extends AbstractChartController { + createChart(canvasContext: ChartItem, payload: ChartConfiguration): any { + return new Chart(canvasContext, payload); + } +} From f86ba3bc59bdb94d2663954bc4892e492ba5d2b9 Mon Sep 17 00:00:00 2001 From: Evert Harmeling Date: Tue, 20 Dec 2022 13:00:01 +0100 Subject: [PATCH 6/8] Create test controller per version (WIP) --- .../assets/test/chart_check_controller.ts | 26 +++++++ ...er.test.ts => chart_v3_controller.test.ts} | 19 +---- .../assets/test/chart_v4_controller.test.ts | 78 +++++++++++++++++++ 3 files changed, 107 insertions(+), 16 deletions(-) create mode 100644 src/Chartjs/assets/test/chart_check_controller.ts rename src/Chartjs/assets/test/{controller.test.ts => chart_v3_controller.test.ts} (86%) create mode 100644 src/Chartjs/assets/test/chart_v4_controller.test.ts diff --git a/src/Chartjs/assets/test/chart_check_controller.ts b/src/Chartjs/assets/test/chart_check_controller.ts new file mode 100644 index 00000000000..39d60f263b7 --- /dev/null +++ b/src/Chartjs/assets/test/chart_check_controller.ts @@ -0,0 +1,26 @@ +/* + * This file is part of the Symfony package. + * + * (c) Fabien Potencier + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +'use strict'; + +import { Controller } from '@hotwired/stimulus' + +// Controller used to check the actual controller was properly booted +export class CheckController extends Controller { + connect() { + this.element.addEventListener('chartjs:pre-connect', () => { + this.element.classList.add('pre-connected'); + }); + + this.element.addEventListener('chartjs:connect', (event) => { + this.element.classList.add('connected'); + this.element.chart = event.detail.chart; + }); + } +} diff --git a/src/Chartjs/assets/test/controller.test.ts b/src/Chartjs/assets/test/chart_v3_controller.test.ts similarity index 86% rename from src/Chartjs/assets/test/controller.test.ts rename to src/Chartjs/assets/test/chart_v3_controller.test.ts index 1f191bfab0d..ec4dfbdff5a 100644 --- a/src/Chartjs/assets/test/controller.test.ts +++ b/src/Chartjs/assets/test/chart_v3_controller.test.ts @@ -9,24 +9,11 @@ 'use strict'; -import { Application, Controller } from '@hotwired/stimulus'; +import { Application } from '@hotwired/stimulus'; +import { CheckController } from './chart_check_controller'; import { getByTestId, waitFor } from '@testing-library/dom'; import { clearDOM, mountDOM } from '@symfony/stimulus-testing'; -import ChartjsController from '../src/controller'; - -// Controller used to check the actual controller was properly booted -class CheckController extends Controller { - connect() { - this.element.addEventListener('chartjs:pre-connect', () => { - this.element.classList.add('pre-connected'); - }); - - this.element.addEventListener('chartjs:connect', (event) => { - this.element.classList.add('connected'); - this.element.chart = event.detail.chart; - }); - } -} +import ChartjsController from '../src/chart_v3_controller'; const startStimulus = () => { const application = Application.start(); diff --git a/src/Chartjs/assets/test/chart_v4_controller.test.ts b/src/Chartjs/assets/test/chart_v4_controller.test.ts new file mode 100644 index 00000000000..bd9cf7b0575 --- /dev/null +++ b/src/Chartjs/assets/test/chart_v4_controller.test.ts @@ -0,0 +1,78 @@ +/* + * This file is part of the Symfony package. + * + * (c) Fabien Potencier + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +'use strict'; + +import { Application } from '@hotwired/stimulus'; +import { CheckController } from './chart_check_controller'; +import { getByTestId, waitFor } from '@testing-library/dom'; +import { clearDOM, mountDOM } from '@symfony/stimulus-testing'; +import ChartjsController from '../src/chart_v4_controller'; + +const startStimulus = () => { + const application = Application.start(); + application.register('check', CheckController); + application.register('chartjs', ChartjsController); + + return application; +}; + +describe('ChartjsController', () => { + let application; + + afterEach(() => { + clearDOM(); + application.stop(); + }); + + it('connect without options', async () => { + const container = mountDOM(` + + `); + + expect(getByTestId(container, 'canvas')).not.toHaveClass('pre-connected'); + expect(getByTestId(container, 'canvas')).not.toHaveClass('connected'); + + application = startStimulus(); + + await waitFor(() => { + expect(getByTestId(container, 'canvas')).toHaveClass('pre-connected'); + expect(getByTestId(container, 'canvas')).toHaveClass('connected'); + }); + + const chart = getByTestId(container, 'canvas').chart; + expect(chart.options.showLines).toBeUndefined(); + }); + + it('connect with options', async () => { + const container = mountDOM(` + + `); + + expect(getByTestId(container, 'canvas')).not.toHaveClass('pre-connected'); + expect(getByTestId(container, 'canvas')).not.toHaveClass('connected'); + + application = startStimulus(); + await waitFor(() => { + expect(getByTestId(container, 'canvas')).toHaveClass('pre-connected'); + expect(getByTestId(container, 'canvas')).toHaveClass('connected'); + }); + + const chart = getByTestId(container, 'canvas').chart; + expect(chart.options.showLines).toBe(false); + }); +}); From 2e8def6cc472af1dff55f953ce1e78f0b613305b Mon Sep 17 00:00:00 2001 From: Evert Harmeling Date: Tue, 20 Dec 2022 13:27:49 +0100 Subject: [PATCH 7/8] Add support for 'low/high' dependency testing --- .github/workflows/test.yaml | 30 +++- src/Chartjs/assets/package.json | 2 +- .../scripts/force-lowest-dependencies.js | 144 ++++++++++++++++++ 3 files changed, 174 insertions(+), 2 deletions(-) create mode 100644 src/Chartjs/assets/scripts/force-lowest-dependencies.js diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 98653ec03a1..648c9601ec7 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -235,13 +235,39 @@ jobs: working-directory: src/Notify run: php vendor/bin/simple-phpunit - tests-js: + tests-js-low-deps: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@master + + - name: Get yarn cache directory path + id: yarn-cache-dir-path + run: echo "dir=$(yarn cache dir)" >> $GITHUB_OUTPUT + + - uses: actions/cache@v2 + id: yarn-cache + with: + path: ${{ steps.yarn-cache-dir-path.outputs.dir }} + key: ${{ runner.os }}-low-yarn-${{ hashFiles('**/package.json') }} + restore-keys: | + ${{ runner.os }}-low-yarn- + + - name: Force Lowest Dependencies + run: node ./src/Chartjs/assets/scripts/force-lowest-dependencies.js + + - run: yarn + + - run: yarn test + + tests-js-high-deps: runs-on: ubuntu-latest steps: - uses: actions/checkout@master + - name: Get yarn cache directory path id: yarn-cache-dir-path run: echo "dir=$(yarn cache dir)" >> $GITHUB_OUTPUT + - uses: actions/cache@v2 id: yarn-cache with: @@ -249,5 +275,7 @@ jobs: key: ${{ runner.os }}-yarn-${{ hashFiles('**/package.json') }} restore-keys: | ${{ runner.os }}-yarn- + - run: yarn + - run: yarn test diff --git a/src/Chartjs/assets/package.json b/src/Chartjs/assets/package.json index 6a97f437cb0..8f3eadfe720 100644 --- a/src/Chartjs/assets/package.json +++ b/src/Chartjs/assets/package.json @@ -30,7 +30,7 @@ "devDependencies": { "@hotwired/stimulus": "^3.0.0", "@types/chart.js": "^2.9.34", - "chart.js": "^4.0", + "chart.js": "^3.4.1 <3.9 || ^4.0", "jest-canvas-mock": "^2.3.0", "resize-observer-polyfill": "^1.5.1" } diff --git a/src/Chartjs/assets/scripts/force-lowest-dependencies.js b/src/Chartjs/assets/scripts/force-lowest-dependencies.js new file mode 100644 index 00000000000..5342c65e061 --- /dev/null +++ b/src/Chartjs/assets/scripts/force-lowest-dependencies.js @@ -0,0 +1,144 @@ +/* + * This file is part of the Symfony Webpack Encore package. + * + * (c) Fabien Potencier + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +'use strict'; + +const fs = require('fs'); +const childProcess = require('child_process'); + +const packageFilePath = 'src/Chartjs/assets/package.json'; +const chartJsLowVersion = '3.4.1'; + +/** + * @param {string} dependency + * @param {string} range + * @return {Promise} + */ +function getLowestVersion(dependency, range) { + return new Promise((resolve, reject) => { + if (range.startsWith('file:')) { + resolve([dependency, range]); + } + + childProcess.exec( + `npm view "${dependency}@${range}" version`, + { encoding: 'utf-8' }, + (error, stdout) => { + if (error) { + reject(`Could not retrieve versions list for "${dependency}@${range}"`); + return; + } + + const versions = stdout + .split('\n') + .filter(line => line); + + if (versions.length === 0) { + reject(`Could not find a lowest version for "${dependency}@${range}"`); + return; + } + + const parts = versions[0].split(' '); + + // If there is only one version available that version + // is directly printed as the output of npm view. + if (parts.length === 1) { + resolve([dependency, parts[0]]); + return; + } + + // If multiple versions are available then it outputs + // multiple lines matching the following format: + // @ '' + if (parts.length === 2) { + resolve([dependency, parts[1].replace(/'/g, '')]); + return; + } + + reject(`Unexpected response for "${dependency}@${range}": ${versions[0]}`); + } + ); + }); +} + +fs.readFile(packageFilePath, (error, data) => { + if (error) { + throw error; + } + + const packageInfo = JSON.parse(data); + + const dependencyPromises = []; + if (packageInfo.dependencies) { + for (const dependency in packageInfo.dependencies) { + dependencyPromises.push(getLowestVersion( + dependency, + packageInfo.dependencies[dependency] + )); + } + } + + const devDependencyPromises = []; + if (packageInfo.devDependencies) { + for (const devDependency in packageInfo.devDependencies) { + devDependencyPromises.push(getLowestVersion( + devDependency, + packageInfo.devDependencies[devDependency] + )); + } + } + + const dependenciesUpdate = Promise.all(dependencyPromises).then(versions => { + versions.forEach(version => { + packageInfo.dependencies[version[0]] = version[1]; + }); + }); + + const devDependenciesUpdate = Promise.all(devDependencyPromises).then(versions => { + versions.forEach(version => { + packageInfo.devDependencies[version[0]] = version[1]; + }); + }); + + // Once all the lowest versions have been resolved, update the + // package.json file accordingly. + Promise + .all([dependenciesUpdate, devDependenciesUpdate]) + .then(() => new Promise((resolve, reject) => { + fs.writeFile(packageFilePath, JSON.stringify(packageInfo, null, 2), (error) => { + if (error) { + reject(error); + return; + } + + resolve(); + }); + })) + .then(() => { + console.log('Manually forcing chart.js to lowest version'); + packageInfo.devDependencies['chart.js'] = chartJsLowVersion; + }) + .then(() => { + console.log('Updated package.json file with lowest dependency versions: '); + + console.log('Dependencies:'); + for (const dependency in packageInfo.dependencies) { + console.log(` - ${dependency}: ${packageInfo.dependencies[dependency]}`); + } + + console.log('Dev dependencies:'); + for (const dependency in packageInfo.devDependencies) { + console.log(` - ${dependency}: ${packageInfo.devDependencies[dependency]}`); + } + }) + .catch(error => { + console.error(error); + process.exit(1); // eslint-disable-line + }); +}); From 0799e87fc97aa636603345a497bbfcaec5b59735 Mon Sep 17 00:00:00 2001 From: Evert Harmeling Date: Tue, 20 Dec 2022 22:07:47 +0100 Subject: [PATCH 8/8] Fix v4 import --- src/Chartjs/assets/src/chart_v4_controller.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Chartjs/assets/src/chart_v4_controller.ts b/src/Chartjs/assets/src/chart_v4_controller.ts index ea7e0e8b32f..44499891691 100644 --- a/src/Chartjs/assets/src/chart_v4_controller.ts +++ b/src/Chartjs/assets/src/chart_v4_controller.ts @@ -10,7 +10,7 @@ 'use strict'; import AbstractChartController from './abstract_controller'; -import Chart from 'chart.js/auto'; +import Chart from 'chart.js/auto/auto.cjs'; import { ChartConfiguration, ChartItem } from 'chart.js'; export default class extends AbstractChartController {