intl: unexpose Intl.v8BreakIterator

It was never an official Ecma-402 API, it is about to be superseded
by `Intl.Segmenter` and it's prone to crash under some circumstances.

Searches don't turn up any usage in the wild and the recommendation
from the V8 team is to remove it.  Now seems like a good a time as
any to do that.

Fixes: https://github.com/nodejs/node/issues/8865
Fixes: https://github.com/nodejs/node/issues/14909
Refs: https://github.com/tc39/proposal-intl-segmenter
Refs: https://chromium-review.googlesource.com/c/v8/v8/+/620755
PR-URL: https://github.com/nodejs/node/pull/15238
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

doc/api/deprecations.md

@@ -185,9 +185,10 @@ and should no longer be used.
 <a id="DEP0017"></a>
 ### DEP0017: Intl.v8BreakIterator
 
-Type: Runtime
+Type: End-of-Life
 
-The `Intl.v8BreakIterator` is deprecated and will be removed or replaced soon.
+`Intl.v8BreakIterator` was a non-standard extension and has been removed.
+See [`Intl.Segmenter`](https://github.com/tc39/proposal-intl-segmenter).
 
 <a id="DEP0018"></a>
 ### DEP0018: Unhandled promise rejections

lib/internal/errors.js

@@ -275,8 +275,6 @@ E('ERR_UNKNOWN_STREAM_TYPE', 'Unknown stream file type');
 E('ERR_VALUE_OUT_OF_RANGE', (start, end, value) => {
   return `The value of "${start}" must be ${end}. Received "${value}"`;
 });
-E('ERR_V8BREAKITERATOR', 'Full ICU data not installed. ' +
-  'See https://github.com/nodejs/node/wiki/Intl');
 E('ERR_VALID_PERFORMANCE_ENTRY_TYPE',
   'At least one valid performance entry type is required');
 E('ERR_VALUE_OUT_OF_RANGE', 'The value of "%s" must be %s. Received "%s"');

lib/internal/process.js

@@ -133,20 +133,6 @@ function setupConfig(_source) {
     if (value === 'false') return false;
     return value;
   });
-  const processConfig = process.binding('config');
-  if (typeof Intl !== 'undefined' && Intl.hasOwnProperty('v8BreakIterator')) {
-    const oldV8BreakIterator = Intl.v8BreakIterator;
-    const des = Object.getOwnPropertyDescriptor(Intl, 'v8BreakIterator');
-    des.value = require('internal/util').deprecate(function v8BreakIterator() {
-      if (processConfig.hasSmallICU && !processConfig.icuDataDir) {
-        // Intl.v8BreakIterator() would crash w/ fatal error, so throw instead.
-        throw new errors.Error('ERR_V8BREAKITERATOR');
-      }
-      return Reflect.construct(oldV8BreakIterator, arguments);
-    }, 'Intl.v8BreakIterator is deprecated and will be removed soon.',
-                                                   'DEP0017');
-    Object.defineProperty(Intl, 'v8BreakIterator', des);
-  }
 }
 
 

src/node.cc

@@ -4586,11 +4586,28 @@ void FreeEnvironment(Environment* env) {
 }
 
 
+Local<Context> NewContext(Isolate* isolate,
+                          Local<ObjectTemplate> object_template) {
+  auto context = Context::New(isolate, nullptr, object_template);
+  if (context.IsEmpty()) return context;
+  HandleScope handle_scope(isolate);
+  auto intl_key = FIXED_ONE_BYTE_STRING(isolate, "Intl");
+  auto break_iter_key = FIXED_ONE_BYTE_STRING(isolate, "v8BreakIterator");
+  Local<Value> intl_v;
+  Local<Object> intl;
+  if (context->Global()->Get(context, intl_key).ToLocal(&intl_v) &&
+      intl_v->ToObject(context).ToLocal(&intl)) {
+    intl->Delete(context, break_iter_key).FromJust();
+  }
+  return context;
+}
+
+
 inline int Start(Isolate* isolate, IsolateData* isolate_data,
                  int argc, const char* const* argv,
                  int exec_argc, const char* const* exec_argv) {
   HandleScope handle_scope(isolate);
-  Local<Context> context = Context::New(isolate);
+  Local<Context> context = NewContext(isolate);
   Context::Scope context_scope(context);
   Environment env(isolate_data, context);
   CHECK_EQ(0, uv_key_create(&thread_local_env));

src/node_contextify.cc

@@ -240,7 +240,7 @@ class ContextifyContext {
                                              CreateDataWrapper(env));
     object_template->SetHandler(config);
 
-    Local<Context> ctx = Context::New(env->isolate(), nullptr, object_template);
+    Local<Context> ctx = NewContext(env->isolate(), object_template);
 
     if (ctx.IsEmpty()) {
       env->ThrowError("Could not instantiate context");

src/node_internals.h

@@ -125,6 +125,14 @@ inline v8::Local<TypeName> PersistentToLocal(
     v8::Isolate* isolate,
     const v8::Persistent<TypeName>& persistent);
 
+// Creates a new context with Node.js-specific tweaks.  Currently, it removes
+// the `v8BreakIterator` property from the global `Intl` object if present.
+// See https://github.com/nodejs/node/issues/14909 for more info.
+v8::Local<v8::Context> NewContext(
+    v8::Isolate* isolate,
+    v8::Local<v8::ObjectTemplate> object_template =
+        v8::Local<v8::ObjectTemplate>());
+
 // Convert a struct sockaddr to a { address: '1.2.3.4', port: 1234 } JS object.
 // Sets address and port properties on the info object and returns it.
 // If |info| is omitted, a new object is returned.

test/parallel/test-intl-v8BreakIterator.js

@@ -1,17 +1,10 @@
 'use strict';
 const common = require('../common');
+const assert = require('assert');
+const vm = require('vm');
 
-if (!common.hasIntl || Intl.v8BreakIterator === undefined)
+if (typeof Intl === 'undefined')
   common.skip('missing Intl');
 
-const assert = require('assert');
+assert(!('v8BreakIterator' in Intl));
-const warning = 'Intl.v8BreakIterator is deprecated and will be removed soon.';
+assert(!vm.runInNewContext('"v8BreakIterator" in Intl'));
-common.expectWarning('DeprecationWarning', warning);
-
-try {
-  new Intl.v8BreakIterator();
-  // May succeed if data is available - OK
-} catch (e) {
-  // May throw this error if ICU data is not available - OK
-  assert.throws(() => new Intl.v8BreakIterator(), /ICU data/);
-}