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

fix: Fallback to parentLoad if parsing fails #104

Merged
merged 8 commits into from
Jun 14, 2024

Conversation

timfish
Copy link
Contributor

@timfish timfish commented Jun 12, 2024

Closes #101

Qard
Qard previously approved these changes Jun 12, 2024
@timfish
Copy link
Contributor Author

timfish commented Jun 12, 2024

I added a test that shows how this works around #105 but this PR should help with a lot of scenarios and edge cases and makes import-in-the-middle less likely to completely break setups.

@timfish timfish marked this pull request as ready for review June 12, 2024 20:27
@timfish
Copy link
Contributor Author

timfish commented Jun 12, 2024

The tests ran on my own fork but show this failing on node v16 due to the expected rejection in import-executable.mjs.

Looking at that test, I would expect this to fail on every Node version with my changes.

I guess it's only failing on that version due to how assert.rejects works with top-level-await across different node versions?

@bengl bengl merged commit 88605a7 into nodejs:main Jun 14, 2024
48 checks passed
@timfish timfish deleted the fix/fail-gracefully branch June 15, 2024 11:49
@tlhunter tlhunter mentioned this pull request Jun 18, 2024
tlhunter added a commit that referenced this pull request Jun 18, 2024
$ git log --oneline --no-decorate v1.8.0..HEAD
a3497a9 v1.8.1
88605a7 fix: Fallback to `parentLoad` if parsing fails (#104)
1c6f7b0 fix: Explicitly named exports should be exported over `export *` (#103)
29f51f6 fix: CommonJs bare specifier resolution (#96)
a8b39f7 fix: `parentResolve` is not a function (#100)
c87090d fix: Named export `Hook` missing from types (#92)
@github-actions github-actions bot mentioned this pull request Jul 8, 2024
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.

Should fail gracefully when exports cannot be parsed
4 participants