chartjs / chartjs-plugin-annotation

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

Error: 'distanceBetweenPoints' is not exported by node_modules/chart.js/helpers/helpers.cjs #831

Closed honzajavorek closed 1 year ago

honzajavorek commented 1 year ago

Hi, upgrading charts.js from 4.1.1 to 4.1.2 (see https://github.com/honzajavorek/junior.guru/pull/1046) causes my CI to fail on this:

[07:25:10] 'buildJS' errored after 11 s
[07:25:10] Error: 'distanceBetweenPoints' is not exported by node_modules/chart.js/helpers/helpers.cjs, imported by node_modules/chartjs-plugin-annotation/dist/chartjs-plugin-annotation.esm.js

I use rollup to build the JS, but the setup is pretty simple as I use just a few lines of extra JS on my site. I'm not sure what I can do about it. Please advise what other info would help to debug this issue, or how otherwise can I contribute to the solution.

stockiNail commented 1 year ago

Not sure that this is related to this plugin. The PR https://github.com/chartjs/Chart.js/pull/11033 in Chart.js, went live with version 4.1.2, changed the export of helpers.

honzajavorek commented 1 year ago

From the line imported by node_modules/chartjs-plugin-annotation/dist/chartjs-plugin-annotation.esm.js in the error I assumed this plugin might need to be updated. Are you suggesting it's rather a regression in charts.js itself?

stockiNail commented 1 year ago

I'd like to say yes... Anyway a PR is already submitted in this plugin: https://github.com/chartjs/chartjs-plugin-annotation/pull/832 which should fix it.

honzajavorek commented 1 year ago

Awesome! That's fast 😳 ❤️

stockiNail commented 1 year ago

Thank you! Due to the fact that I'm not so expert on JS packaging, do you have the chance to test it locally, in your enviroment? just changing package.json in your node_modules for chartjs-plugin-annotation.

honzajavorek commented 1 year ago
  1. npm i chart.js==4.1.2
  2. copied contents of https://raw.githubusercontent.com/chartjs/chartjs-plugin-annotation/f26f4749632f1d51c9bb3de6a3c75c982ca5598b/package.json to node_modules/chartjs-plugin-annotation/package.json
  3. ran my build, failed with the same error 😞
  4. npm i chart.js@4.1.1
  5. ran my build, passed just fine
honzajavorek commented 1 year ago

I use Gulp and my setup looks like this, if that makes any difference:

async function buildJS() {
  const bundle = await rollup({
    input: 'juniorguru/web/static/src/js/index.js',
    plugins: [resolve(), terser()],
  });
  return bundle.write({
    file: 'juniorguru/web/static/bundle.js',
    format: 'iife',
    sourcemap: isLocalDevelopment,
  });
}

If I need to make changes in the setup, I'm okay with it, but then 4.1.2 shouldn't be a patch release 😬

stockiNail commented 1 year ago

The PR has been applied. As soon as new patch version will be released, the issue should be solved.

stockiNail commented 1 year ago

@honzajavorek version 2.1.2 of the plugin has been released. Can you check if it's now working for you?

honzajavorek commented 1 year ago

After this:

npm i chartjs-plugin-annotation@2.1.2 chart.js@4.1.2

I'm getting the same error:

Error: 'distanceBetweenPoints' is not exported by node_modules/chart.js/helpers/helpers.cjs, imported by node_modules/chartjs-plugin-annotation/dist/chartjs-plugin-annotation.esm.js
    at error (/Users/honza/Projects/juniorguru/node_modules/rollup/dist/shared/rollup.js:198:30)
    at Module.error (/Users/honza/Projects/juniorguru/node_modules/rollup/dist/shared/rollup.js:12560:16)
    at Module.traceVariable (/Users/honza/Projects/juniorguru/node_modules/rollup/dist/shared/rollup.js:12919:29)
    at ModuleScope.findVariable (/Users/honza/Projects/juniorguru/node_modules/rollup/dist/shared/rollup.js:11571:39)
    at FunctionScope.findVariable (/Users/honza/Projects/juniorguru/node_modules/rollup/dist/shared/rollup.js:6503:38)
    at ChildScope.findVariable (/Users/honza/Projects/juniorguru/node_modules/rollup/dist/shared/rollup.js:6503:38)
    at ReturnValueScope.findVariable (/Users/honza/Projects/juniorguru/node_modules/rollup/dist/shared/rollup.js:6503:38)
    at ChildScope.findVariable (/Users/honza/Projects/juniorguru/node_modules/rollup/dist/shared/rollup.js:6503:38)
    at Identifier.bind (/Users/honza/Projects/juniorguru/node_modules/rollup/dist/shared/rollup.js:7570:40)
    at CallExpression.bind (/Users/honza/Projects/juniorguru/node_modules/rollup/dist/shared/rollup.js:5400:23)

I reported in https://github.com/chartjs/chartjs-plugin-annotation/issues/831#issuecomment-1375692616 that the proposed solution doesn't work for me, but perhaps I wrote it in a confusing way (first failing with the upgraded version of chart.js, then verifying that the previous version of chart.js works).

stockiNail commented 1 year ago

Thank you! I'll have a look, more in deep to understand what's wrong.

stockiNail commented 1 year ago

@honzajavorek I have cloned your project locally and I was able to reproduce the issue (apologize for time that I needed). To solve the issue, we should wait for new Chart.js release, where PR https://github.com/chartjs/Chart.js/pull/11037 will be included. Applying locally the PR on the package config files (in node_modules/chart.js folder), gulp can build without any issue.

[11:40:15] Using gulpfile ~\git\junior.guru\gulpfile.js
[11:40:15] Starting 'default'...
[11:40:15] Starting 'buildJS'...
[11:40:15] Starting 'buildImages'...
[11:40:15] Starting 'buildStories'...
[11:40:15] Starting 'copyScreenshots'...
[11:40:15] Starting 'buildFlaskCSS'...
[11:40:15] Starting 'copyIconFont'...
[11:40:15] Starting 'buildMkDocsCSS'...
[11:40:34] Finished 'buildFlaskCSS' after 18 s
[11:40:34] Finished 'buildMkDocsCSS' after 18 s
[11:40:34] Finished 'copyIconFont' after 18 s
[11:40:36] gulp-imagemin: Minified 0 images
[11:40:36] Finished 'buildStories' after 20 s
[11:40:40] Finished 'copyScreenshots' after 24 s
[11:40:40] gulp-imagemin: Minified 0 images
[11:40:40] Finished 'buildImages' after 25 s
[11:40:50] Finished 'buildJS' after 34 s //<--------- Ended correctly
honzajavorek commented 1 year ago

@stockiNail Wow, thank you! This is so much more than I'd expect ❤️ My project isn't exactly contributions-friendly (not yet), so thanks for going this far and trying it out like this. I'm fine waiting for next releases, I'm in no rush. Hopefully this helps anyone who could have a similar problem.

Click here for offtopic info about what I'm using this library for… If you are curious, junior.guru is a small project where I'm trying to be as transparent as possible about its financial results. I have open stats. Your library helps me to have vertical lines in my charts so I can mark individual years or milestones. Screenshot 2023-01-16 at 12 25 02
stockiNail commented 1 year ago

Cool! for my curiosity, are you drawing the labels in the chart by the plugin as well?

honzajavorek commented 1 year ago

Yes. I use data- HTML attributes to specify chart options ad hoc, so you can see everything in the code:

Screenshot 2023-01-16 at 13 15 03

The JS is minimal. It's all declarative except that I had to adjust the options of the labels in the JS to position them to the top.

stockiNail commented 1 year ago

@honzajavorek let me inform you that Chart.js 4.2.0 has been released with the PR which should solve this issue. I have tested locally your project with

  "dependencies": {
    "@fontsource/inter": "4.5.14",
    "bootstrap-icons": "1.10.3",
    "chart.js": "4.2.0", // <---- NEW 
    "chartjs-plugin-annotation": "2.1.2", // <---- LAST
    "instant.page": "5.1.1",
    "rough-notation": "0.5.1"
  },

and the building is working without any issue.

[15:38:24] Using gulpfile ~\git\junior.guru\gulpfile.js
[15:38:24] Starting 'default'...
[15:38:24] Starting 'buildJS'...
[15:38:24] Starting 'buildImages'...
[15:38:24] Starting 'buildStories'...
[15:38:24] Starting 'copyScreenshots'...
[15:38:24] Starting 'buildFlaskCSS'...
[15:38:24] Starting 'copyIconFont'...
[15:38:24] Starting 'buildMkDocsCSS'...
[15:38:29] Finished 'buildFlaskCSS' after 5.33 s
[15:38:29] Finished 'buildMkDocsCSS' after 5.33 s
[15:38:29] Finished 'copyIconFont' after 5.33 s
[15:38:30] gulp-imagemin: Minified 0 images
[15:38:30] Finished 'buildStories' after 5.97 s
[15:38:31] Finished 'copyScreenshots' after 7.33 s
[15:38:31] gulp-imagemin: Minified 0 images
[15:38:31] Finished 'buildImages' after 7.37 s
[15:38:34] Finished 'buildJS' after 10 s

If you can confirm that it's working for you as well, we can close the issue.

honzajavorek commented 1 year ago

Hi, I've had a few days of vacation. Dependabot created PRs with the upgrades for me, and both were green, so I merged them yesterday night so I'm on the latest versions. And it seems to be all working now! Thanks so much for fixing this and helping with this issue ❤️

stockiNail commented 1 year ago

Glad to hear that! You're welcome!