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

module: fix ERR_REQUIRE_ESM for parentPath null #40145

Closed

Conversation

guybedford
Copy link
Contributor

Just came across this one when running require('./cjs.js') in a REPL in a project with "type": "module" where the ERR_REQUIRE_ESM was getting masked by an ERR_INVALID_ARG_TYPE error error, followed by the terrible realization it was probably my own fault!

@nodejs-github-bot nodejs-github-bot added errors Issues and PRs related to JavaScript errors originated in Node.js core. needs-ci PRs that need a full CI run. labels Sep 18, 2021
Copy link
Contributor

@cjihrig cjihrig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but is it possible to add a regression test?

@guybedford
Copy link
Contributor Author

@cjihrig I actually argue against testing cases like this - because if every code path needed to be negatively tested on its completion in JS, every single member expression, function argument would need to be tested for its null value. Also I'm tired!

@nodejs-github-bot

This comment has been minimized.

@guybedford guybedford added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 21, 2021
@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot
Copy link
Collaborator

guybedford added a commit that referenced this pull request Sep 28, 2021
PR-URL: #40145
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@guybedford
Copy link
Contributor Author

Landed in 481c160.

@guybedford guybedford closed this Sep 28, 2021
@guybedford guybedford deleted the err-require-esm-parent-fix branch September 28, 2021 14:30
targos pushed a commit that referenced this pull request Oct 4, 2021
PR-URL: #40145
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. errors Issues and PRs related to JavaScript errors originated in Node.js core. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants