From 46d7cb88c7f8b416e667c52de80e6766115b3781 Mon Sep 17 00:00:00 2001 From: Refael Ackermann Date: Mon, 3 Jul 2017 12:51:45 -0400 Subject: [PATCH 1/6] tools: eslint - use `error` and `off` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/14061 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Michaël Zasso Reviewed-By: Vse Mozhet Byt Reviewed-By: Colin Ihrig Reviewed-By: Franziska Hinkelmann Reviewed-By: Luigi Pinca --- .eslintrc.yaml | 194 +++++++++++++++++++-------------------- benchmark/.eslintrc.yaml | 16 ++-- doc/.eslintrc.yaml | 32 +++---- lib/.eslintrc.yaml | 6 +- test/.eslintrc.yaml | 12 +-- tools/.eslintrc.yaml | 16 ++-- 6 files changed, 138 insertions(+), 138 deletions(-) diff --git a/.eslintrc.yaml b/.eslintrc.yaml index 9b0bb9261ee..96f07d7564f 100644 --- a/.eslintrc.yaml +++ b/.eslintrc.yaml @@ -13,39 +13,39 @@ parserOptions: rules: # Possible Errors # http://eslint.org/docs/rules/#possible-errors - no-control-regex: 2 - no-debugger: 2 - no-dupe-args: 2 - no-dupe-keys: 2 - no-duplicate-case: 2 - no-empty-character-class: 2 - no-ex-assign: 2 - no-extra-boolean-cast: 2 - no-extra-parens: [2, functions] - no-extra-semi: 2 - no-func-assign: 2 - no-invalid-regexp: 2 - no-irregular-whitespace: 2 - no-obj-calls: 2 - no-template-curly-in-string: 2 - no-unexpected-multiline: 2 - no-unreachable: 2 - no-unsafe-negation: 2 - use-isnan: 2 - valid-typeof: 2 + no-control-regex: error + no-debugger: error + no-dupe-args: error + no-dupe-keys: error + no-duplicate-case: error + no-empty-character-class: error + no-ex-assign: error + no-extra-boolean-cast: error + no-extra-parens: [error, functions] + no-extra-semi: error + no-func-assign: error + no-invalid-regexp: error + no-irregular-whitespace: error + no-obj-calls: error + no-template-curly-in-string: error + no-unexpected-multiline: error + no-unreachable: error + no-unsafe-negation: error + use-isnan: error + valid-typeof: error # Best Practices # http://eslint.org/docs/rules/#best-practices - dot-location: [2, property] - eqeqeq: [2, smart] - no-fallthrough: 2 - no-global-assign: 2 - no-multi-spaces: [2, {ignoreEOLComments: true}] - no-octal: 2 - no-proto: 2 - no-redeclare: 2 + dot-location: [error, property] + eqeqeq: [error, smart] + no-fallthrough: error + no-global-assign: error + no-multi-spaces: [error, {ignoreEOLComments: true}] + no-octal: error + no-proto: error + no-redeclare: error no-restricted-properties: - - 2 + - error - object: assert property: deepEqual message: Use assert.deepStrictEqual(). @@ -59,71 +59,71 @@ rules: message: __defineGetter__ is deprecated. - property: __defineSetter__ message: __defineSetter__ is deprecated. - no-self-assign: 2 - no-throw-literal: 2 - no-unused-labels: 2 - no-useless-call: 2 - no-useless-concat: 2 - no-useless-escape: 2 - no-useless-return: 2 - no-void: 2 - no-with: 2 + no-self-assign: error + no-throw-literal: error + no-unused-labels: error + no-useless-call: error + no-useless-concat: error + no-useless-escape: error + no-useless-return: error + no-void: error + no-with: error # Strict Mode # http://eslint.org/docs/rules/#strict-mode - strict: [2, global] + strict: [error, global] # Variables # http://eslint.org/docs/rules/#variables - no-delete-var: 2 - no-undef: 2 - no-unused-vars: [2, {args: none}] - no-use-before-define: [2, {classes: true, - functions: false, - variables: false}] + no-delete-var: error + no-undef: error + no-unused-vars: [error, {args: none}] + no-use-before-define: [error, {classes: true, + functions: false, + variables: false}] # Node.js and CommonJS # http://eslint.org/docs/rules/#nodejs-and-commonjs - no-mixed-requires: 2 - no-new-require: 2 - no-path-concat: 2 - no-restricted-modules: [2, sys] + no-mixed-requires: error + no-new-require: error + no-path-concat: error + no-restricted-modules: [error, sys] # Stylistic Issues # http://eslint.org/docs/rules/#stylistic-issues - block-spacing: 2 - brace-style: [2, 1tbs, {allowSingleLine: true}] - comma-dangle: [2, only-multiline] - comma-spacing: 2 - comma-style: 2 - computed-property-spacing: 2 - eol-last: 2 - func-call-spacing: 2 - func-name-matching: 2 - func-style: [2, declaration, {allowArrowFunctions: true}] - # indent: [2, 2, {ArrayExpression: first, + block-spacing: error + brace-style: [error, 1tbs, {allowSingleLine: true}] + comma-dangle: [error, only-multiline] + comma-spacing: error + comma-style: error + computed-property-spacing: error + eol-last: error + func-call-spacing: error + func-name-matching: error + func-style: [error, declaration, {allowArrowFunctions: true}] + # indent: [error, error, {ArrayExpression: first, # CallExpression: {arguments: first}, # FunctionDeclaration: {parameters: first}, # FunctionExpression: {parameters: first}, # MemberExpression: off, # ObjectExpression: first, # SwitchCase: 1}] - indent-legacy: [2, 2, {ArrayExpression: first, + indent-legacy: [error, 2, {ArrayExpression: first, CallExpression: {arguments: first}, MemberExpression: 1, ObjectExpression: first, SwitchCase: 1}] - key-spacing: [2, {mode: minimum}] - keyword-spacing: 2 - linebreak-style: [2, unix] - max-len: [2, {code: 80, + key-spacing: [error, {mode: minimum}] + keyword-spacing: error + linebreak-style: [error, unix] + max-len: [error, {code: 80, ignoreRegExpLiterals: true, ignoreUrls: true, tabWidth: 2}] - new-parens: 2 - no-mixed-spaces-and-tabs: 2 - no-multiple-empty-lines: [2, {max: 2, maxEOF: 0, maxBOF: 0}] - no-restricted-syntax: [2, { + new-parens: error + no-mixed-spaces-and-tabs: error + no-multiple-empty-lines: [error, {max: 2, maxEOF: 0, maxBOF: 0}] + no-restricted-syntax: [error, { selector: "CallExpression[callee.name='setTimeout'][arguments.length<2]", message: "setTimeout() must be invoked with at least two arguments." }, { @@ -133,43 +133,43 @@ rules: selector: "ThrowStatement > CallExpression[callee.name=/Error$/]", message: "Use new keyword when throwing an Error." }] - no-tabs: 2 - no-trailing-spaces: 2 - one-var-declaration-per-line: 2 - operator-linebreak: [2, after] - quotes: [2, single, avoid-escape] - semi: 2 - semi-spacing: 2 - space-before-blocks: [2, always] - space-before-function-paren: [2, { + no-tabs: error + no-trailing-spaces: error + one-var-declaration-per-line: error + operator-linebreak: [error, after] + quotes: [error, single, avoid-escape] + semi: error + semi-spacing: error + space-before-blocks: [error, always] + space-before-function-paren: [error, { "anonymous": "never", "named": "never", "asyncArrow": "always" }] - space-in-parens: [2, never] - space-infix-ops: 2 - space-unary-ops: 2 - unicode-bom: 2 + space-in-parens: [error, never] + space-infix-ops: error + space-unary-ops: error + unicode-bom: error # ECMAScript 6 # http://eslint.org/docs/rules/#ecmascript-6 - arrow-parens: [2, always] - arrow-spacing: [2, {before: true, after: true}] - constructor-super: 2 - no-class-assign: 2 - no-confusing-arrow: 2 - no-const-assign: 2 - no-dupe-class-members: 2 - no-new-symbol: 2 - no-this-before-super: 2 - prefer-const: [2, {ignoreReadBeforeAssign: true}] - rest-spread-spacing: 2 - template-curly-spacing: 2 + arrow-parens: [error, always] + arrow-spacing: [error, {before: true, after: true}] + constructor-super: error + no-class-assign: error + no-confusing-arrow: error + no-const-assign: error + no-dupe-class-members: error + no-new-symbol: error + no-this-before-super: error + prefer-const: [error, {ignoreReadBeforeAssign: true}] + rest-spread-spacing: error + template-curly-spacing: error # Custom rules in tools/eslint-rules - align-multiline-assignment: 2 - assert-throws-arguments: [2, { requireTwo: true }] - no-unescaped-regexp-dot: 2 + align-multiline-assignment: error + assert-throws-arguments: [error, { requireTwo: true }] + no-unescaped-regexp-dot: error # Global scoped method and vars globals: diff --git a/benchmark/.eslintrc.yaml b/benchmark/.eslintrc.yaml index d569a63249f..beaaea041fc 100644 --- a/benchmark/.eslintrc.yaml +++ b/benchmark/.eslintrc.yaml @@ -3,11 +3,11 @@ rules: # Stylistic Issues # http://eslint.org/docs/rules/#stylistic-issues - indent: [2, 2, {ArrayExpression: first, - CallExpression: {arguments: first}, - FunctionDeclaration: {parameters: first}, - FunctionExpression: {parameters: first}, - MemberExpression: off, - ObjectExpression: first, - SwitchCase: 1}] - indent-legacy: 0 + indent: [error, 2, {ArrayExpression: first, + CallExpression: {arguments: first}, + FunctionDeclaration: {parameters: first}, + FunctionExpression: {parameters: first}, + MemberExpression: off, + ObjectExpression: first, + SwitchCase: 1}] + indent-legacy: off diff --git a/doc/.eslintrc.yaml b/doc/.eslintrc.yaml index df93899bfb8..a112e0f54cd 100644 --- a/doc/.eslintrc.yaml +++ b/doc/.eslintrc.yaml @@ -1,25 +1,25 @@ ## Docs-specific linter rules rules: - object-curly-spacing: [2, always] + object-curly-spacing: [error, always] # ease some restrictions in doc examples - no-restricted-properties: 0 - no-undef: 0 - no-unused-vars: 0 - strict: 0 + no-restricted-properties: off + no-undef: off + no-unused-vars: off + strict: off # add new ECMAScript features gradually - no-var: 2 - prefer-const: 2 - prefer-rest-params: 2 + no-var: error + prefer-const: error + prefer-rest-params: error # use stricter indent over indent-legacy - indent-legacy: 0 - indent: [2, 2, {ArrayExpression: first, - CallExpression: {arguments: first}, - FunctionDeclaration: {parameters: first}, - FunctionExpression: {parameters: first}, - MemberExpression: off, - ObjectExpression: first, - SwitchCase: 1}] + indent-legacy: off + indent: [error, 2, {ArrayExpression: first, + CallExpression: {arguments: first}, + FunctionDeclaration: {parameters: first}, + FunctionExpression: {parameters: first}, + MemberExpression: off, + ObjectExpression: first, + SwitchCase: 1}] diff --git a/lib/.eslintrc.yaml b/lib/.eslintrc.yaml index d8e34f85b57..24f54e68263 100644 --- a/lib/.eslintrc.yaml +++ b/lib/.eslintrc.yaml @@ -1,5 +1,5 @@ rules: # Custom rules in tools/eslint-rules - require-buffer: 2 - buffer-constructor: 2 - no-let-in-for-declaration: 2 + require-buffer: error + buffer-constructor: error + no-let-in-for-declaration: error diff --git a/test/.eslintrc.yaml b/test/.eslintrc.yaml index 501c40781c6..aeaf09fb0ff 100644 --- a/test/.eslintrc.yaml +++ b/test/.eslintrc.yaml @@ -3,12 +3,12 @@ rules: # ECMAScript 6 # http://eslint.org/docs/rules/#ecmascript-6 - no-var: 2 - prefer-const: 2 + no-var: error + prefer-const: error # Custom rules in tools/eslint-rules - prefer-assert-iferror: 2 - prefer-assert-methods: 2 - prefer-common-mustnotcall: 2 + prefer-assert-iferror: error + prefer-assert-methods: error + prefer-common-mustnotcall: error ## common module is mandatory in tests - required-modules: [2, common] + required-modules: [error, common] diff --git a/tools/.eslintrc.yaml b/tools/.eslintrc.yaml index 002fe1a6a74..1655ae36f41 100644 --- a/tools/.eslintrc.yaml +++ b/tools/.eslintrc.yaml @@ -3,11 +3,11 @@ rules: # Stylistic Issues # http://eslint.org/docs/rules/#stylistic-issues - indent: [2, 2, {ArrayExpression: first, - CallExpression: {arguments: first}, - FunctionDeclaration: {parameters: first}, - FunctionExpression: {parameters: first}, - MemberExpression: off, - ObjectExpression: first, - SwitchCase: 1}] - indent-legacy: 0 + indent: [error, 2, {ArrayExpression: first, + CallExpression: {arguments: first}, + FunctionDeclaration: {parameters: first}, + FunctionExpression: {parameters: first}, + MemberExpression: off, + ObjectExpression: first, + SwitchCase: 1}] + indent-legacy: off From c45df83b54744ce5b9dcb20b9f8a8355085991a4 Mon Sep 17 00:00:00 2001 From: Sam Roberts Date: Mon, 26 Jun 2017 11:24:33 -0700 Subject: [PATCH 2/6] src: --abort-on-uncaught-exception in NODE_OPTIONS Allow --abort-on-uncaught-exception in NODE_OPTIONS, its useful to enable for post-mortem debugging. PR-URL: https://github.com/nodejs/node/pull/13932 Reviewed-By: Colin Ihrig Reviewed-By: James M Snell --- doc/api/cli.md | 1 + src/node.cc | 1 + test/parallel/test-cli-node-options.js | 1 + 3 files changed, 3 insertions(+) diff --git a/doc/api/cli.md b/doc/api/cli.md index 77973a543f0..0c88eeca175 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -457,6 +457,7 @@ Node options that are allowed are: - `--zero-fill-buffers` V8 options that are allowed are: +- `--abort-on-uncaught-exception` - `--max_old_space_size` ### `NODE_PENDING_DEPRECATION=1` diff --git a/src/node.cc b/src/node.cc index 70f5bbb3032..944ba6deddd 100644 --- a/src/node.cc +++ b/src/node.cc @@ -3766,6 +3766,7 @@ static void CheckIfAllowedInEnv(const char* exe, bool is_env, "--icu-data-dir", // V8 options + "--abort-on-uncaught-exception", "--max_old_space_size", }; diff --git a/test/parallel/test-cli-node-options.js b/test/parallel/test-cli-node-options.js index 8622dafbe10..558aae44ef3 100644 --- a/test/parallel/test-cli-node-options.js +++ b/test/parallel/test-cli-node-options.js @@ -40,6 +40,7 @@ function disallow(opt) { const printA = require.resolve('../fixtures/printA.js'); +expect('--abort-on-uncaught-exception', 'B\n'); expect(`-r ${printA}`, 'A\nB\n'); expect('--no-deprecation', 'B\n'); expect('--no-warnings', 'B\n'); From 34cf8ad73d005d86458124ab5c6b62a0aab7ed8e Mon Sep 17 00:00:00 2001 From: Michael Dawson Date: Thu, 29 Jun 2017 13:07:03 -0400 Subject: [PATCH 3/6] test: add coverage for napi_typeof We had some, but not complete coverage indirectly through other tests. Add test to validate it specifically and covers cases that were not being covered. PR-URL: https://github.com/nodejs/node/pull/13990 Reviewed-By: James M Snell Reviewed-By: Colin Ihrig --- test/addons-napi/test_general/test.js | 16 ++++++++++ test/addons-napi/test_general/test_general.c | 32 +++++++++++++++++++- 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/test/addons-napi/test_general/test.js b/test/addons-napi/test_general/test.js index 2aff480eeb2..32d26475cd6 100644 --- a/test/addons-napi/test_general/test.js +++ b/test/addons-napi/test_general/test.js @@ -34,3 +34,19 @@ assert.ok(test_general.testGetPrototype(baseObject) !== // test version management functions // expected version is currently 1 assert.strictEqual(test_general.testGetVersion(), 1); + +[ + 123, + 'test string', + function() {}, + new Object(), + true, + undefined, + Symbol() +].forEach((val) => { + assert.strictEqual(test_general.testNapiTypeof(val), typeof val); +}); + +// since typeof in js return object need to validate specific case +// for null +assert.strictEqual(test_general.testNapiTypeof(null), 'null'); diff --git a/test/addons-napi/test_general/test_general.c b/test/addons-napi/test_general/test_general.c index 0b69cc41e29..ab2428b9752 100644 --- a/test/addons-napi/test_general/test_general.c +++ b/test/addons-napi/test_general/test_general.c @@ -29,7 +29,7 @@ napi_value testGetVersion(napi_env env, napi_callback_info info) { uint32_t version; napi_value result; NAPI_CALL(env, napi_get_version(env, &version)); - NAPI_CALL(env ,napi_create_number(env, version, &result)); + NAPI_CALL(env, napi_create_number(env, version, &result)); return result; } @@ -90,6 +90,35 @@ napi_value testNapiErrorCleanup(napi_env env, napi_callback_info info) { return result; } +napi_value testNapiTypeof(napi_env env, napi_callback_info info) { + size_t argc = 1; + napi_value args[1]; + NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL)); + + napi_valuetype argument_type; + NAPI_CALL(env, napi_typeof(env, args[0], &argument_type)); + + napi_value result; + if (argument_type == napi_number) { + NAPI_CALL(env, napi_create_string_utf8(env, "number", -1, &result)); + } else if (argument_type == napi_string) { + NAPI_CALL(env, napi_create_string_utf8(env, "string", -1, &result)); + } else if (argument_type == napi_function) { + NAPI_CALL(env, napi_create_string_utf8(env, "function", -1, &result)); + } else if (argument_type == napi_object) { + NAPI_CALL(env, napi_create_string_utf8(env, "object", -1, &result)); + } else if (argument_type == napi_boolean) { + NAPI_CALL(env, napi_create_string_utf8(env, "boolean", -1, &result)); + } else if (argument_type == napi_undefined) { + NAPI_CALL(env, napi_create_string_utf8(env, "undefined", -1, &result)); + } else if (argument_type == napi_symbol) { + NAPI_CALL(env, napi_create_string_utf8(env, "symbol", -1, &result)); + } else if (argument_type == napi_null) { + NAPI_CALL(env, napi_create_string_utf8(env, "null", -1, &result)); + } + return result; +} + void Init(napi_env env, napi_value exports, napi_value module, void* priv) { napi_property_descriptor descriptors[] = { DECLARE_NAPI_PROPERTY("testStrictEquals", testStrictEquals), @@ -100,6 +129,7 @@ void Init(napi_env env, napi_value exports, napi_value module, void* priv) { DECLARE_NAPI_PROPERTY("getNull", getNull), DECLARE_NAPI_PROPERTY("createNapiError", createNapiError), DECLARE_NAPI_PROPERTY("testNapiErrorCleanup", testNapiErrorCleanup), + DECLARE_NAPI_PROPERTY("testNapiTypeof", testNapiTypeof), }; NAPI_CALL_RETURN_VOID(env, napi_define_properties( From 6809429cfa035b35ca8c83815e644234092f53be Mon Sep 17 00:00:00 2001 From: Timothy Gu Date: Mon, 26 Jun 2017 13:05:49 +0800 Subject: [PATCH 4/6] doc: add documentation on ICU PR-URL: https://github.com/nodejs/node/pull/13916 Refs: https://github.com/nodejs/node/pull/13644#discussion_r121616327 Reviewed-By: Vse Mozhet Byt --- doc/api/_toc.md | 1 + doc/api/all.md | 1 + doc/api/intl.md | 209 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 211 insertions(+) create mode 100644 doc/api/intl.md diff --git a/doc/api/_toc.md b/doc/api/_toc.md index 4865334ec02..1075bc6be39 100644 --- a/doc/api/_toc.md +++ b/doc/api/_toc.md @@ -26,6 +26,7 @@ * [HTTP](http.html) * [HTTPS](https.html) * [Inspector](inspector.html) +* [Internationalization](intl.html) * [Modules](modules.html) * [Net](net.html) * [OS](os.html) diff --git a/doc/api/all.md b/doc/api/all.md index 7122fe96fa0..24eda32f44d 100644 --- a/doc/api/all.md +++ b/doc/api/all.md @@ -21,6 +21,7 @@ @include http @include https @include inspector +@include intl @include modules @include net @include os diff --git a/doc/api/intl.md b/doc/api/intl.md new file mode 100644 index 00000000000..c0d6b822892 --- /dev/null +++ b/doc/api/intl.md @@ -0,0 +1,209 @@ +# Internationalization Support + +Node.js has many features that make it easier to write internationalized +programs. Some of them are: + +- Locale-sensitive or Unicode-aware functions in the [ECMAScript Language + Specification][ECMA-262]: + - [`String.prototype.normalize()`][] + - [`String.prototype.toLowerCase()`][] + - [`String.prototype.toUpperCase()`][] +- All functionality described in the [ECMAScript Internationalization API + Specification][ECMA-402] (aka ECMA-402): + - [`Intl`][] object + - Locale-sensitive methods like [`String.prototype.localeCompare()`][] and + [`Date.prototype.toLocaleString()`][] +- The [WHATWG URL parser][]'s [internationalized domain names][] (IDNs) support +- [`require('buffer').transcode()`][] +- More accurate [REPL][] line editing + +Node.js (and its underlying V8 engine) uses [ICU][] to implement these features +in native C/C++ code. However, some of them require a very large ICU data file +in order to support all locales of the world. Because it is expected that most +Node.js users will make use of only a small portion of ICU functionality, only +a subset of the full ICU data set is provided by Node.js by default. Several +options are provided for customizing and expanding the ICU data set either when +building or running Node.js. + +## Options for building Node.js + +To control how ICU is used in Node.js, four `configure` options are available +during compilation. Additional details on how to compile Node.js are documented +in [BUILDING.md][]. + +- `--with-intl=none` / `--without-intl` +- `--with-intl=system-icu` +- `--with-intl=small-icu` (default) +- `--with-intl=full-icu` + +An overview of available Node.js and JavaScript features for each `configure` +option: + +| | `none` | `system-icu` | `small-icu` | `full-icu` +|-----------------------------------------|-----------------------------------|------------------------------|------------------------|------------ +| [`String.prototype.normalize()`][] | none (function is no-op) | full | full | full +| `String.prototype.to*Case()` | full | full | full | full +| [`Intl`][] | none (object does not exist) | partial/full (depends on OS) | partial (English-only) | full +| [`String.prototype.localeCompare()`][] | partial (not locale-aware) | full | full | full +| `String.prototype.toLocale*Case()` | partial (not locale-aware) | full | full | full +| [`Number.prototype.toLocaleString()`][] | partial (not locale-aware) | partial/full (depends on OS) | partial (English-only) | full +| `Date.prototype.toLocale*String()` | partial (not locale-aware) | partial/full (depends on OS) | partial (English-only) | full +| [WHATWG URL Parser][] | partial (no IDN support) | full | full | full +| [`require('buffer').transcode()`][] | none (function does not exist) | full | full | full +| [REPL][] | partial (inaccurate line editing) | full | full | full + +*Note*: The "(not locale-aware)" designation denotes that the function carries +out its operation just like the non-`Locale` version of the function, if one +exists. For example, under `none` mode, `Date.prototype.toLocaleString()`'s +operation is identical to that of `Date.prototype.toString()`. + +### Disable all internationalization features (`none`) + +If this option is chosen, most internationalization features mentioned above +will be **unavailable** in the resulting `node` binary. + +### Build with a pre-installed ICU (`system-icu`) + +Node.js can link against an ICU build already installed on the system. In fact, +most Linux distributions already come with ICU installed, and this option would +make it possible to reuse the same set of data used by other components in the +OS. + +Functionalities that only require the ICU library itself, such as +[`String.prototype.normalize()`][] and the [WHATWG URL parser][], are fully +supported under `system-icu`. Features that require ICU locale data in +addition, such as [`Intl.DateTimeFormat`][] *may* be fully or partially +supported, depending on the completeness of the ICU data installed on the +system. + +### Embed a limited set of ICU data (`small-icu`) + +This option makes the resulting binary link against the ICU library statically, +and includes a subset of ICU data (typically only the English locale) within +the `node` executable. + +Functionalities that only require the ICU library itself, such as +[`String.prototype.normalize()`][] and the [WHATWG URL parser][], are fully +supported under `small-icu`. Features that require ICU locale data in addition, +such as [`Intl.DateTimeFormat`][], generally only work with the English locale: + +```js +const january = new Date(9e8); +const english = new Intl.DateTimeFormat('en', { month: 'long' }); +const spanish = new Intl.DateTimeFormat('es', { month: 'long' }); + +console.log(english.format(january)); +// Prints "January" +console.log(spanish.format(january)); +// Prints "M01" on small-icu +// Should print "enero" +``` + +This mode provides a good balance between features and binary size, and it is +the default behavior if no `--with-intl` flag is passed. The official binaries +are also built in this mode. + +#### Providing ICU data at runtime + +If the `small-icu` option is used, one can still provide additional locale data +at runtime so that the JS methods would work for all ICU locales. Assuming the +data file is stored at `/some/directory`, it can be made available to ICU +through either: + +* The [`NODE_ICU_DATA`][] environmental variable: + + ```shell + env NODE_ICU_DATA=/some/directory node + ``` + +* The [`--icu-data-dir`][] CLI parameter: + + ```shell + node --icu-data-dir=/some/directory + ``` + +(If both are specified, the `--icu-data-dir` CLI parameter takes precedence.) + +ICU is able to automatically find and load a variety of data formats, but the +data must be appropriate for the ICU version, and the file correctly named. +The most common name for the data file is `icudt5X[bl].dat`, where `5X` denotes +the intended ICU version, and `b` or `l` indicates the system's endianness. +Check ["ICU Data"][] article in the ICU User Guide for other supported formats +and more details on ICU data in general. + +The [full-icu][] npm module can greatly simplify ICU data installation by +detecting the ICU version of the running `node` executable and downloading the +appropriate data file. After installing the module through `npm i full-icu`, +the data file will be available at `./node_modules/full-icu`. This path can be +then passed either to `NODE_ICU_DATA` or `--icu-data-dir` as shown above to +enable full `Intl` support. + +### Embed the entire ICU (`full-icu`) + +This option makes the resulting binary link against ICU statically and include +a full set of ICU data. A binary created this way has no further external +dependencies and supports all locales, but might be rather large. See +[BUILDING.md][BUILDING.md#full-icu] on how to compile a binary using this mode. + +## Detecting internationalization support + +To verify that ICU is enabled at all (`system-icu`, `small-icu`, or +`full-icu`), simply checking the existence of `Intl` should suffice: + +```js +const hasICU = typeof Intl === 'object'; +``` + +Alternatively, checking for `process.versions.icu`, a property defined only +when ICU is enabled, works too: + +```js +const hasICU = typeof process.versions.icu === 'string'; +``` + +To check for support for a non-English locale (i.e. `full-icu` or +`system-icu`), [`Intl.DateTimeFormat`][] can be a good distinguishing factor: + +```js +const hasFullICU = (() => { + try { + const january = new Date(9e8); + const spanish = new Intl.DateTimeFormat('es', { month: 'long' }); + return spanish.format(january) === 'enero'; + } catch (err) { + return false; + } +})(); +``` + +For more verbose tests for `Intl` support, the following resources may be found +to be helpful: + +- [btest402][]: Generally used to check whether Node.js with `Intl` support is + built correctly. +- [Test262][]: ECMAScript's official conformance test suite includes a section + dedicated to ECMA-402. + +[btest402]: https://github.com/srl295/btest402 +[BUILDING.md]: https://github.com/nodejs/node/blob/master/BUILDING.md +[BUILDING.md#full-icu]: https://github.com/nodejs/node/blob/master/BUILDING.md#build-with-full-icu-support-all-locales-supported-by-icu +[`Date.prototype.toLocaleString()`]: https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Date/toLocaleString +[ECMA-262]: https://tc39.github.io/ecma262/ +[ECMA-402]: https://tc39.github.io/ecma402/ +[full-icu]: https://www.npmjs.com/package/full-icu +[ICU]: http://icu-project.org/ +["ICU Data"]: http://userguide.icu-project.org/icudata +[`--icu-data-dir`]: cli.html#cli_icu_data_dir_file +[internationalized domain names]: https://en.wikipedia.org/wiki/Internationalized_domain_name +[`Intl`]: https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Intl +[`Intl.DateTimeFormat`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/DateTimeFormat +[`NODE_ICU_DATA`]: cli.html#cli_node_icu_data_file +[`Number.prototype.toLocaleString()`]: https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Number/toLocaleString +[REPL]: repl.html#repl_repl +[`require('buffer').transcode()`]: buffer.html#buffer_buffer_transcode_source_fromenc_toenc +[`String.prototype.localeCompare()`]: https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/String/localeCompare +[`String.prototype.normalize()`]: https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/String/normalize +[`String.prototype.toLowerCase()`]: https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/String/toLowerCase +[`String.prototype.toUpperCase()`]: https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/String/toUpperCase +[Test262]: https://github.com/tc39/test262/tree/master/test/intl402 +[WHATWG URL parser]: url.html#url_the_whatwg_url_api From c6ce500edf364692efa9d46bc1bd9e959611f7da Mon Sep 17 00:00:00 2001 From: Andreas Madsen Date: Thu, 6 Jul 2017 08:20:03 +0200 Subject: [PATCH 5/6] async_hooks: C++ Embedder API overhaul * Fix AsyncHooksGetTriggerAsyncId such it corresponds to async_hooks.triggerAsyncId and not async_hooks.initTriggerId. * Use an async_context struct instead of two async_uid values. This change was necessary since the fixing AsyncHooksGetTriggerAsyncId otherwise makes it impossible to get the correct default trigger id. It also prevents an invalid triggerAsyncId in MakeCallback. * Rename async_uid to async_id for consistency * Rename get_uid to get_async_id * Add get_trigger_async_id to AsyncResource class PR-URL: https://github.com/nodejs/node/pull/14040 Reviewed-By: Trevor Norris Reviewed-By: Anna Henningsen --- src/async-wrap.cc | 39 +++++++++----- src/inspector_agent.cc | 2 +- src/node.cc | 49 ++++++++---------- src/node.h | 72 ++++++++++++++------------ src/node_crypto.cc | 2 +- test/addons/async-hooks-id/binding.cc | 27 ++++++++++ test/addons/async-hooks-id/binding.gyp | 9 ++++ test/addons/async-hooks-id/test.js | 26 ++++++++++ test/addons/async-resource/binding.cc | 19 +++---- test/addons/async-resource/test.js | 10 ++-- 10 files changed, 167 insertions(+), 88 deletions(-) create mode 100644 test/addons/async-hooks-id/binding.cc create mode 100644 test/addons/async-hooks-id/binding.gyp create mode 100644 test/addons/async-hooks-id/test.js diff --git a/src/async-wrap.cc b/src/async-wrap.cc index 7cfa9e0fe81..0afaf663716 100644 --- a/src/async-wrap.cc +++ b/src/async-wrap.cc @@ -741,40 +741,51 @@ Local AsyncWrap::MakeCallback(const Local cb, /* Public C++ embedder API */ -async_uid AsyncHooksGetExecutionAsyncId(Isolate* isolate) { +async_id AsyncHooksGetExecutionAsyncId(Isolate* isolate) { return Environment::GetCurrent(isolate)->current_async_id(); } -async_uid AsyncHooksGetCurrentId(Isolate* isolate) { +async_id AsyncHooksGetCurrentId(Isolate* isolate) { return AsyncHooksGetExecutionAsyncId(isolate); } -async_uid AsyncHooksGetTriggerAsyncId(Isolate* isolate) { - return Environment::GetCurrent(isolate)->get_init_trigger_id(); +async_id AsyncHooksGetTriggerAsyncId(Isolate* isolate) { + return Environment::GetCurrent(isolate)->trigger_id(); } -async_uid AsyncHooksGetTriggerId(Isolate* isolate) { +async_id AsyncHooksGetTriggerId(Isolate* isolate) { return AsyncHooksGetTriggerAsyncId(isolate); } -async_uid EmitAsyncInit(Isolate* isolate, - Local resource, - const char* name, - async_uid trigger_id) { +async_context EmitAsyncInit(Isolate* isolate, + Local resource, + const char* name, + async_id trigger_async_id) { Environment* env = Environment::GetCurrent(isolate); - async_uid async_id = env->new_async_id(); + // Initialize async context struct + if (trigger_async_id == -1) + trigger_async_id = env->get_init_trigger_id(); + + async_context context = { + env->new_async_id(), // async_id_ + trigger_async_id // trigger_async_id_ + }; + + // Run init hooks Local type = String::NewFromUtf8(isolate, name, v8::NewStringType::kInternalized) .ToLocalChecked(); - AsyncWrap::EmitAsyncInit(env, resource, type, async_id, trigger_id); - return async_id; + AsyncWrap::EmitAsyncInit(env, resource, type, context.async_id, + context.trigger_async_id); + + return context; } -void EmitAsyncDestroy(Isolate* isolate, async_uid id) { - PushBackDestroyId(Environment::GetCurrent(isolate), id); +void EmitAsyncDestroy(Isolate* isolate, async_context asyncContext) { + PushBackDestroyId(Environment::GetCurrent(isolate), asyncContext.async_id); } } // namespace node diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc index bfa2b082b4e..a06ff032ff7 100644 --- a/src/inspector_agent.cc +++ b/src/inspector_agent.cc @@ -604,7 +604,7 @@ bool Agent::StartIoThread(bool wait_for_connect) { message }; MakeCallback(parent_env_->isolate(), process_object, emit_fn.As(), - arraysize(argv), argv, 0, 0); + arraysize(argv), argv, {0, 0}); return true; } diff --git a/src/node.cc b/src/node.cc index 944ba6deddd..3252a4adf05 100644 --- a/src/node.cc +++ b/src/node.cc @@ -363,7 +363,7 @@ static void CheckImmediate(uv_check_t* handle) { env->immediate_callback_string(), 0, nullptr, - 0, 0).ToLocalChecked(); + {0, 0}).ToLocalChecked(); } @@ -1298,8 +1298,7 @@ MaybeLocal MakeCallback(Environment* env, const Local callback, int argc, Local argv[], - double async_id, - double trigger_id) { + async_context asyncContext) { // If you hit this assertion, you forgot to enter the v8::Context first. CHECK_EQ(env->context(), env->isolate()->GetCurrentContext()); @@ -1321,10 +1320,12 @@ MaybeLocal MakeCallback(Environment* env, MaybeLocal ret; { - AsyncHooks::ExecScope exec_scope(env, async_id, trigger_id); + AsyncHooks::ExecScope exec_scope(env, asyncContext.async_id, + asyncContext.trigger_async_id); - if (async_id != 0) { - if (!AsyncWrap::EmitBefore(env, async_id)) return Local(); + if (asyncContext.async_id != 0) { + if (!AsyncWrap::EmitBefore(env, asyncContext.async_id)) + return Local(); } ret = callback->Call(env->context(), recv, argc, argv); @@ -1336,8 +1337,9 @@ MaybeLocal MakeCallback(Environment* env, ret : Undefined(env->isolate()); } - if (async_id != 0) { - if (!AsyncWrap::EmitAfter(env, async_id)) return Local(); + if (asyncContext.async_id != 0) { + if (!AsyncWrap::EmitAfter(env, asyncContext.async_id)) + return Local(); } } @@ -1358,8 +1360,8 @@ MaybeLocal MakeCallback(Environment* env, // Make sure the stack unwound properly. If there are nested MakeCallback's // then it should return early and not reach this code. - CHECK_EQ(env->current_async_id(), async_id); - CHECK_EQ(env->trigger_id(), trigger_id); + CHECK_EQ(env->current_async_id(), asyncContext.async_id); + CHECK_EQ(env->trigger_id(), asyncContext.trigger_async_id); Local process = env->process_object(); @@ -1384,13 +1386,11 @@ MaybeLocal MakeCallback(Isolate* isolate, const char* method, int argc, Local argv[], - async_uid async_id, - async_uid trigger_id) { + async_context asyncContext) { Local method_string = String::NewFromUtf8(isolate, method, v8::NewStringType::kNormal) .ToLocalChecked(); - return MakeCallback(isolate, recv, method_string, argc, argv, - async_id, trigger_id); + return MakeCallback(isolate, recv, method_string, argc, argv, asyncContext); } @@ -1399,14 +1399,12 @@ MaybeLocal MakeCallback(Isolate* isolate, Local symbol, int argc, Local argv[], - async_uid async_id, - async_uid trigger_id) { + async_context asyncContext) { Local callback_v = recv->Get(symbol); if (callback_v.IsEmpty()) return Local(); if (!callback_v->IsFunction()) return Local(); Local callback = callback_v.As(); - return MakeCallback(isolate, recv, callback, argc, argv, - async_id, trigger_id); + return MakeCallback(isolate, recv, callback, argc, argv, asyncContext); } @@ -1415,8 +1413,7 @@ MaybeLocal MakeCallback(Isolate* isolate, Local callback, int argc, Local argv[], - async_uid async_id, - async_uid trigger_id) { + async_context asyncContext) { // Observe the following two subtleties: // // 1. The environment is retrieved from the callback function's context. @@ -1427,7 +1424,7 @@ MaybeLocal MakeCallback(Isolate* isolate, Environment* env = Environment::GetCurrent(callback->CreationContext()); Context::Scope context_scope(env->context()); return MakeCallback(env, recv.As(), callback, argc, argv, - async_id, trigger_id); + asyncContext); } @@ -1440,7 +1437,7 @@ Local MakeCallback(Isolate* isolate, Local* argv) { EscapableHandleScope handle_scope(isolate); return handle_scope.Escape( - MakeCallback(isolate, recv, method, argc, argv, 0, 0) + MakeCallback(isolate, recv, method, argc, argv, {0, 0}) .FromMaybe(Local())); } @@ -1452,7 +1449,7 @@ Local MakeCallback(Isolate* isolate, Local* argv) { EscapableHandleScope handle_scope(isolate); return handle_scope.Escape( - MakeCallback(isolate, recv, symbol, argc, argv, 0, 0) + MakeCallback(isolate, recv, symbol, argc, argv, {0, 0}) .FromMaybe(Local())); } @@ -1464,7 +1461,7 @@ Local MakeCallback(Isolate* isolate, Local* argv) { EscapableHandleScope handle_scope(isolate); return handle_scope.Escape( - MakeCallback(isolate, recv, callback, argc, argv, 0, 0) + MakeCallback(isolate, recv, callback, argc, argv, {0, 0}) .FromMaybe(Local())); } @@ -4426,7 +4423,7 @@ void EmitBeforeExit(Environment* env) { }; MakeCallback(env->isolate(), process_object, "emit", arraysize(args), args, - 0, 0).ToLocalChecked(); + {0, 0}).ToLocalChecked(); } @@ -4447,7 +4444,7 @@ int EmitExit(Environment* env) { MakeCallback(env->isolate(), process_object, "emit", arraysize(args), args, - 0, 0).ToLocalChecked(); + {0, 0}).ToLocalChecked(); // Reload exit code, it may be changed by `emit('exit')` return process_object->Get(exitCode)->Int32Value(); diff --git a/src/node.h b/src/node.h index 9e4edd06277..88465a76347 100644 --- a/src/node.h +++ b/src/node.h @@ -145,7 +145,7 @@ inline v8::Local UVException(int errorno, * These methods need to be called in a HandleScope. * * It is preferred that you use the `MakeCallback` overloads taking - * `async_uid` arguments. + * `async_id` arguments. */ NODE_EXTERN v8::Local MakeCallback( @@ -517,7 +517,11 @@ typedef void (*promise_hook_func) (v8::PromiseHookType type, v8::Local parent, void* arg); -typedef double async_uid; +typedef double async_id; +struct async_context { + ::node::async_id async_id; + ::node::async_id trigger_async_id; +}; /* Registers an additional v8::PromiseHook wrapper. This API exists because V8 * itself supports only a single PromiseHook. */ @@ -528,17 +532,17 @@ NODE_EXTERN void AddPromiseHook(v8::Isolate* isolate, /* Returns the id of the current execution context. If the return value is * zero then no execution has been set. This will happen if the user handles * I/O from native code. */ -NODE_EXTERN async_uid AsyncHooksGetExecutionAsyncId(v8::Isolate* isolate); +NODE_EXTERN async_id AsyncHooksGetExecutionAsyncId(v8::Isolate* isolate); /* legacy alias */ NODE_EXTERN NODE_DEPRECATED("Use AsyncHooksGetExecutionAsyncId(isolate)", - async_uid AsyncHooksGetCurrentId(v8::Isolate* isolate)); + async_id AsyncHooksGetCurrentId(v8::Isolate* isolate)); /* Return same value as async_hooks.triggerAsyncId(); */ -NODE_EXTERN async_uid AsyncHooksGetTriggerAsyncId(v8::Isolate* isolate); +NODE_EXTERN async_id AsyncHooksGetTriggerAsyncId(v8::Isolate* isolate); /* legacy alias */ NODE_EXTERN NODE_DEPRECATED("Use AsyncHooksGetTriggerAsyncId(isolate)", - async_uid AsyncHooksGetTriggerId(v8::Isolate* isolate)); + async_id AsyncHooksGetTriggerId(v8::Isolate* isolate)); /* If the native API doesn't inherit from the helper class then the callbacks @@ -548,13 +552,14 @@ NODE_EXTERN NODE_DEPRECATED("Use AsyncHooksGetTriggerAsyncId(isolate)", * The `trigger_async_id` parameter should correspond to the resource which is * creating the new resource, which will usually be the return value of * `AsyncHooksGetTriggerAsyncId()`. */ -NODE_EXTERN async_uid EmitAsyncInit(v8::Isolate* isolate, - v8::Local resource, - const char* name, - async_uid trigger_async_id); +NODE_EXTERN async_context EmitAsyncInit(v8::Isolate* isolate, + v8::Local resource, + const char* name, + async_id trigger_async_id = -1); /* Emit the destroy() callback. */ -NODE_EXTERN void EmitAsyncDestroy(v8::Isolate* isolate, async_uid id); +NODE_EXTERN void EmitAsyncDestroy(v8::Isolate* isolate, + async_context asyncContext); /* An API specific to emit before/after callbacks is unnecessary because * MakeCallback will automatically call them for you. @@ -572,24 +577,21 @@ v8::MaybeLocal MakeCallback(v8::Isolate* isolate, v8::Local callback, int argc, v8::Local* argv, - async_uid asyncId, - async_uid triggerAsyncId); + async_context asyncContext); NODE_EXTERN v8::MaybeLocal MakeCallback(v8::Isolate* isolate, v8::Local recv, const char* method, int argc, v8::Local* argv, - async_uid asyncId, - async_uid triggerAsyncId); + async_context asyncContext); NODE_EXTERN v8::MaybeLocal MakeCallback(v8::Isolate* isolate, v8::Local recv, v8::Local symbol, int argc, v8::Local* argv, - async_uid asyncId, - async_uid triggerAsyncId); + async_context asyncContext); /* Helper class users can optionally inherit from. If * `AsyncResource::MakeCallback()` is used, then all four callbacks will be @@ -599,18 +601,15 @@ class AsyncResource { AsyncResource(v8::Isolate* isolate, v8::Local resource, const char* name, - async_uid trigger_async_id = -1) + async_id trigger_async_id = -1) : isolate_(isolate), - resource_(isolate, resource), - trigger_async_id_(trigger_async_id) { - if (trigger_async_id_ == -1) - trigger_async_id_ = AsyncHooksGetTriggerAsyncId(isolate); - - uid_ = EmitAsyncInit(isolate, resource, name, trigger_async_id_); + resource_(isolate, resource) { + async_context_ = EmitAsyncInit(isolate, resource, name, + trigger_async_id); } ~AsyncResource() { - EmitAsyncDestroy(isolate_, uid_); + EmitAsyncDestroy(isolate_, async_context_); } v8::MaybeLocal MakeCallback( @@ -619,7 +618,7 @@ class AsyncResource { v8::Local* argv) { return node::MakeCallback(isolate_, get_resource(), callback, argc, argv, - uid_, trigger_async_id_); + async_context_); } v8::MaybeLocal MakeCallback( @@ -628,7 +627,7 @@ class AsyncResource { v8::Local* argv) { return node::MakeCallback(isolate_, get_resource(), method, argc, argv, - uid_, trigger_async_id_); + async_context_); } v8::MaybeLocal MakeCallback( @@ -637,21 +636,30 @@ class AsyncResource { v8::Local* argv) { return node::MakeCallback(isolate_, get_resource(), symbol, argc, argv, - uid_, trigger_async_id_); + async_context_); } v8::Local get_resource() { return resource_.Get(isolate_); } - async_uid get_uid() const { - return uid_; + NODE_DEPRECATED("Use AsyncResource::get_async_id()", + async_id get_uid() const { + return get_async_id(); + } + ) + + async_id get_async_id() const { + return async_context_.async_id; + } + + async_id get_trigger_async_id() const { + return async_context_.trigger_async_id; } private: v8::Isolate* isolate_; v8::Persistent resource_; - async_uid uid_; - async_uid trigger_async_id_; + async_context async_context_; }; } // namespace node diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 9d196c10cc8..36102e53f9c 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -1237,7 +1237,7 @@ int SecureContext::TicketKeyCallback(SSL* ssl, env->ticketkeycallback_string(), arraysize(argv), argv, - 0, 0).ToLocalChecked(); + {0, 0}).ToLocalChecked(); Local arr = ret.As(); int r = arr->Get(kTicketKeyReturnIndex)->Int32Value(); diff --git a/test/addons/async-hooks-id/binding.cc b/test/addons/async-hooks-id/binding.cc new file mode 100644 index 00000000000..611c250e9a2 --- /dev/null +++ b/test/addons/async-hooks-id/binding.cc @@ -0,0 +1,27 @@ +#include "node.h" + +namespace { + +using v8::FunctionCallbackInfo; +using v8::Local; +using v8::Object; +using v8::Value; + +void GetExecutionAsyncId(const FunctionCallbackInfo& args) { + args.GetReturnValue().Set( + node::AsyncHooksGetExecutionAsyncId(args.GetIsolate())); +} + +void GetTriggerAsyncId(const FunctionCallbackInfo& args) { + args.GetReturnValue().Set( + node::AsyncHooksGetTriggerAsyncId(args.GetIsolate())); +} + +void Initialize(Local exports) { + NODE_SET_METHOD(exports, "getExecutionAsyncId", GetExecutionAsyncId); + NODE_SET_METHOD(exports, "getTriggerAsyncId", GetTriggerAsyncId); +} + +} // namespace + +NODE_MODULE(binding, Initialize) diff --git a/test/addons/async-hooks-id/binding.gyp b/test/addons/async-hooks-id/binding.gyp new file mode 100644 index 00000000000..7ede63d94a0 --- /dev/null +++ b/test/addons/async-hooks-id/binding.gyp @@ -0,0 +1,9 @@ +{ + 'targets': [ + { + 'target_name': 'binding', + 'defines': [ 'V8_DEPRECATION_WARNINGS=1' ], + 'sources': [ 'binding.cc' ] + } + ] +} diff --git a/test/addons/async-hooks-id/test.js b/test/addons/async-hooks-id/test.js new file mode 100644 index 00000000000..e6c3cf612ca --- /dev/null +++ b/test/addons/async-hooks-id/test.js @@ -0,0 +1,26 @@ +'use strict'; + +const common = require('../../common'); +const assert = require('assert'); +const binding = require(`./build/${common.buildType}/binding`); +const async_hooks = require('async_hooks'); + +assert.strictEqual( + binding.getExecutionAsyncId(), + async_hooks.executionAsyncId() +); +assert.strictEqual( + binding.getTriggerAsyncId(), + async_hooks.triggerAsyncId() +); + +process.nextTick(common.mustCall(function() { + assert.strictEqual( + binding.getExecutionAsyncId(), + async_hooks.executionAsyncId() + ); + assert.strictEqual( + binding.getTriggerAsyncId(), + async_hooks.triggerAsyncId() + ); +})); diff --git a/test/addons/async-resource/binding.cc b/test/addons/async-resource/binding.cc index 372f7a6fa46..9d3ab37e12a 100644 --- a/test/addons/async-resource/binding.cc +++ b/test/addons/async-resource/binding.cc @@ -80,21 +80,22 @@ void CallViaUtf8Name(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(ret.FromMaybe(Local())); } -void GetUid(const FunctionCallbackInfo& args) { +void GetAsyncId(const FunctionCallbackInfo& args) { assert(args[0]->IsExternal()); auto r = static_cast(args[0].As()->Value()); - args.GetReturnValue().Set(r->get_uid()); + args.GetReturnValue().Set(r->get_async_id()); } -void GetResource(const FunctionCallbackInfo& args) { +void GetTriggerAsyncId(const FunctionCallbackInfo& args) { assert(args[0]->IsExternal()); auto r = static_cast(args[0].As()->Value()); - args.GetReturnValue().Set(r->get_resource()); + args.GetReturnValue().Set(r->get_trigger_async_id()); } -void GetCurrentId(const FunctionCallbackInfo& args) { - args.GetReturnValue().Set( - node::AsyncHooksGetExecutionAsyncId(args.GetIsolate())); +void GetResource(const FunctionCallbackInfo& args) { + assert(args[0]->IsExternal()); + auto r = static_cast(args[0].As()->Value()); + args.GetReturnValue().Set(r->get_resource()); } void Initialize(Local exports) { @@ -103,9 +104,9 @@ void Initialize(Local exports) { NODE_SET_METHOD(exports, "callViaFunction", CallViaFunction); NODE_SET_METHOD(exports, "callViaString", CallViaString); NODE_SET_METHOD(exports, "callViaUtf8Name", CallViaUtf8Name); - NODE_SET_METHOD(exports, "getUid", GetUid); + NODE_SET_METHOD(exports, "getAsyncId", GetAsyncId); + NODE_SET_METHOD(exports, "getTriggerAsyncId", GetTriggerAsyncId); NODE_SET_METHOD(exports, "getResource", GetResource); - NODE_SET_METHOD(exports, "getCurrentId", GetCurrentId); } } // namespace diff --git a/test/addons/async-resource/test.js b/test/addons/async-resource/test.js index b52db61a95c..f19ad58637f 100644 --- a/test/addons/async-resource/test.js +++ b/test/addons/async-resource/test.js @@ -6,6 +6,7 @@ const binding = require(`./build/${common.buildType}/binding`); const async_hooks = require('async_hooks'); const kObjectTag = Symbol('kObjectTag'); +const rootAsyncId = async_hooks.executionAsyncId(); const bindingUids = []; let expectedTriggerId; @@ -38,8 +39,6 @@ async_hooks.createHook({ } }).enable(); -assert.strictEqual(binding.getCurrentId(), 1); - for (const call of [binding.callViaFunction, binding.callViaString, binding.callViaUtf8Name]) { @@ -49,14 +48,14 @@ for (const call of [binding.callViaFunction, methöd(arg) { assert.strictEqual(this, object); assert.strictEqual(arg, 42); - assert.strictEqual(binding.getCurrentId(), uid); + assert.strictEqual(async_hooks.executionAsyncId(), uid); return 'baz'; }, kObjectTag }; if (passedTriggerId === undefined) - expectedTriggerId = 1; + expectedTriggerId = rootAsyncId; else expectedTriggerId = passedTriggerId; @@ -66,7 +65,8 @@ for (const call of [binding.callViaFunction, const ret = call(resource); assert.strictEqual(ret, 'baz'); assert.strictEqual(binding.getResource(resource), object); - assert.strictEqual(binding.getUid(resource), uid); + assert.strictEqual(binding.getAsyncId(resource), uid); + assert.strictEqual(binding.getTriggerAsyncId(resource), expectedTriggerId); binding.destroyAsyncResource(resource); } From f651e4035026feb4cf2fc70b0a4f0a6be9d92862 Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Wed, 5 Jul 2017 05:35:50 +0200 Subject: [PATCH 6/6] test: check and fail inspector-cluster-port-clash Currently this test fail when configured --without-inspector or --without-ssl as it is expected to fail but the skipIfInspectorDisabled check will exit as if the test was sucessful. This commit checks if inspector support is available and fails the test allowing the test to be skipped. PR-URL: https://github.com/nodejs/node/pull/14074 Reviewed-By: Colin Ihrig Reviewed-By: Refael Ackermann --- .../known_issues/test-inspector-cluster-port-clash.js | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/test/known_issues/test-inspector-cluster-port-clash.js b/test/known_issues/test-inspector-cluster-port-clash.js index 41b00aacc14..9fa2b483568 100644 --- a/test/known_issues/test-inspector-cluster-port-clash.js +++ b/test/known_issues/test-inspector-cluster-port-clash.js @@ -1,6 +1,7 @@ // Flags: --inspect=0 'use strict'; const common = require('../common'); +const assert = require('assert'); // With the current behavior of Node.js (at least as late as 8.1.0), this // test fails with the following error: @@ -10,9 +11,15 @@ const common = require('../common'); // // Refs: https://github.com/nodejs/node/issues/13343 -common.skipIfInspectorDisabled(); +// This following check should be replaced by common.skipIfInspectorDisabled() +// if moved out of the known_issues directory. +if (process.config.variables.v8_enable_inspector === 0) { + // When the V8 inspector is disabled, using either --without-inspector or + // --without-ssl, this test will not fail which it is expected to do. + // The following fail will allow this test to be skipped by failing it. + assert.fail('skipping as V8 inspector is disabled'); +} -const assert = require('assert'); const cluster = require('cluster'); const net = require('net');