remove support for features

nodejs · Apr 13, 2022 · 9917174 · 9917174
1 parent cdff4f1
commit 9917174
Show file tree

Hide file tree

Showing 10 changed files with 11 additions and 156 deletions.
diff --git a/src/js_native_api.h b/src/js_native_api.h
@@ -562,15 +562,6 @@ NAPI_EXTERN napi_status napi_object_freeze(napi_env env, napi_value object);
 NAPI_EXTERN napi_status napi_object_seal(napi_env env, napi_value object);
 #endif  // NAPI_VERSION >= 8
 
-#ifdef NAPI_EXPERIMENTAL
-NAPI_EXTERN napi_status node_api_get_features(napi_env env,
-                                              node_api_features* features);
-NAPI_EXTERN napi_status node_api_add_features(napi_env env,
-                                              node_api_features features);
-NAPI_EXTERN napi_status node_api_remove_features(napi_env env,
-                                                 node_api_features features);
-#endif  // NAPI_EXPERIMENTAL
-
 EXTERN_C_END
 
 #endif  // SRC_JS_NATIVE_API_H_
diff --git a/src/js_native_api_types.h b/src/js_native_api_types.h
@@ -100,23 +100,6 @@ typedef enum {
 //   * the definition of `napi_status` in doc/api/n-api.md to reflect the newly
 //     added value(s).
 
-#ifdef NAPI_EXPERIMENTAL
-// Feature bit flags allow to tweak internal behavior of the Node-API functions.
-// E.g. we can use it to fix old bugs or change result status codes.
-// The node_api_features_default defines the default set of features per
-// Node-API version. It is used during the native module initialization.
-// More specifically, NAPI_MODULE_X macro uses NAPI_DEFAULT_FEATURES that is set
-// to node_api_features_default. To override the default behavior define
-// NAPI_DEFAULT_FEATURES before the node_api.h is included.
-typedef enum {
-  node_api_features_none = 0,
-  // Fix use of napi_key_configurable in napi_get_all_property_names.
-  node_api_features_use_configurable_key_filter = 1 << 0,
-  // The default features per Node-API version.
-  node_api_features_default = node_api_features_use_configurable_key_filter
-} node_api_features;
-#endif
-
 typedef napi_value (*napi_callback)(napi_env env, napi_callback_info info);
 typedef void (*napi_finalize)(napi_env env,
                               void* finalize_data,

diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc
@@ -936,14 +936,8 @@ napi_status napi_get_all_property_names(napi_env env,
         filter | v8::PropertyFilter::ONLY_ENUMERABLE);
   }
   if (key_filter & napi_key_configurable) {
-    if (env->HasFeature(node_api_features_use_configurable_key_filter)) {
-      filter = static_cast<v8::PropertyFilter>(
-          filter | v8::PropertyFilter::ONLY_CONFIGURABLE);
-    } else {
-      // The code path before the bug fix
-      filter = static_cast<v8::PropertyFilter>(
-          filter | v8::PropertyFilter::ONLY_WRITABLE);
-    }
+    filter = static_cast<v8::PropertyFilter>(
+        filter | v8::PropertyFilter::ONLY_CONFIGURABLE);
   }
   if (key_filter & napi_key_skip_strings) {
     filter = static_cast<v8::PropertyFilter>(filter |
@@ -3216,25 +3210,3 @@ napi_status napi_is_detached_arraybuffer(napi_env env,
 
   return napi_clear_last_error(env);
 }
-
-NAPI_EXTERN napi_status node_api_get_features(napi_env env,
-                                              node_api_features* features) {
-  CHECK_ENV(env);
-  CHECK_ARG(env, features);
-  *features = env->features;
-  return napi_clear_last_error(env);
-}
-
-NAPI_EXTERN napi_status node_api_add_features(napi_env env,
-                                              node_api_features features) {
-  CHECK_ENV(env);
-  env->features = static_cast<node_api_features>(env->features | features);
-  return napi_clear_last_error(env);
-}
-
-NAPI_EXTERN napi_status node_api_remove_features(napi_env env,
-                                                 node_api_features features) {
-  CHECK_ENV(env);
-  env->features = static_cast<node_api_features>(env->features & ~features);
-  return napi_clear_last_error(env);
-}
diff --git a/src/js_native_api_v8.h b/src/js_native_api_v8.h
@@ -52,11 +52,8 @@ class RefTracker {
 }  // end of namespace v8impl
 
 struct napi_env__ {
-  explicit napi_env__(v8::Local<v8::Context> context,
-                      node_api_features features)
-      : isolate(context->GetIsolate()),
-        context_persistent(isolate, context),
-        features(features) {
+  explicit napi_env__(v8::Local<v8::Context> context)
+      : isolate(context->GetIsolate()), context_persistent(isolate, context) {
     CHECK_EQ(isolate, context->GetIsolate());
     napi_clear_last_error(this);
   }
@@ -110,10 +107,6 @@ struct napi_env__ {
     CallIntoModule([&](napi_env env) { cb(env, data, hint); });
   }
 
-  bool HasFeature(node_api_features feature) {
-    return (features & feature) != 0;
-  }
-
   v8impl::Persistent<v8::Value> last_exception;
 
   // We store references in two different lists, depending on whether they have
@@ -125,13 +118,9 @@ struct napi_env__ {
   int open_handle_scopes = 0;
   int open_callback_scopes = 0;
   int refs = 1;
-  node_api_features features = node_api_features_default;
   void* instance_data = nullptr;
 };
 
-static_assert(node_api_features_default < INT_MAX,
-              "node_api_features must fit into an int32_t.");
-
 // This class is used to keep a napi_env live in a way that
 // is exception safe versus calling Ref/Unref directly
 class EnvRefHolder {

diff --git a/src/node_api.cc b/src/node_api.cc
@@ -18,9 +18,8 @@
 #include <memory>
 
 node_napi_env__::node_napi_env__(v8::Local<v8::Context> context,
-                                 const std::string& module_filename,
-                                 node_api_features features)
-    : napi_env__(context, features), filename(module_filename) {
+                                 const std::string& module_filename)
+    : napi_env__(context), filename(module_filename) {
   CHECK_NOT_NULL(node_env());
 }
 
@@ -85,11 +84,10 @@ class BufferFinalizer : private Finalizer {
 };
 
 static inline napi_env NewEnv(v8::Local<v8::Context> context,
-                              const std::string& module_filename,
-                              node_api_features features) {
+                              const std::string& module_filename) {
   node_napi_env result;
 
-  result = new node_napi_env__(context, module_filename, features);
+  result = new node_napi_env__(context, module_filename);
   // TODO(addaleax): There was previously code that tried to delete the
   // napi_env when its v8::Context was garbage collected;
   // However, as long as N-API addons using this napi_env are in place,
@@ -551,13 +549,6 @@ class AsyncContext {
 
 }  // end of namespace v8impl
 
-void napi_module_register_by_symbol_with_features(
-    v8::Local<v8::Object> exports,
-    v8::Local<v8::Value> module,
-    v8::Local<v8::Context> context,
-    napi_addon_register_func init,
-    node_api_features features);
-
 // Intercepts the Node-V8 module registration callback. Converts parameters
 // to NAPI equivalents and then calls the registration callback specified
 // by the NAPI module.
@@ -576,16 +567,6 @@ void napi_module_register_by_symbol(v8::Local<v8::Object> exports,
                                     v8::Local<v8::Value> module,
                                     v8::Local<v8::Context> context,
                                     napi_addon_register_func init) {
-  napi_module_register_by_symbol_with_features(
-      exports, module, context, init, node_api_features_none);
-}
-
-void napi_module_register_by_symbol_with_features(
-    v8::Local<v8::Object> exports,
-    v8::Local<v8::Value> module,
-    v8::Local<v8::Context> context,
-    napi_addon_register_func init,
-    node_api_features features) {
   node::Environment* node_env = node::Environment::GetCurrent(context);
   std::string module_filename = "";
   if (init == nullptr) {
@@ -613,7 +594,7 @@ void napi_module_register_by_symbol_with_features(
   }
 
   // Create a new napi_env for this specific module.
-  napi_env env = v8impl::NewEnv(context, module_filename, features);
+  napi_env env = v8impl::NewEnv(context, module_filename);
 
   napi_value _exports;
   env->CallIntoModule([&](napi_env env) {

diff --git a/src/node_api.h b/src/node_api.h
@@ -38,8 +38,7 @@ typedef struct napi_module {
   napi_addon_register_func nm_register_func;
   const char* nm_modname;
   void* nm_priv;
-  uintptr_t nm_features;
-  void* reserved[3];
+  void* reserved[4];
 } napi_module;
 
 #define NAPI_MODULE_VERSION 1
@@ -74,14 +73,6 @@ typedef struct napi_module {
   static void fn(void)
 #endif
 
-#ifndef NAPI_DEFAULT_FEATURES
-#ifdef NAPI_EXPERIMENTAL
-#define NAPI_DEFAULT_FEATURES (uintptr_t) node_api_features_default
-#else
-#define NAPI_DEFAULT_FEATURES (uintptr_t)0
-#endif
-#endif
-
 #define NAPI_MODULE_X(modname, regfunc, priv, flags)                           \
   EXTERN_C_START                                                               \
   static napi_module _module = {                                               \
@@ -91,7 +82,6 @@ typedef struct napi_module {
       regfunc,                                                                 \
       #modname,                                                                \
       priv,                                                                    \
-      NAPI_DEFAULT_FEATURES,                                                   \
       {0},                                                                     \
   };                                                                           \
   NAPI_C_CTOR(_register_##modname) { napi_module_register(&_module); }         \

diff --git a/src/node_api_internals.h b/src/node_api_internals.h
@@ -10,8 +10,7 @@
 
 struct node_napi_env__ : public napi_env__ {
   node_napi_env__(v8::Local<v8::Context> context,
-                  const std::string& module_filename,
-                  node_api_features features);
+                  const std::string& module_filename);
 
   bool can_call_into_js() const override;
   v8::Maybe<bool> mark_arraybuffer_as_untransferable(

diff --git a/test/js-native-api/entry_point.c b/test/js-native-api/entry_point.c
@@ -1,4 +1,3 @@
-#define NAPI_EXPERIMENTAL
 #include <node_api.h>
 
 EXTERN_C_START

diff --git a/test/js-native-api/test_object/test.js b/test/js-native-api/test_object/test.js
@@ -271,14 +271,6 @@ assert.strictEqual(newObject.test_string, 'test string');
                           fooSymbol,
                           'inherited']);
 
-  // Previously Node-API used writable key instead of configurable
-  assert.deepStrictEqual(test_object.GetConfigurableNamesUnfixed(object),
-                         ['5',
-                          'normal',
-                          'writable',
-                          fooSymbol,
-                          'inherited']);
-
   assert.deepStrictEqual(test_object.GetConfigurableNames(object),
                          ['5',
                           'normal',

diff --git a/test/js-native-api/test_object/test_object.c b/test/js-native-api/test_object/test_object.c
@@ -1,4 +1,3 @@
-#define NAPI_EXPERIMENTAL
 #include <js_native_api.h>
 #include "../common.h"
 #include <string.h>
@@ -163,45 +162,6 @@ static napi_value GetConfigurableNames(napi_env env, napi_callback_info info) {
   return output;
 }
 
-static napi_value GetConfigurableNamesUnfixed(napi_env env,
-                                              napi_callback_info info) {
-  size_t argc = 1;
-  napi_value args[1];
-  NODE_API_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL));
-
-  NODE_API_ASSERT(env, argc >= 1, "Wrong number of arguments");
-
-  napi_valuetype value_type0;
-  NODE_API_CALL(env, napi_typeof(env, args[0], &value_type0));
-
-  NODE_API_ASSERT(
-      env,
-      value_type0 == napi_object,
-      "Wrong type of arguments. Expects an object as first argument.");
-
-  // Remove the feature that fixes the napi_get_all_property_names behavior.
-  NODE_API_CALL(env,
-                node_api_remove_features(
-                    env, node_api_features_use_configurable_key_filter));
-
-  napi_value output;
-  NODE_API_CALL(
-      env,
-      napi_get_all_property_names(env,
-                                  args[0],
-                                  napi_key_include_prototypes,
-                                  napi_key_enumerable | napi_key_configurable,
-                                  napi_key_numbers_to_strings,
-                                  &output));
-
-  // Restore the features.
-  NODE_API_CALL(env,
-                node_api_add_features(
-                    env, node_api_features_use_configurable_key_filter));
-
-  return output;
-}
-
 static napi_value Set(napi_env env, napi_callback_info info) {
   size_t argc = 3;
   napi_value args[3];
@@ -634,7 +594,6 @@ napi_value Init(napi_env env, napi_value exports) {
     DECLARE_NODE_API_PROPERTY("GetSymbolNames", GetSymbolNames),
     DECLARE_NODE_API_PROPERTY("GetWritableNames", GetWritableNames),
     DECLARE_NODE_API_PROPERTY("GetConfigurableNames", GetConfigurableNames),
-    DECLARE_NODE_API_PROPERTY("GetConfigurableNamesUnfixed", GetConfigurableNamesUnfixed),
     DECLARE_NODE_API_PROPERTY("Set", Set),
     DECLARE_NODE_API_PROPERTY("SetNamed", SetNamed),
     DECLARE_NODE_API_PROPERTY("Has", Has),