★ wanayoo — archive 1999 https://github.com/nodejs/node/pull/18990/filesNouvelle recherche | Portail wanayoo
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

process: doc-only deprecate non-string env value #18990

Closed
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.
+52 −2
Diff settings

Always

Just for now

Copy path View file
@@ -915,7 +915,6 @@ Type: Runtime
This was never a documented feature.
<a id="DEP0101"></a>
### DEP0101: --with-lttng
@@ -932,6 +931,17 @@ Using the `noAssert` argument has no functionality anymore. All input is going
to be verified, no matter if it is set to true or not. Skipping the verification
could lead to hard to find errors and crashes.
<a id="DEP00XX"></a>
### DEP00XX: process.env string coercion
Type: Documentation-only (supports [`--pending-deprecation`][])
Currently when assigning a property to [`process.env`][], the assigned value is
implicitly converted to a string if it is not a string. This behavior is
deprecated if the assigned value is not a string, boolean, or number. In the
future, such assignment may result in a thrown error. Please convert the
property to a string before assigning it to `process.env`.
[`--pending-deprecation`]: cli.html#cli_pending_deprecation
[`Buffer.allocUnsafeSlow(size)`]: buffer.html#buffer_class_method_buffer_allocunsafeslow_size
[`Buffer.from(array)`]: buffer.html#buffer_class_method_buffer_from_array
@@ -968,6 +978,7 @@ could lead to hard to find errors and crashes.
[`fs.stat()`]: fs.html#fs_fs_stat_path_callback
[`os.networkInterfaces`]: os.html#os_os_networkinterfaces
[`os.tmpdir()`]: os.html#os_os_tmpdir
[`process.env`]: process.html#process_process_env
[`punycode`]: punycode.html
[`require.extensions`]: modules.html#modules_require_extensions
[`setInterval()`]: timers.html#timers_setinterval_callback_delay_args
Copy path View file
@@ -836,6 +836,10 @@ emitMyWarning();
## process.env
<!-- YAML
added: v0.1.27
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/18990
description: Implicit conversion of variable value to string is deprecated.
-->

* {Object}
@@ -877,7 +881,8 @@ console.log(process.env.foo);
```

Assigning a property on `process.env` will implicitly convert the value
to a string.
to a string. **This behavior is deprecated.** Future versions of Node.js may
throw an error when the value is not a string, number, or boolean.

Example:

Copy path View file
@@ -327,6 +327,7 @@ inline Environment::Environment(IsolateData* isolate_data,
trace_sync_io_(false),
abort_on_uncaught_exception_(false),
emit_napi_warning_(true),
emit_env_nonstring_warning_(true),
makecallback_cntr_(0),
should_abort_on_uncaught_toggle_(isolate_, 1),
#if HAVE_INSPECTOR
Copy path View file
@@ -735,6 +735,11 @@ class Environment {
void AddPromiseHook(promise_hook_func fn, void* arg);
bool RemovePromiseHook(promise_hook_func fn, void* arg);
bool EmitNapiWarning();
inline bool EmitProcessEnvWarning() {
bool current_value = emit_env_nonstring_warning_;
emit_env_nonstring_warning_ = false;
return current_value;
}

typedef void (*native_immediate_callback)(Environment* env, void* data);
// cb will be called as cb(env, data) on the next event loop iteration.
@@ -789,6 +794,7 @@ class Environment {
bool trace_sync_io_;
bool abort_on_uncaught_exception_;
bool emit_napi_warning_;
bool emit_env_nonstring_warning_;
size_t makecallback_cntr_;
std::vector<double> destroy_async_id_list_;

Copy path View file
@@ -2659,6 +2659,17 @@ static void EnvGetter(Local<Name> property,
static void EnvSetter(Local<Name> property,
Local<Value> value,
const PropertyCallbackInfo<Value>& info) {
Environment* env = Environment::GetCurrent(info);
if (config_pending_deprecation && env->EmitProcessEnvWarning() &&
!value->IsString() && !value->IsNumber() && !value->IsBoolean()) {
if (ProcessEmitDeprecationWarning(
env,
"Assigning any value other than a string, number, or boolean value "

This comment has been minimized.

@thefourtheye

thefourtheye Mar 6, 2018

Contributor

Nit: I am not a native English speaker, but I believe this message can be presented better. Unfortunately, I am also not sure how to improve it.

This comment has been minimized.

@TimothyGu

TimothyGu Mar 7, 2018

Author Member

I'll remove the second "value" on landing. Hope that helps.

This comment has been minimized.

@thefourtheye

thefourtheye Mar 7, 2018

Contributor

Thanks. That felt redundant to me, when I read that for the first time.

Also, does it sound better if we change the second part to, "If the value is not one of them, use the string representation of it."?

"to a process.env property is deprecated. Please make sure to "
"convert the value to a string before setting process.env with it.",
"DEP00XX").IsNothing())
return;
}
#ifdef __POSIX__
node::Utf8Value key(info.GetIsolate(), property);
node::Utf8Value val(info.GetIsolate(), value);
@@ -0,0 +1,16 @@
'use strict';
const common = require('../common');
const assert = require('assert');

// Flags: --pending-deprecation

common.expectWarning(
'DeprecationWarning',
'Assigning any value other than a string, number, or boolean value to a ' +
'process.env property is deprecated. Please make sure to convert the value ' +
'to a string before setting process.env with it.',
'DEP00XX'
);

process.env.ABC = undefined;
assert.strictEqual(process.env.ABC, 'undefined');
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.