nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
106.55k stars 29.04k forks source link

node -e invocation that doesn't mention sys yields deprecation warning #40871

Open cemerick opened 2 years ago

cemerick commented 2 years ago

Version

v17.1.0

Platform

Linux t440p 5.4.0-89-generic #100-Ubuntu SMP Fri Sep 24 14:50:10 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

No response

What steps will reproduce the bug?

$ node --version
v17.1.0
$ node --trace-deprecation -e 'let env = (function () { return Object.getOwnPropertyNames(this).reduce((g, k) => { g[k] = this[k]; return g }, { }) })();'
(node:465563) [DEP0025] DeprecationWarning: sys is deprecated. Use util instead.
    at node:sys:28:9
    at NativeModule.compileForInternalLoader (node:internal/bootstrap/loaders:312:7)
    at NativeModule.compileForPublicLoader (node:internal/bootstrap/loaders:252:10)
    at loadNativeModule (node:internal/modules/cjs/helpers:49:9)
    at Function.Module._load (node:internal/modules/cjs/loader:804:15)
    at Module.require (node:internal/modules/cjs/loader:999:19)
    at get (node:internal/modules/cjs/helpers:175:33)
    at [eval]:1:96
    at Array.reduce (<anonymous>)
    at [eval]:1:66

How often does it reproduce? Is there a required condition?

Always reproducible.

What is the expected behavior?

No deprecation warning,

What do you see instead?

No response

Additional information

The provided code:

  1. doesn't reference sys, and uses no libraries
  2. loads and runs fine when put into a source file or loaded from stdin:
$ node - <<END
let env = (function () { return Object.getOwnPropertyNames(this).reduce((g, k) => { g[k] = this[k]; return g }, { }) })();
END
$
Trott commented 2 years ago

Here's a simpler replication:

node -e 'Object.getOwnPropertyNames(this).forEach((foo) => this[foo])'

The issue is that the value of this is different. If you put console.log(this) into a file and run it, you get {}. But if you do node -e 'console.log(this)', you get the global object.

I suspect this is Working-As-Expected, as this is sensitive to context.

cemerick commented 2 years ago

I suspect this is Working-As-Expected, as this is sensitive to context.

tbc, I the varying character of this isn't the issue. The code in question (either my original, or the simplified replication) emits a deprecation warning through no fault of its own, so to speak. Surely internal use of sys (that is apparently implicated only when loading code via -e) shouldn't produce warnings that end users can't possibly address?

cemerick commented 2 years ago

Maybe this is what @Trott was getting at with the properties of this being different: "sys" is a property name on this, and simply dereferencing this["sys"] triggers the deprecation warning because the module is implicitly loaded/required?

Trott commented 2 years ago

Maybe this is what @Trott was getting at with the properties of this being different: "sys" is a property name on this, and simply dereferencing this["sys"] triggers the deprecation warning because the module is implicitly loaded/required?

Yeah, that's what I was referring to. Sorry that I wasn't clear about it and/or didn't read what you initially wrote closely enough. Anyway, the deprecation warning occurs the first time the sys module is loaded. So foo = this['sys'] causes the deprecation warning. Whether or not it should in this context is a reasonable question, but the answer may very likely be "yes". (The user is using a deprecated module, so....)

targos commented 2 years ago

One solution could be to omit deprecated modules when we fill the global object for node -e and the REPL.

Trott commented 2 years ago

One solution could be to omit deprecated modules when we fill the global object for node -e and the REPL.

I like this idea, although I imagine it would be a breaking change.