fix(ext/node): emit DEP0192 deprecation warning when _tls_common is required#33819
Conversation
…equired Enables tests/node_compat/runner/suite/test/parallel/test-warn-tls-common-deprecation.js Co-authored-by: Divy Srivastava <[email protected]>
fibibot
left a comment
fibibot
left a comment
There was a problem hiding this comment.
LGTM. Tiny, focused fix — 7-line process.emitWarning for require("_tls_common") matching Node's DEP0192 exactly, plus the test enable. No surprises.
What I verified
Warning shape (ext/node/polyfills/01_require.js:1770-1775):
if (request === "_tls_common") {
process.emitWarning(
"The _tls_common module is deprecated. Use `node:tls` instead.",
"DeprecationWarning",
"DEP0192",
);
}Matches the upstream test-warn-tls-common-deprecation.js assertion byte-for-byte:
common.expectWarning('DeprecationWarning',
'The _tls_common module is deprecated. Use `node:tls` instead.', 'DEP0192');Message, name, and code all align with Node's expectation. Node's process.emitWarning deduplicates by code (per Node docs), so even though this branch fires inside loadNativeModule on every require("_tls_common") call, the user only sees the warning once per process — same observable behavior as Node's own _tls_common.js top-level emit.
Placement (01_require.js:1770): the warning fires before new Module(request) and before nativeModuleExports[request] is loaded. This matches Node's order — _tls_common.js calls process.emitWarning at the top of the module, then re-exports tls. ✓
Test enable (tests/node_compat/config.jsonc:3469): parallel/test-warn-tls-common-deprecation.js added in alphabetical order between test-vm-util-lazy-properties.js and test-warn-sigprof.js. ✓
CI
All release-mode test categories pass across all 5 platforms: 15/15 test node_compat shards, 5/5 test unit_node shards. Debug-mode shards still in flight (waiting on builds), but for a 7-line process.emitWarning addition with no platform-specific surface, the green release matrix across all platforms is sufficient confidence — debug-vs-release wouldn't surface a behavioral difference here.
55 success, 10 in-flight (build debug × 6 platforms + build release linux + lint debug macos/windows + miri), 1 skipped, 0 fail.
…ous DEP0192 The wrap_eval_code helper sets every builtinModule on globalThis via process.getBuiltinModule(m). That caused _tls_common to be loaded (and the newly-added DEP0192 warning to fire) for every child process spawned with -e/-p, breaking tests that expect empty stderr. Add _tls_common to the filter so the deprecated module is never silently pre-loaded in eval contexts; user code that explicitly require()s it still triggers the warning correctly via loadNativeModule. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Head branch was pushed to by a user without write access
|
The two CI failures ( This is unrelated to the PR changes. All release shards (5 platforms × all test categories) and all other debug shards passed. Please re-run the failed job or approve for merge. |
…equired (denoland#33819) Enables `test-warn-tls-common-deprecation` in node_compat suite. Co-authored-by: Divy Srivastava <[email protected]>
3 participants
Summary
Enables
test-warn-tls-common-deprecationin node_compat suite.Test plan
cargo test --test node_compat -- test-warn-tls-common-deprecation