search

unfold / heroku-buildpack-pnpm

Run PNPM install on Heroku
MIT License
38 stars 39 forks source link

Fix issues with devDependencies to support Typescript projects #20

Closed TheSecurityDev closed 1 year ago

TheSecurityDev commented 2 years ago

This pull request changes how PNPM installs devDependencies to behave more like the official NodeJS buildpack.

It temporarily overrides the NODE_ENV variable when running pnpm install to force it to install devDependencies. Then during the prune step it runs pnpm prune --prod to remove the devDependencies. This allows building Typescript projects without moving typescript out of devDependencies.

shadoath commented 2 years ago

Very nice update I think the options you added in this are what we need.

It might be worth adding some references to the README about the use of the new env values and how they can be used to customize the build process.

TheSecurityDev commented 2 years ago

Very nice update I think the options you added in this are what we need.

It might be worth adding some references to the README about the use of the new env values and how they can be used to customize the build process.

I did update the README to explain the new behavior, but there's no way to customize the build process any further than already possible. The NODE_ENV variable is only overridden temporarily for the install command and shouldn't change any other behavior.

shadoath commented 2 years ago

I had been browsing on my phone and didn't take enough time to search the whole repo for the ENV values being used. If I could approve this, I would.

TheSecurityDev commented 2 years ago

I had been browsing on my phone and didn't take enough time to search the whole repo for the ENV values being used. If I could approve this, I would.

So I basically just copied that part from the NPM prune function, and then removed some irrelevant checks. I see now that there's also a YARN_PRODUCTION variable, so perhaps I should have used a separate PNPM_PRODUCTION instead. Or maybe it's unnecessary and could be removed.

shadoath commented 2 years ago

@einarlove or @mharnang would one of you two be willing to review and merge if it all looks good?

einarlove commented 2 years ago

@hampusborgos ☝️

lunatupini commented 1 year ago

Very nice PR! Thanks a lot

dduboncr commented 1 year ago

please lets merge this, would help me a lot. nice job

TheSecurityDev commented 1 year ago

please lets merge this, would help me a lot. nice job

It's been several months and whoever is maintaining this doesn't seem to care, so I would recommend using my fork here:

https://github.com/TheSecurityDev/heroku-buildpack-nodejs-pnpm

shadoath commented 1 year ago

I wish I could merge this

hampusborgos commented 1 year ago

Merged, sorry for taking so long! :)