Today’s on the UI group meeting, @sharvit made a deep dive about the @theforeman/vendor sub-package.
This explains how foreman core and plugins import shared packages from the foreman’s vendor, and also why the test environment is different from development and production by its dependencies usage.
You are welcome to watch -
Thanks, @sharvit for this deep dive!
In summary, development (while running on webpack-dev-server) and production use the compiled version of vendor-core packages list, these compiled files are located under @theforeman/vendor/dist folder.
On the other hand, @theforeman/test and @theforeman/stories install all vendor-core packages via npm according to the vendor-core package.json file (and not from the lock file), hence this explains our test errors and broken snapshots in our test environment from time to time.
I understand why this approach made, during testing we can find issues.
However, when a package doesn’t comply with semantic versioning, then a breaking change might occur even in a minor update, moreover, foreman-js also uses dependency-bot, so if there’s a patch, it creates a PR automatically.
Due to our experience with patterfnly 4 versioning, I wonder if this approach helps us or just makes us slower.
What do you think? do we benefit from these differences between our environments anymore? do you have any other approach to suggest?
I don’t want to judge here too much, I belive that keeping the dependencies at one place make sense and makes our packaging easier and also shrinks the final vendor.js bundle by a lot so it’s quite usefull to have the dependencies as external.
I’d argue, that in tests we should try mock the external dependecies as much as possible, but where we don’t mock, we should try to use the same versions as we do in production. As tests are just means to tell us if production doesn’t have bugs, so testing with other versions is not as usefull.
I belive that keeping the dependencies at one place make sense and makes our packaging easier and also shrinks the final vendor.js bundle by a lot so it’s quite usefull to have the dependencies as external.
I agree, @theforeman/vendor saves us from a lot of troubles.
we should try to use the same versions as we do in production. As tests are just means to tell us if production doesn’t have bugs, so testing with other versions is not as useful.
Agree as well, tests should reflect the current state, if not, black magic might happen .
I think a naive (and easy to implement) solution for this inconsistency, can be pinning packages versions (using ~ rather than ^) in the @theforeman/vendor-core package.json, after all, the package-lock file doesn’t publish at all, and we have dependency-bot to cover us when an upgrade is needed.
Another approach would be using the same mechanism of webpack external with jest, but the only problem is that jest doesn’t use webpack.
I think getting dev and production envs closer is generally a good thing. Having dev env moving faster allows longer stabilization on newer versions of dependencies. However I don’t think we use this benefit much today. So I’d lean towards unification. But that’s just gut feeling without much frontend development done.
This sounds like convergence on using the compiled JS from each package, as production does, makes sense to unify how the JS is being used. This also aims to ensure that what is released by foreman-js works and puts the focus more on releasing working packages.
Would any of the proposed changes here have found this issue that did not show up within the JS tests themselves but did show up in packaging where webpack compile is run?
.