Skip to content

Conversation

@oBusk
Copy link

@oBusk oBusk commented Oct 28, 2025

Closes #185

Removes the "post" action, since it is hard to benefit from it for caching purposes.

Instead shifts to run pnpm store prune directly after executing run_install(s), if any.

Also added an option to the action, store_prune which can be set to false to disable this pruning.

Considerations

Something I'm unsure of, is if some use of the run_install array/object could be used to install something that would be prunable, meaning that the automatic pruning would then remove what you just installed? But installing globally should not make it prunable, I believe? If there's some expected action of running pruning directly following run_install, would it be enough to add to the documentation to explain that you might want to store_prune: false if you do anything weird like that? Or it'd be possible to run prune automatically if run_install is true, and not if it's some kind of advanced setup (array/objects)

oBusk added 8 commits October 28, 2025 00:26
The post action will execute too late to actually improve any caching. By pruning directly after running pnpm install(s), we make sure the store is as clean as possible before continuing. This means any attempt to cache the pnpm store will be caching a pruned store
The original message made it sound like it checked the store and decided there was nothing to prune, in reality pruning just did not run if run_install was not set.
@oBusk
Copy link
Author

oBusk commented Nov 2, 2025

Adding built-in caching makes pruning in the post action a great idea! So #188 is to prefer over this PR

@KSXGitHub KSXGitHub requested a review from zkochan December 5, 2025 12:34
@KSXGitHub KSXGitHub changed the title Prune after installing feat: prune after installing Dec 5, 2025
@KSXGitHub KSXGitHub changed the title feat: prune after installing fix: prune after installing Dec 5, 2025
@KSXGitHub
Copy link
Collaborator

Adding built-in caching makes pruning in the post action a great idea! So #188 is to prefer over this PR

I think both can be combined just fine.

@zkochan
Copy link
Member

zkochan commented Dec 7, 2025

So by default we will always run prune after install? But this is a waste unless node_modules is actually cached.

@oBusk
Copy link
Author

oBusk commented Dec 7, 2025

So by default we will always run prune after install? But this is a waste unless node_modules is actually cached.

True. Would it make sense to merge #188 first, and I can revise this PR to prune by default if you use the new caching, or add option if you use some other caching?

@oBusk
Copy link
Author

oBusk commented Dec 7, 2025

Was looking at refactor this PR to do pruning right after install still, but still keep the postaction around for the caching. But with built in caching, pruning right before the caching is what makes sense.

If we don't want to add pruning right after install by default to potentially aid people who use their own caching, then I'd say there's probably no benefit to any version of this PR.

@oBusk oBusk closed this Dec 7, 2025
@KSXGitHub
Copy link
Collaborator

@oBusk You closed this PR already?

@KSXGitHub
Copy link
Collaborator

Since your last comment did not imply that you intentionally closed this PR. I am going to assume that you pressed the wrong button in haste. Re-opening.

@KSXGitHub KSXGitHub reopened this Dec 7, 2025
@oBusk
Copy link
Author

oBusk commented Dec 7, 2025

@oBusk You closed this PR already?

@KSXGitHub yeah, like I said, with the caching, it makes sense to prune right before the caching. Helpful part of this PR could be to prune by default to help people who use their own caching, but if we don't want to do pruning by default, see no value 😄

@KSXGitHub
Copy link
Collaborator

I see. I guess custom cache steps require custom prune steps. It's not terribly hard. So I'm closing this again.

@KSXGitHub KSXGitHub closed this Dec 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Prune as the post action is not useful

3 participants