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

Allow cache file glob customization #714

Closed
bennypowers opened this issue Dec 20, 2021 · 2 comments
Closed

Allow cache file glob customization #714

bennypowers opened this issue Dec 20, 2021 · 2 comments
Labels
type: feature code contributing to the implementation of a feature and/or user facing functionality

Comments

@bennypowers
Copy link

Hello! Netlify is the 🐝 's πŸ§Žβ€β™‚οΈ s. Thanks for maintaining it.

This issue relates to a chronic problem I encounter maintaining NodeJS builds using npm (or yarn), for example, an app might specify npm run build as it's build command for the netlify runner.

The problem occurs when I use patch-package to maintain patches against my node_modules dependencies. If you are unaware, patch-package is a node module which, when run against a package name, creates diff files in patches/*.patch that reflect the project maintainer's changes to her dependencies. An example workflow:

  • project maintainer runs npm install slidem
  • project maintainer manually edits files in node_modules/slidem/**/*.js
  • project maintainer runs npx patch-package slidem
  • patch-package writes patches/slidem+1.2.8.patch, which is a git diff between the canonical slidem@1.2.8 and maintainer's changes
  • project maintainer adds "postinstall": "patch-package" to the project's root package.json
  • Whenever postinstall runs, it applies the patch created earlier.

The problem with this workflow is that the node_modules are cached, even if the patch file changes. This typically has one of two effects:

  1. patch-package fails with non-0 exit code, failing the build
  2. the build succeeds, but the changes in the patch file are not implemented in the final build, potentially breaking the deployed app, which contains project code that expects the patch to-have-been applied.

I described this problem in a support forum thread where @fool kindly offered some suggestions.

I believe that in the second case I listed, his API-call approach will still fail, so the best solution is one in which the user can optionally specify additional files to consider when deciding whether-or-not to bust the cache. i.e. the netlify user should be allowed to specify that changes in patches/*.patch should invalidate the cache just as changes in package-lock.json do.

Would you be interested in reviewing a PR to that effect?

See also #281 #113

@overlordofmu overlordofmu added the type: feature code contributing to the implementation of a feature and/or user facing functionality label Dec 21, 2021
@overlordofmu
Copy link

I want to clarify a nuance here. Technically speaking, the issue isn't the cached node_modules directory itself. The issue is that, when a node_modules directory is cached, we only run npm install again if at least one of the following is true:

  • There is a node version change since the cached build.
  • The package.json file changed from the cached build.

These changes are detected with this function call here:

https://github.com/netlify/build-image/blob/focal/run-build-functions.sh#L187

Which is using this function to detect changes:

https://github.com/netlify/build-image/blob/focal/run-build-functions.sh#L75-L87

Because the patches directory isn't checked, we don't update the cache despite changes occurring via the patches directory and patch-packages.

One solution for this feature request is for the install_deps function code to take into consideration any changes to the patches directory in addition to the test above (changes to the $NODE_VERSION environment variable and the file package.json).

That would ensure that npm install is run when there are differences in the patches directory as well. This would then correctly keep the cached node_modules in the build image in sync with the versions created by patch-packages.

@JGAntunes
Copy link
Contributor

Thanks for opening this issue @bennypowers

As @overlordofmu perfectly described I think that's the exact problem here, which we're tracking via #523 (for reference we also have an internal issue over at https://github.com/netlify/pillar-workflow/issues/138), so I'm going to close it as a duplicate of such (let us know if you think that's not the case though πŸ‘)

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
type: feature code contributing to the implementation of a feature and/or user facing functionality
Projects
None yet
Development

No branches or pull requests

3 participants