fix: guard against require.cache being unavailable - #122
Open
GiHoon1123 wants to merge 1 commit into
Open
Conversation
ExportsCache's has()/get()/set() assumed `require.cache` always exists. When this package is bundled by a tool like esbuild for an ESM target, the bundler substitutes its own `require` shim (without a `.cache` property) for the real Node.js `require`, so indexing into `require.cache` throws instead of falling back to the internal Map the class already maintains for exactly this kind of case. Add a `require.cache` existence check alongside the existing `isBuiltin` check in all three methods, so the fallback to the internal Map also covers a missing `require.cache`, not just a missing entry within it. Note: when require.cache is unavailable, non-core modules are cached in the internal Map instead, same as modules that are unexpectedly missing from require.cache. This means the existing delete-require.cache[...]-to-force-a-reload behavior (see test/require-cache-inval.js) does not apply in that environment - this is an inherent limitation of the environment, not a new gap introduced by this fix. Fixes nodejs#113
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ExportsCache'shas()/get()/set()methods assumerequire.cachealways exists as an object. When this package is bundled by a tool like esbuild for an ESM target, the bundler substitutes its ownrequireshim (no.cacheproperty) for the real Node.jsrequire. Indexing intorequire.cachethen throwsCannot read properties of undefined (reading '...'), instead of falling back to the internal Map the class already uses when a module is missing fromrequire.cache.Fix
Add a
require.cachetruthiness check alongside the existingisBuiltincheck inhas(),get(), andset(). Falls back to the internal Map whenrequire.cacheitself is missing, not just when an entry is missing from it. No change in normal Node.js environments, whererequire.cacheis always present.Known limitation, not introduced by this fix: when
require.cacheis unavailable, non-core modules are cached in the internal Map instead. The existingdelete require.cache[...]-to-force-a-reload trick (seetest/require-cache-inval.js) doesn't work in that environment, since the internal Map has no external invalidation hook.Test
Added
test/require-cache-undefined.test.js. Usesvm+Module.wrap()to loadindex.js's own source with arequireshim that has no.cacheproperty, matching esbuild's shim shape - no need to add esbuild as a dependency.index.js.npm test: lint + 102 tape tests + babel test) passes.Relationship to #121
#121 addresses #120 (webpack replacing
require.resolve, a broader external-module-resolution problem). This PR is a narrower, independent fix for therequire.cache === undefinedcrash in #113 - no overlap in scope.Test plan
Fixes #113