openrewrite / rewrite-nodejs

Apache License 2.0
2 stars 1 forks source link

Add possibility of installing / uninstalling dependencies #6

Open martinfrancois opened 1 month ago

martinfrancois commented 1 month ago

What problem are you trying to solve?

If I want to write a recipe to use Jest instead of Jasmine in Angular, I need to uninstall all Karma / Jasmine dependencies and add Jest dependencies. Migrating from Jasmine to Jest can be something that is interesting at scale and become more and more relevant, as Angular is considering to move over to Jest being the default at some point.

Describe the solution you'd like

Similar to upgrading dependencies but for installing / uninstalling dependencies.

API design wise what I could come up with would be the obvious solution of adding one recipe to install a dependency and another to uninstall a dependency. Another possibility would be to introduce a recipe ManageDependency which when null or an empty version is passed as an option will remove the dependency, otherwise install the dependency or update it if it exists. This would be more implicit and might be a bit more complexity in one recipe, but would allow the users of the recipe not having to think about whether a package is already installed or not. However, this would make it necessary to deprecated and eventually remove UpgradeDependencyVersion as it would be too similar. The same advantage of the user not having to know whether the dependency is installed or not could also be achieved by making InstallDependency either add or upgrade as well, where UpgradeDependency could be explicitly used only to update when the dependency exists and throw an exception otherwise, but I can't think of a use case right now where that would be useful.

Have you considered any alternatives or workarounds?

An alternative could be to do it manually, but it would be a bit more difficult to read and could not be reused by others. Another option would be to run an npm command, however this wouldn't be compatible with yarn for example and would require additional logic and couple the implementation to package managers which isn't a very elegant solution.

Additional context

Was discussed with @timtebeek last week in person.

Are you interested in contributing this feature to OpenRewrite?

Yes! I could start the implementation and ask back in case of questions.

timtebeek commented 1 month ago

Great to see you here! Thanks again for the suggestion and offer to help. Let me know how we can best support you through this effort.

martinfrancois commented 1 month ago

@timtebeek you're welcome! I added some details on some ideas I had around the "API design" (recipe design?) by editing the description under "Describe the solution you'd like", I'd be glad to hear your thoughts on them, or please suggest alternatives if you can come up with something even better!

timtebeek commented 1 month ago

Great! I think AddDependency and RemoveDependency, to mirror the recipes we already have for Maven and Gradle:

Note that those examples abstract over Maven or Gradle; there would be no need for that here. Also: the AddDependency is a ScanningRecipe; I also think that wont be necessary here just yet.

Easiest way to start is likely with just an outline of the recipes, and then tests that show a number of before/after scenarios. Feel free to open a draft PR early such that I can help guide the implementation.

timtebeek commented 1 month ago

Removing a dependency is probably the absolute easiest; visit the tree, and if found: return null.

ljharb commented 1 month ago

You don't need any hacks for this kind of thing; just use https://npmjs.com/@npmcli/arborist which is what npm itself uses.