gjaldon / heroku-buildpack-phoenix-static

A Heroku buildpack for building Phoenix's static assets
MIT License
229 stars 224 forks source link

Only install production npm dependencies #120

Closed marcandre closed 2 years ago

marcandre commented 2 years ago

We are hosted on gigalixir and were quite surprised to see that dev-only dependencies we had put at the root level (even though it's no longer the standard) were being installed, by default, and without the possibility of overriding this.

Would you consider installing only production dependencies?

gjaldon commented 2 years ago

You're right we should use that. What happens though if older versions of npm are used and don't yet support --only. Did a quick search and apparently the flag in older versions would be --production.

Need to watch out for those since there's no versioning with buildpacks. And if there are any users with the older versions of npm using it in Heroku/Gigalixir, their deploys would break. Not sure though if it's likely any deployments would still be using older NPM versions.

marcandre commented 2 years ago

And if there are any users with the older versions of npm using it in Heroku/Gigalixir, their deploys would break.

I didn't check with older npm, but modern npm seem to ignore unknown options, so I think the deploys would not break.

We could even call it with both --only=prod and --production

marcandre commented 2 years ago

After a bit of thought, the build pack is run when building the release, so dev dependencies are probably needed. Moreover the risk for existing deploys to break would be too big. Our initial issue was actually that we added a package.json at the root level, which this buildpack considered in priority over that of assets/. Once we specified assets_path, things went back to normal.

In short, I'm no longer thinking this is a good idea, sorry for the noise.

gjaldon commented 2 years ago

No problem, @marcandre!