Skip to content

Conversation

G-Rath
Copy link
Contributor

@G-Rath G-Rath commented Sep 29, 2024

This (for me at least) was always a bit of a weird one and something we introduced to be on the safe side rather than as a behaviour we'd explicitly decided was a good thing.

From what I understand it was born from webpacker/shakapacker having this behaviour as the default, which then later changed around shakapacker v6/v7 before finally being definitely removed in v8.

Now that it is gone, I think we should axe our task too as we generally always have our dependencies installed by this point and combining these two independent actions is horrible for caching (i.e. in images you typically copy just the package.json + lockfile, do the install command, then copy everything else and proceed), can undo optimizations (i.e. if in the aforementioned docker image I decide to install just my production dependencies, this'll then undo that), and even hide subtle "bugs" (i.e. if we forget to actually install our dependencies, this'll hide that which while technically does avoid an error, means we might not realize there's a larger issue with our pipeline).

@G-Rath
Copy link
Contributor Author

G-Rath commented Jan 17, 2025

It seems like our capistrano based deployments are relying on this, which makes sense (especially since we're not sharing node_modules, which maybe we should 🤔), so we'll want to include something for our Capistrano variant to handle this.

I still think the underlying change is good because for other situations like Docker and Heroku, this results in more work being done

@G-Rath G-Rath marked this pull request as draft January 17, 2025 01:00
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.

3 participants