feat(runtime): populate require.cache #9841
Conversation
Codecov Report
@@ Coverage Diff @@
## master #9841 +/- ##
=======================================
Coverage 64.46% 64.47%
=======================================
Files 289 289
Lines 12327 12333 +6
Branches 3049 3053 +4
=======================================
+ Hits 7947 7952 +5
- Misses 3740 3741 +1
Partials 640 640
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Oh, very cool! I never thought we'd actually add support for this property, but this is actually pretty nice. Lays a path for respecting deletions as well at some point 👍
Mind updating the changelog as well (not that I'll be making a release in a few minutes, so you'll get a conflict in the changelog)
|
@SimenB, hold on. I'm going to refactor some piece. There's no need for separate cache object when proxy is used. |
|
Ah ok, cool! 👍 |
|
@antongolub I think this looks great, ready to go? |
|
@SimenB, Iet's merge. |
|
Thanks! |
|
@antongolub hmm, looking at CI I'm seeing this:
Ideas? 😀 Wait, might only be in #9853, I've probably messed it up somehow |
|
No, happens on master as well. Happens in a few files, but the worst one is, by far, |
Couple of things here: **require.cache** In version `v25.5.0`, jest introduced an [implementation](jestjs/jest#9841) of their own to the `require.cache` object. It seems that it doesn't handle caching `json` modules properly, which causes our code to fail because `mod.paths` is `undefined` when we are querying for `json` files that were required (for example `package.json` or `cloud-assembly.schema.json`). https://github.com/aws/aws-cdk/blob/07fe642e38118e24837b492fa182737fc41bb429/packages/%40aws-cdk/core/lib/private/runtime-info.ts#L66 ```console TypeError: Cannot read property 'map' of undefined ``` The "fix" was to add a null check to prevent failure when looking up modules who don't have the `paths` property defined. Note that this was only observed in test environments using `jest > v25.5.0`, not during actual runtime of `cdk synth`. This is because `jest` manipulates the built-in `require` module of `nodejs`. **graceful-fs** In version `v25.5.0`, jest [added](jestjs/jest#9443) a dependency on the `graceful-fs` package. The version they depend on (`4.2.4`) differs from the version that we bring (`4.2.3`). This caused the `graceful-fs` dependency that comes from `jest` no to be deduped by node at installation. In turn, this caused multiple copies of the library to be installed on disk. The `graceful-fs` package monkey patches the `process.chdir` and `process.cwd` functions with a cached version for better performance. For reasons not completely clear to us, the existence of multiple copies of the module, caused `process.cwd()` to return incorrect cached results, essentially always returning the directory that the process was started from, without consideration to calls to `process.chdir`. This broke `decdk` (and possibly many more) tests which rely on `process.chdir`. Fixes #7657
|
In Tailwind CSS we rely on being able to delete keys in https://github.com/tailwindcss/tailwindcss/blob/master/src/index.js#L54 This change is causing us to see warnings in our test runs now. Nothing is actually broken but it would be great to get rid of the warnings — any suggestions on the best way to do it? |
Couple of things here: **require.cache** In version `v25.5.0`, jest introduced an [implementation](jestjs/jest#9841) of their own to the `require.cache` object. It seems that it doesn't handle caching `json` modules properly, which causes our code to fail because `mod.paths` is `undefined` when we are querying for `json` files that were required (for example `package.json` or `cloud-assembly.schema.json`). https://github.com/aws/aws-cdk/blob/07fe642e38118e24837b492fa182737fc41bb429/packages/%40aws-cdk/core/lib/private/runtime-info.ts#L66 ```console TypeError: Cannot read property 'map' of undefined ``` The "fix" was to add a null check to prevent failure when looking up modules who don't have the `paths` property defined. Note that this was only observed in test environments using `jest > v25.5.0`, not during actual runtime of `cdk synth`. This is because `jest` manipulates the built-in `require` module of `nodejs`. **graceful-fs** In version `v25.5.0`, jest [added](jestjs/jest#9443) a dependency on the `graceful-fs` package. The version they depend on (`4.2.4`) differs from the version that we bring (`4.2.3`). This caused the `graceful-fs` dependency that comes from `jest` no to be deduped by node at installation. In turn, this caused multiple copies of the library to be installed on disk. The `graceful-fs` package monkey patches the `process.chdir` and `process.cwd` functions with a cached version for better performance. For reasons not completely clear to us, the existence of multiple copies of the module, caused `process.cwd()` to return incorrect cached results, essentially always returning the directory that the process was started from, without consideration to calls to `process.chdir`. This broke `decdk` (and possibly many more) tests which rely on `process.chdir`. Fixes #7657
|
No really good way of suppressing the warning. I'm currently leaning towards silencing the warning, and maybe add it back for Jest 26. The code you link to can be supported though (deleting single entries from the cache), we just haven't done so. We can discuss this is #9916 though |
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
This PR brings more expected behavior for
require.cache: populates with loaded modules data, but protects from modification.In my specific practical case I need to verify the huge app context initialization (app runner, low level api injections, hooks, DI IoC container, and so on). Checking all side effects requires tons of code, but in some cases this annoying routine can be replaced by a simple module loading check.
Relates: #6725, #6066, #5741, #5120.
Test plan