Skip to content

fix(ext/node): fix Module._resolveLookupPaths and require.resolve compat#33261

Open
bartlomieju wants to merge 1 commit intomainfrom
fix/require-resolve-builtins-null
Open

fix(ext/node): fix Module._resolveLookupPaths and require.resolve compat#33261
bartlomieju wants to merge 1 commit intomainfrom
fix/require-resolve-builtins-null

Conversation

@bartlomieju
Copy link
Copy Markdown
Member

Summary

  • Module._resolveLookupPaths now returns null for built-in modules, matching Node.js behavior
  • require.resolve / require.resolve.paths validate argument types (ERR_INVALID_ARG_TYPE)
  • require.resolve with options.paths validates path entries and resolves relative paths against CWD
  • require.resolve('node:unknown') throws MODULE_NOT_FOUND instead of silently returning
  • Fix panic in op_require_resolve_lookup_paths when parent_filename is empty
  • Treat "." as a relative request in op_require_is_request_relative
  • Enable 3 Node.js compat tests: test-require-resolve, test-require-resolve-invalid-paths, test-require-resolve-opts-paths-relative

Closes #33258

Test plan

  • 3 previously-failing Node.js compat tests now pass (./x test-compat test-require-resolve)
  • tools/format.js and tools/lint.js clean

🤖 Generated with Claude Code

Fixes several Node.js compatibility issues in the CJS module resolver:

- `Module._resolveLookupPaths` now returns `null` for built-in modules,
  matching Node.js behavior. Libraries like requizzle (used by jsdoc)
  rely on this to detect native modules.
- `require.resolve` and `require.resolve.paths` now validate argument
  types, throwing `ERR_INVALID_ARG_TYPE` for non-string arguments.
- `require.resolve` with `options.paths` now validates path entries are
  strings and resolves relative paths against CWD.
- `require.resolve('node:unknown')` now throws `MODULE_NOT_FOUND`
  instead of silently returning the invalid specifier.
- Fix panic in `op_require_resolve_lookup_paths` when `parent_filename`
  is empty (e.g. REPL or fakeParent contexts).
- Treat `"."` as a relative request in `op_require_is_request_relative`.

Closes #33258

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Copy link
Copy Markdown

@miracatbot miracatbot left a comment

Choose a reason for hiding this comment

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

Looks good to me.

The built-in handling now matches Node much better: _resolveLookupPaths() returns null for builtins, valid node: builtins resolve correctly, and unknown node: specifiers now fail with MODULE_NOT_FOUND instead of being treated as valid.

The additional request/options validation and the empty parent.filename handling also look like the right fixes for the compat cases covered by the newly enabled tests.

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.

npm package jsdoc throws error in Deno but not in Node

2 participants