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

add assert function, assert import.meta.resolve returns string or null #236

Closed

Conversation

iambumblehead
Copy link
Owner

closes #234

src/esmockModule.js Outdated Show resolved Hide resolved
@koshic
Copy link
Collaborator

koshic commented Sep 6, 2023

Nice hotfix, thank you!

In general it would be nice to detect --experimental-import-meta-resolve flag once instead of asserting return value (and using expensive throw -> catch mechanics) on every 'resolve' call.

@iambumblehead
Copy link
Owner Author

@koshic locally, the assertion resolves most tests here that do not use the --experimental flag. Possibly import.meta.resolve is broken with and without the --experimental flag and, if import.meta.resolve is fixed upstream, esmock could remove the assertion and error.

I'm first trying to resolve tests without --experimental and, when those are passing, try to resolve issues that occur when --experimental is used.

@koshic
Copy link
Collaborator

koshic commented Sep 6, 2023

Sure, this is why I approved that PR :)

@iambumblehead
Copy link
Owner Author

In general it would be nice to detect --experimental-import-meta-resolve flag once instead of asserting return value (and using expensive throw -> catch mechanics) on every 'resolve' call.

yes you are right

@iambumblehead
Copy link
Owner Author

Sure, this is why I approved that PR :)

My mental powers are not strong today and I misunderstood several things but am reading more carefully. Thanks for your patience.

@iambumblehead
Copy link
Owner Author

closing this in favor of #237

@iambumblehead iambumblehead deleted the assert-valid-immport.meta.resolve-results branch September 7, 2023 19:51
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.

Breakage in Node.js v20.6.0
3 participants