feat: support other js package managers

[G-Rath]

Summary

This adds support for package managers other than yarn by using a new prototype library I've created called package_json which provides an abstraction over top of package.json and the respective managers.

Related to #195

Pull Request checklist

Remove this line after checking all the items here. If the item does not apply to the PR, both check it out and wrap it by ~.

• Add/update test to cover these changes
• Update documentation
• Update CHANGELOG file

Other Information

Foremost I'd like to get feedback from the maintainers about if this is an approach they'd be ok with; beyond that I'm happy to have feedback both on the implementation here & related repos, and on package_json itself - at this point the underlying logic is pretty stable, but I'm very much open to changing the API if the folks have opinions.

I've got this working locally and in our rails-template with all three package managers - I'm still working on getting the test suites for here and react-rails passing, and will also be adding more actual tests to rails-template to confirm everything is actually working with the different managers.

Note that currently Yarn Berry is not supported by package_json mainly because it can't be installed via npm like the others can - I've already looked into the mappings for its commands, and will tackle that once I've got everything across the board.

This will also require a similar change to react-rails which I've got locally but will do that PR later.

[G-Rath]

@ahangarha @justin808 I would appreciate some initial feedback as I think it'll require a major change and probably a bit of overhauling of the test suite so want to make sure you're ok with the general direction before I continue to sink more time into it.

I would also like to land the addition of from_pnpm_lock ahead of this PR even though it won't really be used by itself, because that will greatly reduce the diff and is ready to go already.

[justin808]

I like the concept!

My main concern is the balance of the:

1. Extra complexity of test suites. Will it be easy for us to break compatibility?
2. Value from the other bundlers. Is there a lot of value?

[G-Rath]

Extra complexity of test suites. Will it be easy for us to break compatibility?

I don't think so so long as Shakapacker (& co) don't try to do too much - imo the focus for gems like Shakapacker and react-rails should be in bridging webpack and rails, rather than being a big all-purpose tool; for example, I don't think Shakapacker should try to facilitate installing package managers or node - if a user wants a particular manager, that's something they should be expected to handle.

That also goes for package_json whereby the goal is not "support every thing you could possible do with js package managers" but "provide a uniform Ruby API for doing the standard install/add/remove/run commands across the main package managers" - that means we're not trying to do things like normalize the behaviour regardless of package managers.

In practice this means the main use-case of package_json is intended to be on generators and similar scripts because (imo) once you've got the thing generated, you should switch to using the package manager of choice directly rather than continue using an abstraction because applications shouldn't be supporting multiple package managers.

A good example of this is with bin/setup: I don't think that should contain package_json.manager.install - instead the generator should use package_json to add the native install command for the package manager of choice, and so insert a system call for that.

Shakapacker technically already stretches this because it provides bin scripts for webpack and webpack-dev-server - if it wasn't for those, package_json could be a purely "generator time only" dependency, but while I personally wouldn't be sad if those did get axed (or inlined similar to what I described with bin/setup), I think they're an acceptable middleground.

I expect that approach should make it easier to be compatible across different major versions and managers, and I think that's already been validated somewhat by how straightforward the logic and test suite for package_json has turned out (though don't get me wrong, I'd want this to be available as an experimental version for people to be able to use to get more real-world data 😅).

Value from the other bundlers. Is there a lot of value?

I think that lies in the eyes of the downstream developer - people will tell you that there are advantages to each of the different managers, and they're not wrong though I think it's not entirely wrong either to argue just one of those managers is enough.

I wouldn't say this is necessarily something Shakapacker & co need, but I think it would make them better since it would be giving people downstream more flexibility.

For what it's worth, we prefer npm at work because it natively supports updating packages regardless of their depth in the tree which makes it easier to apply security updates which is something the authors of Yarn don't believe in, and it also ships with node meaning its one less thing to provision on our servers and docker images - but we're using it for Rails applications as its a requirement of Shakapacker and co.

[G-Rath]

fwiw I've got CI passing for my PR to rails-template for all three package managers using a shim of the other two to ensure they're never called - in addition to having used this successfully locally, that's a strong indication that this is working and stable; now to get the CI passing here...

I'm going to attempt to make this opt-in to start with.

[justin808]

@G-Rath what's pending on this draft PR? Is this ready for final review?

[G-Rath]

@justin808 yup I think it should be good to get at least an initial review - CI is green for rails-template and react-rails so I'm pretty confident this works, I just need to write some tests; I ended up feature flagging this, which is why CI is currently passing.

(some of the diff will be reduced by getting #350 merged too)

[G-Rath]

Ok I'm going to officially mark this ready for review the tests that are failing CI are for rendering the react component which I'll have to dig into later because I can't run browser specs with my current local setup, but the failures don't look directly related to this change.

I have also opened a draft PR adding support to react-rails which is passing CI, and the PR to the Ackama rails-template that tests across all package managers and configurations is passing CI (ignoring the failures we get from Lighthouse auditing 😅).

Once this has gone through an initial round of review, I'll look to officially publish the package_json gem unless we've found something very missing from its API. I'll also look to do a PR for react-on-rails, and can also look at other gems that people know which currently hardcode their support for yarn.

I think the main leftover that will need discussion is what to do about the bin/yarn script - personally I think Shakapacker should probably remove it entirely in the next major which would match the spirit of "let downstream devs manage their package managers" and also be the easiest path forward.

Finally, this is currently not documented - I'll get to that over the next couple of days, but for those wanting to try it out now you want to set SHAKAPACKER_USE_PACKAGE_JSON_GEM=true in your env and then set PACKAGE_JSON_FALLBACK_MANAGER to either npm, yarn_classic, yarn_berry, or pnpm. Note that you'll have to handle ensuring the package manager is available.

[justin808]

Why have a separate package_json gem? If react-rails and react-on-rails both use shakapacker, does it need to be something else?

[G-Rath]

Because shakapacker might not be the only use-case, and it doesn't necessarily make sense to have the whole of shakapacker as a dependency for this - I can imagine other gems like jsbundling and even non-rails gems wanting to take advantage of this.

The gem also needs to be installed natively (I.e with bundler inline) for scripts like the Ackama rails-template and shakapackers own template.rb, and I'm not sure how well self-installing will work.

I'm not against it moving into shakapacker or being a shakacode gem, but I think it's better to start as its own gem for now - one of the things I'll be doing now that I've finished the initial implementation here is looking elsewhere in the ecosystem to see what might benefit from this and that'll help inform if it should be its own gem or not.

[justin808]

@G-Rath Any updates?

[G-Rath]

@justin808 what updates are you expecting? I'm currently waiting for you to review this PR

[ahangarha]

@justin808
The package_json gem looks good. And I think we can think of a beta release for Shakapacker.

[justin808]

very quick review

I might be a bit confused...

let me know your thoughts.

[justin808]

doesn't the gem need to be added if configuring to tru?

[justin808]

Usually, we have a config Shakapacker.yml that can be overridden by the ENV value.

[G-Rath]

doesn't the gem need to be added if configuring to tru?

The install should be handled by shakapacker - ackama/rails-template#466 is passing CI using the same self-install method as shakapacker.

Usually, we have a config Shakapacker.yml that can be overridden by the ENV value.

We can't rely on a config alone because this is used when setting up shakapacker - so the config won't exist and users won't have a chance to change it.

Also this is just to allow this to be released as part of stable minor versions so people can test it before it gets made the default in a new major; the env variable can be axed entirely if you're happy to stick with just beta versions and a new major release

[justin808]

do we want this to be an optional dependency, then throw an error if config is true but gem not installed?

[G-Rath]

I don't think so since the long-term goal is that this won't be optional - the only reason it is currently is to allow it to be released in a minor version so people can try it out while still being backwards compatible; that compatibility isn't impacted by the gem being listed as a dependency, and it reduces the complexity of the implementation

[justin808]

Why not traditional memorization?

b/c this value might be false?

[G-Rath]

Because we have to ensure the package_json gem has been installed and required before we attempt to use it - this is still using memorization, it just has another step as part of the initial assignment.

[justin808]

I'm confused since this is a config value.

[G-Rath]

I'm not sure what you mean by this - SHAKAPACKER_USE_PACKAGE_JSON_GEM is not a configuration value; are you saying that you think the error message should be more explicit about it wanting an environment variable?

[justin808]

OK -- looks OK..

Let's push out a version of the other ruby gem first, and then I'll merge this.

Just be ready in case anybody reports any issues!

[justin808]

Only general comment is that code with all the if statements is pretty awkward. I guess once we consider this stable, we'll remove the ENV and if statements.

[justin808]

Just need to fix the CHANGELOG? and see #369.

[G-Rath]

@justin808 have added a changelog - I think we're good to get this landed and do a new release

[h0jeZvgoxFepBQ2C]

So nice, please lets get this merged! ❤️

[justin808]

LGTM