Commit 23e5fce
authored
Fix pnpm interaction with custom cache (#1522)
* Fix pnpm interaction with custom cache
When a `cacheDirectories` field is set in `package.json`, this triggers [custom cache behavior](https://devcenter.heroku.com/articles/nodejs-classic-buildpack-builds#custom-caching) in the buildpack. This is often used with tools that produce their own specialized caches like Next.js which can cache asset compilation results.
Since custom cache behavior means the user is taking full control of what needs to be cached, the `node_modules` folder is typically added to the cache directory list to ensure that installed modules are also cached. For npm and Yarn this doesn't present a problem but, for pnpm, [the `node_modules` folder is used differently](https://pnpm.io/motivation#creating-a-non-flat-node_modules-directory) and, if the folder already exists (e.g.; on cache restore), it will cause the build to halt with an `ERR_PNPM_ABORTED_REMOVE_MODULES_DIR_NO_TTY` error.
After examining the custom cache code, I'm certain that even how npm and Yarn handle this scenario is incorrect. The user should never cache the `node_modules` folder. What should happen instead is (for default or custom cache behavior), the npm cache, Yarn cache, and pnpm store are what should be always cached unless `NODE_MODULES_CACHE=false`. For now, I'm just going to address this pnpm case though. This PR makes the following changes:
### Custom Cache Behavior with pnpm
- if `node_modules` is specified, it will be ignored
- all other cache directories will be processed
- the pnpm store will be inserted into the custom cache list
### pnpm Reporter
A `PNPM_INSTALL_REPORTER` environment variable was introduced to allow for more detailed pnpm output to be produced during install. If set it, the value of this environment variable will be used to append the [`--reporter`](https://pnpm.io/cli/install#--reportername) argument when `pnpm install` is executed. This is used by the tests in this PR to ensure that the pnpm store is functioning correctly, but it could also be useful when debugging builds where pnpm installs are failing unexpectedly.
* Use explicit comparison of `PNPM` variable to `true`
* Use a more precise pattern for matching the `node_modules` folder
* Fix typo
* Validate the PNPM_INSTALL_REPORTER env var
* Update CHANGELOG.md
Signed-off-by: Colin Casey <[email protected]>
* Fix test output validations
---------
Signed-off-by: Colin Casey <[email protected]>1 parent 15a348b commit 23e5fce
File tree
8 files changed
+105
-12
lines changed- bin
- lib
- test
- fixtures/pnpm-custom-cache
- data
8 files changed
+105
-12
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2 | 2 | | |
3 | 3 | | |
4 | 4 | | |
| 5 | + | |
5 | 6 | | |
6 | 7 | | |
7 | 8 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
339 | 339 | | |
340 | 340 | | |
341 | 341 | | |
342 | | - | |
| 342 | + | |
343 | 343 | | |
344 | 344 | | |
345 | 345 | | |
| |||
414 | 414 | | |
415 | 415 | | |
416 | 416 | | |
417 | | - | |
| 417 | + | |
418 | 418 | | |
419 | 419 | | |
420 | 420 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
108 | 108 | | |
109 | 109 | | |
110 | 110 | | |
| 111 | + | |
111 | 112 | | |
112 | | - | |
| 113 | + | |
113 | 114 | | |
114 | | - | |
| 115 | + | |
115 | 116 | | |
116 | 117 | | |
117 | | - | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
118 | 121 | | |
119 | 122 | | |
120 | 123 | | |
| |||
126 | 129 | | |
127 | 130 | | |
128 | 131 | | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
129 | 141 | | |
130 | 142 | | |
131 | 143 | | |
| |||
192 | 204 | | |
193 | 205 | | |
194 | 206 | | |
| 207 | + | |
195 | 208 | | |
196 | | - | |
| 209 | + | |
197 | 210 | | |
198 | | - | |
| 211 | + | |
199 | 212 | | |
200 | 213 | | |
201 | | - | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
202 | 217 | | |
203 | 218 | | |
204 | 219 | | |
| |||
207 | 222 | | |
208 | 223 | | |
209 | 224 | | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
210 | 231 | | |
211 | 232 | | |
212 | 233 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
305 | 305 | | |
306 | 306 | | |
307 | 307 | | |
308 | | - | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
| 313 | + | |
| 314 | + | |
| 315 | + | |
| 316 | + | |
| 317 | + | |
| 318 | + | |
| 319 | + | |
| 320 | + | |
| 321 | + | |
| 322 | + | |
309 | 323 | | |
310 | 324 | | |
311 | 325 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
557 | 557 | | |
558 | 558 | | |
559 | 559 | | |
560 | | - | |
| 560 | + | |
561 | 561 | | |
562 | 562 | | |
563 | 563 | | |
| |||
963 | 963 | | |
964 | 964 | | |
965 | 965 | | |
966 | | - | |
| 966 | + | |
967 | 967 | | |
968 | 968 | | |
969 | 969 | | |
970 | 970 | | |
971 | 971 | | |
972 | | - | |
| 972 | + | |
973 | 973 | | |
974 | 974 | | |
975 | 975 | | |
| |||
1716 | 1716 | | |
1717 | 1717 | | |
1718 | 1718 | | |
| 1719 | + | |
| 1720 | + | |
| 1721 | + | |
| 1722 | + | |
| 1723 | + | |
| 1724 | + | |
| 1725 | + | |
| 1726 | + | |
| 1727 | + | |
| 1728 | + | |
| 1729 | + | |
| 1730 | + | |
| 1731 | + | |
| 1732 | + | |
| 1733 | + | |
| 1734 | + | |
| 1735 | + | |
| 1736 | + | |
| 1737 | + | |
| 1738 | + | |
| 1739 | + | |
| 1740 | + | |
1719 | 1741 | | |
1720 | 1742 | | |
1721 | 1743 | | |
| |||
0 commit comments