Use Node.js compile cache#193
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR enables Node.js compile cache functionality to improve module instantiation performance. The changes add compile cache support in both CommonJS and ESM entry points while disabling it during testing to ensure test environment consistency.
- Adds
enableCompileCachecalls to both ESM and CommonJS entry points for performance optimization - Configures test environment to disable compile cache for consistent test results
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| lib/esm.mjs | Imports and enables Node.js compile cache with optional chaining |
| lib/cjs.cjs | Enables compile cache using require() approach for CommonJS compatibility |
| .github/workflows/test.yml | Disables compile cache in CI environment for consistent testing |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #193 +/- ##
==========================================
+ Coverage 96.03% 96.24% +0.20%
==========================================
Files 36 36
Lines 2143 2155 +12
Branches 84 86 +2
==========================================
+ Hits 2058 2074 +16
+ Misses 83 81 -2
+ Partials 2 0 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Ooh. I didn't account for the engines. Give me a minute |
|
This works, but it might not be the cleanest approach. Open to better ideas if anyone has suggestions |
voxpelli
left a comment
There was a problem hiding this comment.
Since at least I personally often nest calls to npm-run-all2 we maybe need / should do a module.flushCompileCache() before spawning child processes?
|
Any noticeable differences in performance? Refreshing my familiarity with the implications of doing this. |
|
Sorry I didn't get to this this week, but its still on my todo. |
|
I was really busy as well. I'll upload before/after on the weekend |
|
Look at maybe landing this. |
|
The problem with this PR is that by the time the compile cache method calls, the import graph is already open. I could switch to running it before the main import but then I would need t switch to dynamic import which has it OWN slowdowns. |
|
I tired to land this in a time box and couldn't come to anything I was convinced was wroth landing: But I'm open to a working / proven solution if someone else can figure it out. |
Resolves #177