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

Test import nodefs promises #282

Merged
merged 7 commits into from
Jan 14, 2024
Merged

Conversation

iambumblehead
Copy link
Owner

@iambumblehead iambumblehead commented Dec 22, 2023

re #281

@iambumblehead
Copy link
Owner Author

iambumblehead commented Dec 29, 2023

I don't fully understand the probalm and solution. Things could be studied more but I would rather move on as all tests are passing.

Here is generally what I think I know. This line in the load hook stripped the entire query string from the url returned by the load hook

url = url.replace(esmkgdefsAndAfterRe, '')
/* ... */
return nextLoad(url, context)

Both load and resolve hooks return module url values. The load hook url is not always required, but the resolve hook url is required. Usually, when a module loads another module, the parent module url comes from a pior resolve hook call, but in our case here, the url came from the load hook and adding the import tree id onto the load url resolved the issue. The resolve hook is then called with that url and continues mocking that import tree.

@iambumblehead
Copy link
Owner Author

maybe it has been little bit too long of a wait waiting for review... merging this

@iambumblehead iambumblehead merged commit 44fda68 into main Jan 14, 2024
10 checks passed
@iambumblehead iambumblehead deleted the test-import-nodefs-promises branch January 14, 2024 03:31
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