chartjs / chartjs-plugin-annotation

Annotation plugin for Chart.js
MIT License
603 stars 325 forks source link

Upgrade dependencies and implementation of the integration test #827

Closed stockiNail closed 1 year ago

stockiNail commented 1 year ago

This PR is changing the dependency with CHART.JS, going to version4. It's also adding the test on TS 4.7.x

stockiNail commented 1 year ago

Maybe it should ve testing both?

Chart.js 3 and 4?

kurkle commented 1 year ago

Yes, or was v3 support already droped?

stockiNail commented 1 year ago

V3 should be supported, version 3.7.0

stockiNail commented 1 year ago

I will work tomorrow on that. I think we need a package.json template to update at runtime, settimg v3 and then v4. I need to see how the fs api in js are working

kurkle commented 1 year ago

Its already running for each of the ts versions, should not be hard to add another loop for that. Or just split to separate folders (and maybe test only some of the later ts versions with v4)

stockiNail commented 1 year ago

Its already running for each of the ts versions, should not be hard to add another loop for that. Or just split to separate folders (and maybe test only some of the later ts versions with v4)

@kurkle I have done! Have a look if ok. Let me say that the implementation is getting the devDep and peerDep of the plugin to chart.js and based on that it calculates the amount of majors between them. And then, configure the package.json for testing to use the last version in the major, by [major].x.x. In this way we don't have to change anything next time, when the plugin will change the dependencies to Chart.js.

[test-*types-integration] added 9 packages, and audited 10 packages in 15s
[test-*types-integration] 
[test-*types-integration] found 0 vulnerabilities
[test-*types-integration] 
[test-*types-integration] > test
[test-*types-integration] > node test.js
[test-*types-integration] 
[test-*types-integration] Testing on typescript-4.7 ...
[test-*types-integration] Testing on typescript-4.6 ...
[test-*types-integration] Testing on typescript-4.5 ...
[test-*types-integration] Testing on typescript-4.4 ...
[test-*types-integration] Testing on typescript-4.3 ...
[test-*types-integration] Testing on typescript-4.2 ...
[test-*types-integration] Testing on typescript-4.1 ...
[test-*types-integration]     ✔ chartjs-plugin-annotation should compile with all supported TS versions on Chart.js version 3.x.x (51533ms)
[test-*types-integration] 
[test-*types-integration] added 10 packages, and audited 11 packages in 4s
[test-*types-integration] 
[test-*types-integration] found 0 vulnerabilities
[test-*types-integration] 
[test-*types-integration] > test
[test-*types-integration] > node test.js
[test-*types-integration] 
[test-*types-integration] Testing on typescript-4.7 ...
[test-*types-integration] Testing on typescript-4.6 ...
[test-*types-integration] Testing on typescript-4.5 ...
[test-*types-integration] Testing on typescript-4.4 ...
[test-*types-integration] Testing on typescript-4.3 ...
[test-*types-integration] Testing on typescript-4.2 ...
[test-*types-integration] Testing on typescript-4.1 ...
[test-*types-integration]     ✔ chartjs-plugin-annotation should compile with all supported TS versions on Chart.js version 4.x.x (23987ms)
stockiNail commented 1 year ago

Must be rebased when #838 will be merged. Now in draft

kurkle commented 1 year ago

Many lines of code to avoid adding a chart.js major once a year or two? :) Less maintenance is less maintenance, so why not.

kurkle commented 1 year ago

Another thought, maybe this one should be converted to pnpm too, so it could use workspace dependencies and avoid the temporary directory stuff. Like its done in chart.js now.

stockiNail commented 1 year ago

Many lines of code to avoid adding a chart.js major once a year or two? :) Less maintenance is less maintenance, so why not.

I could agree. For my mindset, I'm never afraid about maintenance or LoC ;)

Another thought, maybe this one should be converted to pnpm too, so it could use workspace dependencies and avoid the temporary directory stuff. Like its done in chart.js now.

OK! for this one, I need more time to have a look how it works with more details that I have got now.

stockiNail commented 1 year ago

Close for now