Skip to content
This repository has been archived by the owner on Jan 25, 2023. It is now read-only.

Fix npm caching issues #510

Closed
wants to merge 1 commit into from
Closed

Conversation

denis-sokolov
Copy link

@denis-sokolov denis-sokolov commented Dec 21, 2020

npm install is optimized for the development process and if node_modules is in a correct state,
npm install will quit quickly after checking the status of the cache.

Removing the conditional will prevent breakages related to stale cache, like in #113.

After including #509, on a mid-size project with a laptop npm install quits after 1.5s for me.

Also closes #523.

npm install is optimized for the development process and if node_modules is in a correct state,
npm install will quit quickly after checking the status of the cache.

Removing the conditional will prevent breakages related to stale cache.
@denis-sokolov denis-sokolov requested a review from a team as a code owner December 21, 2020 06:42
@denis-sokolov denis-sokolov mentioned this pull request Dec 21, 2020
@denis-sokolov denis-sokolov changed the title Run npm install without checking for cache status Fix npm caching issues Jan 20, 2021
@denis-sokolov
Copy link
Author

The caching issues are causing us and our clients numerous annoyances, and I speculate many users can not correctly identify their issues or are not even aware they are not deploying their security upgrades correctly. We need this PR as a first fix for the broader npm caching problem.

The npm-related caching problems issues and PRs receive no response for a while now. How can I further help merge these PRs and fix these issues? @mraerino, I apologize for pinging you directly, but I’ve noticed you being active on the repository and having some background with js tooling. Could you guide me who can help me integrate these corrections? Thank you.

@JGAntunes
Copy link
Contributor

Hey @denis-sokolov 👋 First of all, thank you very much for putting time and effort into this and the other PRs you've opened and apologies for taking so long to get back to you.

We know the way we currently install and cache node_modules is prone to some problems, and we're actively working on improving that. Your issues and PRs have been super helpful in highlighting some of the problems, so thank you for that. We're driving a discussion to assess the impact these changes might cause to our build system and based on that we'll most likely come up with a rollout plan to address some of these issues.

I know all of this is probably not the quick feedback and solution you were expecting, but I just wanted to share that your work and feedback has been heard and we're actively looking into this. I can't commit to a specific timeline, but I promise I'll do my best to keep you updated 👍

@denis-sokolov
Copy link
Author

Don’t worry about me: I am merely inconvenienced by having to press “Clean cache and deploy site”. Worry about the less informed users instead. So it’s good to know you’re aware of the issues, thanks for adding this comment! I’m happy to help, so reach out if I can further help with advice and opinion.

@kaufmo
Copy link

kaufmo commented Mar 3, 2021

I think this would be the better solution: #513

@kitop
Copy link
Contributor

kitop commented Aug 24, 2022

Closing this PR as the it's against xenial branch and the Xenial image is going to be deprecated: https://answers.netlify.com/t/please-read-end-of-support-for-xenial-build-image-everything-you-need-to-know/68239

Feel free to reopen against focal.

@kitop kitop closed this Aug 24, 2022
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
type: bug code to address defects in shipped code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

npm preinstall and postinstall scripts are not run
5 participants