Closed (fixed)
Project:
Charts
Version:
5.1.x-dev
Component:
Documentation
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
20 May 2025 at 19:46 UTC
Updated:
7 Oct 2025 at 04:29 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
tommasorandazzoTaking a look at MidCamp 2025
Comment #4
tommasorandazzoI updated the MR with a first pass at the Highcharts libraries. Let's refine Highcharts and then we can duplicate the effort for the other library options!
Comment #5
tommasorandazzoandileco mentioned that the install hooks are checking for these in the `/libraries` dir in the webroot. Instead of `[submodule/dist/js` we will want to match those paths to the `/libraries` dir.
Comment #6
andileco commentedThank you for getting this started, @tommasorandazzo! The video of the presentation is up: https://youtu.be/irKVKHPSWZ4?si=OrASg2ELCrJ3SLPv&t=2014
Comment #8
andileco commentedThank you for your work on this, @tommasorandazzo! Would you be willing to take a look at the branch I created? This allows one to have a package.json at the root like:
{
"name": "foo",
"private": true,
"scripts": {
"postinstall": "npm run libraries:copy --workspaces --if-present"
},
"workspaces": [
"web/modules/**/*"
]
}
and when you run `npm install`, it puts the libraries into their proper folders (if you only wanted certain charts submodules, you could adjust the workspaces).
You are clearly more adept at using npm than me, so please let me know if your thoughts on this approach.
Comment #9
erutan commentedHaving trouble pushing, in the base readme.md I'd make the following changes to lines 1 & 2 to make them actionable vs vague "ensure":
1. Run `composer require wikimedia/composer-merge-plugin` to ensure that you have
the `wikimedia/composer-merge-plugin` package installed.
2. Run `composer require oomphinc/composer-installers-extender` to ensure that you
have the `oomphinc/composer-installers-extender` package installed.
Should these be pinned to a major version or something?
Also the following should refer to creating/modifying package.json, not running `composer require`
7. Run the `composer require` specified in the submodule’s README.md file
Comment #10
erutan commentedSo this runs, and is really cool. Once things settle down I think it'd be really neat to have some kind of post-install script in composer that just runs `npm install` after a `composer upgrade` run. :)
There's a LOT of deprecated packages in here that should be modernized (redundant console output removed). I had a package-lock.json but it basically just had the project name in it. Along with updating the readme.md I'd put it as needs work, but this runs as is (I'll keep the changes in my composer.json heh).
Comment #11
erutan commentedRan a standard composer upgrade on top of it out of curiousity and it pulled highcharts/solid-gauge. Looking at the package.json in charts_highcharts it's missing that package.
Comment #12
andileco commented@erutan, good catch about solid-gauge.
I'm realizing that it may make sense to update the readme to be more specific to charts. The "workspaces" that you used travelled through *all* the modules in your site, and some of them must be using outdated npm versions. Instead, we should put, "web/modules/contrib/charts/**/*" for "workspaces".
That said, I did identify that some D3 packages need updating. I don't know if these are recent updates. But anyway, those will need to be updated before this is ready to go live.
Comment #13
andileco commentedComment #14
andileco commented- Fix eslint
- Update C3 libraries.yml
- Update Highcharts in package.json
Comment #15
erutan commentedYeah I just ran `npm install` at the base level of the site - interesting side effect there. If things are properly scoped it'd be nice to just be able to update all the modules in a project with it at once, I have to imagine it's capable of running different tasks with their own dependencies.
I've bumped into (and noticed others having) this issue of not being able to update someone else's issue branch, sorry for not being able to just fix some of the little things. :)
Comment #16
andileco commentedComment #17
erutan commentedI might just be doing something wrong here, but I think the workspace limitation might have borked the install?
Copied this fork over, deleted my
package-lock.json, and removed the highcharts libraries fromweb/libraries.Running npm install from both root and from the contrib folder just outputs the following and no highcharts libraries are downloaded.
Comment #18
andileco commented@erutan - I've attached a screenshare. Please take a look and let me know what you are doing differently.
Comment #19
erutan commentedLooked things over, and the most likely explanation was that I goofed on copying over the right charts folder. Did it again and it worked. Assuming the rest of the libraries work as you've gone over them I'll mark it as RBTC. Once it's in dev I can pull an issue fork and make a stab at updating documentation at some point.
The old root package.json had:
I have a very basic functional understanding of NPM but never have had the need to dig deep into it. On a more meta level going forward in order to keep dependencies clean one would just keep adding new workspaces to package.json per project that uses this approach?
Comment #20
andileco commented@erutan, yes, exactly, that's how they can opt-in other projects where they need a JS library to be installed.
Comment #21
andileco commented