Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve failing cjs import mock tests #238

Merged
merged 5 commits into from
Sep 7, 2023

Conversation

iambumblehead
Copy link
Owner

@iambumblehead iambumblehead commented Sep 7, 2023

This PR addresses a small issue that popped-up when resolving issues from node v20.6 #237

This is the important patch which detects null and undefined sources, both of which can surprisingly be returned by node's loader,

diff --git a/src/esmockLoader.js b/src/esmockLoader.js
index 69dedd4..e835ec3 100644
--- a/src/esmockLoader.js
+++ b/src/esmockLoader.js
@@ -145,7 +145,8 @@ const load = async (url, context, nextLoad) => {
       if (!/^(commonjs|module)$/.test(nextLoadRes.format))
         return nextLoad(url, context)
 
-      const source = nextLoadRes.source === null
+      // nextLoadRes.source sometimes 'undefined' and other times 'null' :(
+      const source = nextLoadRes.source === null || nextLoadRes.source === undefined
         ? String(await fs.readFile(new URL(url)))
         : String(nextLoadRes.source)
       const hbang = (source.match(hashbangRe) || [])[0] || ''

@iambumblehead
Copy link
Owner Author

This is a low-risk change that could be urgently needed by some users. I am planning to merge and publish this right away.

@iambumblehead iambumblehead merged commit 2fc016c into master Sep 7, 2023
8 checks passed
@iambumblehead iambumblehead deleted the resolve-failing-cjs-import-mock-tests branch September 7, 2023 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants