Hi, in the last UX interest group meeting @Dewar suggested adding a package-lock.json to Foreman
to speed up installation and also to provide a more stable environment.
we can update the lock file each time there is a new version bump of @theforeman/vendor and after we test that everything still works.
some numbers:
without Foreman lock file: added 4254 packages from 2661 contributors and audited 4262 packages in 199.281s
with Foreman lock file ( the lock file only contains foreman dependencies, not plugins ): added 3821 packages in 77.409s
That’s a two-minute diff (on a good machine!)
any reason not to move to package-lock.json?
Thanks
A lot of noise to update it every time in git. Noise is PRs and more commits in git log
A lot more PRs and builds means a lot more load on our CI (which is already stressed)
Huge increase in our Git repository size. The current package-lock.json is 1.48 MB. If it changes a lot (and with JS it does), that really makes our repository a lot bigger. It is cloned every time in CI as well and we can’t remove anything (because it’s git)
I also wonder how a package-lock.json in git will behave with plugins. In vanilla Foreman itself we will have a different package set than if you have Katello installed as well. Will that mean git diff will by definition give a different set?
So a concrete test:
Have a pristine foreman.git checkout
bundle install + npm install
git add package-lock.json
Add a plugin with webpack
bundle install + npm install
Now is there a diff? If so, that means we simply can’t include package-lock.json because it will be different for various developers.
All in all I can’t stress how much I am against this.
yes, it will make more PRs, but also a more stable environment.
about the diff, this is what I did:
remove the postinstall script (prevent plugins from being install)
run npm install
remove package-lock.json from .gitignore
stage package-lock.json
returned the postinstall script
run npm ci
results: There was no diff in the package-lock.json.
looks like the posinstall script install the node_modules of plugins.
and I guess that webpack knows how to handle and build each plugin properly with their node_modules. maybe @MariaAga can add more about it?
I think there wont be an issue with it as each plugin has it’s own package-lock.json.
But if I remember the Katello team tried to pin a package in the katello package-lock.json and it didn’t work? @Dewar
That’s correct, pinning versions in Katello doesn’t seem to work.
My understanding of things is as follows:
Katello inherits node_modules and their versions from Foreman, regardless of what we do in Katello as far as I’ve seen. The only benefit I can see of our local node modules is to add packages not in foreman or foreman-js.
Commiting the package-lock to foreman will help to stabilize Katello’s node modules and prevent similar test breakages and/or nightly failures in the future. Dependencies and their working versions are stored there, and if someone updates the package-lock with breaking versions, tests will fail preventing those versions from being possible to commit to git.
And to be clear, all tests were failing in CI as well as locally in Katello, so this wasn’t just a local issue.
When we all discussed this the best practices were for monolithic applications while we are maintaining a pluggable application. The tooling simply wasn’t a good fit.
I still think the current way we deal with plugins and NPM can be considered a hack. I know @MariaAga is working on updating to webpack 5 and after that look at properly exposing the Foreman code as a real module that can be reused in plugins. Only then can you say that webpack understands the relations. Until then, having a package-lock.json is IMHO just fooling yourself and giving a false sense of sanity.
