Skip to content

repl: avoid deprecated require.extensions in tab completion #58653

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

gulbaki
Copy link

@gulbaki gulbaki commented Jun 9, 2025

This PR replaces the usage of require.extensions in the REPL autocompletion logic with Module._extensions.

require.extensions has been runtime-deprecated under DEP0039, and its usage triggers a warning in recent Node.js versions. Since require.extensions is just a user-land alias for Module._extensions, we can safely access the same object via Module._extensions without any behavioral change or deprecation warning.

Changes:

  • this.context.require.extensions → Module._extensions in the REPL complete() function
  • Adds an inline comment explaining the deprecation and replacement
  • No functional behavior change

Fixes: #58641

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. repl Issues and PRs related to the REPL subsystem. labels Jun 9, 2025
Copy link
Member

@dario-piotrowicz dario-piotrowicz left a comment

Choose a reason for hiding this comment

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

Very nice, thanks for the PR @gulbaki 😃🙏

I've left a couple of small comments

Copy link
Contributor

@Ethan-Arrowood Ethan-Arrowood left a comment

Choose a reason for hiding this comment

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

Could you also include a test for this? Here is an example of how I tested process warnings: https://github.com/nodejs/node/blob/main/test/parallel/test-process-warnings.mjs

lib/repl.js Outdated
// `require.extensions` is runtime-deprecated (DEP0039).
// Use `Module._extensions` to access the same extension map
// without triggering the deprecation warning.
const Module = require('module');
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider moving this to the top of the file.

@marco-ippolito
Copy link
Member

marco-ippolito commented Jun 10, 2025

Could you also include a test for this? Here is an example of how I tested process warnings: https://github.com/nodejs/node/blob/main/test/parallel/test-process-warnings.mjs

It's not runtime deprecated (doc only) so it's not possible to capture the experimental warning

Copy link
Member

@dario-piotrowicz dario-piotrowicz left a comment

Choose a reason for hiding this comment

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

LGTM 🙂

@gulbaki gulbaki requested a review from Ethan-Arrowood June 11, 2025 10:45
Copy link

codecov bot commented Jun 11, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.14%. Comparing base (905a722) to head (5cb5079).
Report is 33 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #58653      +/-   ##
==========================================
- Coverage   90.15%   90.14%   -0.01%     
==========================================
  Files         636      636              
  Lines      188040   188057      +17     
  Branches    36903    36895       -8     
==========================================
- Hits       169535   169532       -3     
- Misses      11239    11278      +39     
+ Partials     7266     7247      -19     
Files with missing lines Coverage Δ
lib/repl.js 95.13% <100.00%> (ø)

... and 36 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@marco-ippolito marco-ippolito added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 11, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 11, 2025
@nodejs-github-bot
Copy link
Collaborator

@juanarbol juanarbol added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Jun 13, 2025
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. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. needs-ci PRs that need a full CI run. repl Issues and PRs related to the REPL subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

repl: don't rely on require.extensions
9 participants