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

util: introduce util.types.is[…] type checks #18415

Closed
wants to merge 2 commits into from

Conversation

addaleax
Copy link
Member

Provide public APIs for native typechecking that is actually useful. The motivation for this is providing alternatives to userland modules that would currently rely on process.binding('util').

This is based off the list in v8.h, with things that have better JS equivalents removed; I’m happy to bikeshed this list.

Also, for context, previous efforts:

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

util

@addaleax addaleax added util Issues and PRs related to the built-in util module. semver-minor PRs that contain new features and should be released in the next minor version. labels Jan 27, 2018
@nodejs-github-bot nodejs-github-bot added the lib / src Issues and PRs related to general changes in the lib or src directory. label Jan 27, 2018
Copy link
Contributor

@evanlucas evanlucas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with a few nits. Are we sure we want to put this on util as opposed to a new module?

I would lean more towards a types module, but am fine either way. Thanks for doing this!!

For example:

```js
util.types.isAsyncFunction(function foo() {}); // Returns false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be isGeneratorFunction

doc/api/util.md Outdated
For example:

```js
util.types.isPromise(new Map()); // Returns true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isMap()

@ChALkeR
Copy link
Member

ChALkeR commented Jan 28, 2018

@evanlucas Introducing a new module is a semver-major, as it would break npmjs.com/types module.

'Accessing native typechecking bindings of Node ' +
'directly is deprecated. ' +
`Please use \`util.types.${name}\` instead.`,
'DEP0XXX') :
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a new deprecation is introduced, it should be recorded in doc/api/deprecations.md.

V(WeakMap) \
V(WeakSet) \
V(ArrayBuffer) \
V(TypedArray) \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We no longer need this with isTypedArray being present in lib/internal/types.js.

namespace {

#define VALUE_METHOD_MAP(V) \
V(External) \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Four-space indentation. Ditto elsewhere.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TimothyGu All other macro lists I can find use 2 spaces for indentation?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heh. Never mind then, I guess.

@addaleax
Copy link
Member Author

@evanlucas Like @ChALkeR said, I couldn’t find a suitable name, that’s all. I don’t think util is a terrible home for this, though.

@evanlucas @ChALkeR @TimothyGu Addressed your nits :)

doc/api/util.md Outdated
@@ -943,11 +1450,11 @@ added: v0.6.0
deprecated: v4.0.0
-->

> Stability: 0 - Deprecated
> Stability: 0 - Deprecated: Use [`Array.isArray`][] instead.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe Array.isArray() as per our style guide note about parentheses after methods (+ below and in the bottom reference).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vsemozhetbyt Yup, done!

@@ -1014,7 +1521,7 @@ added: v0.6.0
deprecated: v4.0.0
-->

> Stability: 0 - Deprecated
> Stability: 0 - Deprecated: Use [`util.types.isDate()`][] instead.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

util.types.isDate() seems undocumented.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vsemozhetbyt Oops, thanks for catching this!

doc/api/util.md Outdated
@@ -1176,7 +1697,8 @@ added: v0.11.5
deprecated: v4.0.0
-->

> Stability: 0 - Deprecated
> Stability: 0 - Deprecated: Use
> `typeof value === 'object' && typeof value !== 'function' ` instead.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

&& value !== null?

Copy link
Contributor

@vsemozhetbyt vsemozhetbyt Jan 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+ Is not typeof value can be either'object' or 'function'?

> const v = ()=>{}
undefined
> typeof v === 'object'
false
> typeof v
'function'

doc/api/util.md Outdated
@@ -1202,7 +1724,8 @@ added: v0.11.5
deprecated: v4.0.0
-->

> Stability: 0 - Deprecated
> Stability: 0 - Deprecated:
> Use `typeof value !== 'object' && typeof value !== 'function'` instead.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

|| value === null ?

For example:

```js
util.types.isNativeError(new Error()); // Returns true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to have TypeError (and/or some other built-in Errors) here too.

doc/api/util.md Outdated
For example:

```js
util.types.isMapIterator((new Map())[Symbol.iterator]()); // Returns true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something that may be surprising to JavaScript developers unfamiliar with iterators is that the iterators returned from .keys(), .values(), .entries(), and [@@iterator]() in fact look the same from the outside. Maybe have them here as well would aid comprehension? (Ditto for Set.)

util.types.isUint8ClampedArray(new Float64Array()); // Returns false
```

### util.types.isUint16Array(value)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alphabetically (what other API docs do), Uint16 would sort earlier than Uint8. This is certainly as good of a place for exception as any, but I'd like to keep it consistent for now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same for util.types.isInt8Array(value).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do think this sorting order is a little less weird, tbh … 😄

doc/api/util.md Outdated

```js
async function checkModule(wasmBuffer) {
const { module } = await WebAssembly.compile(wasmBuffer);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would new WebAssembly.Module(wasmBuffer) be a better example because of its synchronicity?

If this asynchronous example is to be kept, then it seems MDN spec it should be const module = rather than const { module } =.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this asynchronous example is to be kept, then it seems MDN spec it should be const module = rather than const { module } =.

I don’t know why, but I don’t think V8 and MDN agree here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like you used compile here but instantiate in the test. Instantiate does indeed return { module, instance }.

doc/api/util.md Outdated
@@ -1176,7 +1697,8 @@ added: v0.11.5
deprecated: v4.0.0
-->

> Stability: 0 - Deprecated
> Stability: 0 - Deprecated: Use
> `typeof value === 'object' && typeof value !== 'function' ` instead.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is implemented as value !== null && typeof value === 'object'.

Maybe also mention that even though functions are also objects, isObject historically returned false even for functions, because it's broken.

The doc as-is doesn't make much sense.

added: REPLACEME
-->

Returns `true` if the value is a string object, e.g. created
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/string/symbol/

Also, no need for e.g. as Object() is the only way to get symbol objects in JavaScript (new Symbol() always throws an error).


(async function() {
const buffer = fixtures.readSync('test.wasm');
const { module } = await WebAssembly.instantiate(buffer, {});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto as the documentation change. You could use WebAssembly.Module's constructor to avoid the async-ness. If not, we don't really need to instantiate it. WebAssembly.compile should be sufficient.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I simply didn’t know you could just, you know, use new to construct a Module. ;)

const exclude = [
'Undefined', 'Null', 'NullOrUndefined', 'True', 'False', 'Name', 'String',
'Symbol', 'Function', 'Array', 'Object', 'Boolean', 'Number', 'Int32',
'Uint32', 'GeneratorObject'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have an existing check for Generator objects? In any case, I am interested in seeing a reliable JS equivalent for isGeneratorObject.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TimothyGu I’d guess you’d usually just check for presence of the methods? It’s not “reliable” in the sense that the V8 method is, but I didn’t see much benefit in including this, plus (and I could be completely wrong about that) I’d guess that this doesn’t work on transpiled generator functions anyway?

I’m happy to include it if you think it makes sense.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’d guess you’d usually just check for presence of the methods?

Doesn't this kind of defeats the whole point of having these functions available?

I’d guess that this doesn’t work on transpiled generator functions anyway?

True, but this wouldn't be the first function that doesn't either.

continue;
assert(`is${match[1]}` in types,
`util.types should provide check for Is${match[1]}`);
}
Copy link
Member

@TimothyGu TimothyGu Jan 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like this test!

];

if (v8_h === undefined) {
process.on('exit', () => common.skip('Could not read v8.h'));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

process.on('exit') wouldn't be necessary if the WebAssembly.Module is created synchronously.

doc/api/util.md Outdated
[`util.types.isDate()`]: #util_util_types_isdate_value
[`util.types.isNativeError()`]: #util_util_types_isnativeerror_value
[`util.types.isSharedArrayBuffer()`]: #util_util_types_issharedarraybuffer_value
[`TypedArray`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypedArray
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of ABC-order?

<a id="DEP0XXX"></a>
### DEP0XXX: process.binding('util').is[...] typechecks

Type: Documentation-only/`--pending-deprecation`
Copy link
Member

@ChALkeR ChALkeR Jan 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is --pending-deprecation the correct type here? It's the first such one, afaik.
DEP0005 is also a --pending-deprecation, but does not mention that explicitly.
/cc @jasnell, I suppose.

Also, --pending-deprecation (and it being a semver-minor) is not mentioned in COLLABORATOR_GUIDE.md#deprecations, but that's probably a separate issue.

As I understand, currently --pending-deprecation is just a subclass of doc-only deprecations.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opened #18417 for that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I understand, currently --pending-deprecation is just a subclass of doc-only deprecations.

I’m okay with seeing it that way, yeah. And I don’t mind being explicit about this here.

Copy link
Member

@ChALkeR ChALkeR Jan 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#18433 landed, so could you change this line to

Type: Documentation-only (supports [`--pending-deprecation`][])

for consistency?

Copy link
Member

@joyeecheung joyeecheung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with the wasm example/test changed to sync version

doc/api/util.md Outdated
For example:

```js
util.types.isDataView(new Date()); // Returns true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be util.types.isDate(new Date())

doc/api/util.md Outdated
added: REPLACEME
-->

Returns `true` if the value is a async function.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a async ->an async?

doc/api/util.md Outdated
util.types.isDataView(new Float64Array()); // Returns false
```

See also [`ArrayBuffer.isView`][].
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ArrayBuffer.isView -> ArrayBuffer.isView() (+ everywhere including bottom reference)?

doc/api/util.md Outdated

```js
const target = {};
const proxy = new Proxy({}, {});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe const proxy = new Proxy(target, {}); to make target more target?)

util.types.isUint8ClampedArray(new Float64Array()); // Returns false
```

### util.types.isUint16Array(value)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same for util.types.isInt8Array(value).

doc/api/util.md Outdated
[`Uint8ClampedArray`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Uint8ClampedArray
[`Uint16Array`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Uint16Array
[`Uint32Array`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Uint32Array
[`WeakSet`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/WeakSet
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be after WeakMap.

@vsemozhetbyt
Copy link
Contributor

Maybe it is due to big diff, but some old review comments are not hidden now and are associated with the wrong lines, so do not be confused:

#18415 (comment) (or https://github.com/nodejs/node/pull/18415/files#r164285093)

#18415 (comment) (or https://github.com/nodejs/node/pull/18415/files#r164287166)

@@ -894,6 +894,534 @@ encoded bytes.

The encoding supported by the `TextEncoder` instance. Always set to `'utf-8'`.

## util.types
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noting that this will have a named-import-in-ESM issue just like the fs.promises addition.

@@ -29,8 +29,48 @@ function isUint8Array(value) {
return TypedArrayProto_toStringTag(value) === 'Uint8Array';
}

function isUint8ClampedArray(value) {
return TypedArrayProto_toStringTag(value) === 'Uint8ClampedArray';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should likely note in the documentation how these checks are performed because types can lie about what they are if the user overrides Symbol.toStringTag.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is cached at Node.js startup and thus immune to userland overrides.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right right... completely zoned on the caching. Just verified locally that even preload script can't lie about this. Dismiss this comment then :-)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually... @addaleax ... this would be something worth adding to the tests... specifically, ensuring that a user-provided class overriding a built-in set within a preload module cannot lie and that the checks will come up as false.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a user-provided class overriding a built-in set within a preload module cannot lie

I’m not sure what you mean by that. I have added a test that checks that overriding Symbol.toStringTag doesn’t fool this.

if ((types.isArrayBufferView(value) ||
types.isAnyArrayBuffer(value)) && key.includes('Array')) {
continue;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test should also likely verify that the result is consistent across vm contexts... e.g. check that vm.runInNewContext('new Map()') passes the type check.

util.types.isBooleanObject(false); // Returns false
util.types.isBooleanObject(true); // Returns false
util.types.isBooleanObject(new Boolean(false)); // Returns true
util.types.isBooleanObject(new Boolean(true)); // Returns true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add:

util.types.isBooleanObject(Boolean(false)); // Returns false
util.types.isBooleanObject(Boolean(true)); // Returns false

added: REPLACEME
-->

Returns `true` if the value is a built-in [`DataView`][] instance.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if we should assert that it is 'built-in' in these. The objects can lie, and user code can create their own objects (polyfills) that are not actually built-ins.

doc/api/util.md Outdated
added: REPLACEME
-->

Returns `true` if the value is an async function.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

async function here could be a link to more documentation about async functions... perhaps a link over the MDN documentation?


The result generally does not make any guarantees about what kinds of
properties or behavior a value exposes in JavaScript. They are primarily
useful for addon developers who prefer to do type checking in JavaScript.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm... this last sentence is a bit content free. why would developers prefer to do type checking in JavaScript :-) ... I think it's better to say something about specific use cases such as "These are primarily useful for debugging" or something similar

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jasnell I can’t really agree … the “why” is, it’s a preference. I personally don’t think it’s a good idea, but it’s what Node decided to migrate to?

Saying “these are primarily useful for debugging” doesn’t really seem more helpful, because it doesn’t even say how you’d use these methods for debugging…

@@ -0,0 +1,99 @@
/*global SharedArrayBuffer*/
'use strict';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's already https://github.com/nodejs/node/blob/master/test/parallel/test-types.js. You might want to look into combining these two files.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TimothyGu I’ve just copied the contents into a block in this file, let me know if that doesn’t sound good to you. The tests have somewhat different structures, but I think that’s fine?

@addaleax addaleax removed the lib / src Issues and PRs related to general changes in the lib or src directory. label Mar 5, 2018
addaleax added a commit that referenced this pull request Mar 5, 2018
PR-URL: #18415
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
addaleax added a commit that referenced this pull request Mar 5, 2018
Provide public APIs for native typechecking that is actually useful.
The motivation for this is providing alternatives to userland
modules that would currently rely on `process.binding('util')`.

PR-URL: #18415
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
addaleax added a commit to addaleax/node that referenced this pull request Mar 5, 2018
addaleax added a commit that referenced this pull request Mar 5, 2018
Refs: #18415
PR-URL: #19149
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@MylesBorins
Copy link
Contributor

Should this be backported to v9.x-staging? If yes please follow the guide and raise a backport PR, if not let me know or add the dont-land-on label.

@MylesBorins
Copy link
Contributor

this would need to come with #19149

joyeecheung added a commit to joyeecheung/lodash that referenced this pull request Mar 23, 2018
- Use require('util').types instead of using process.binding('util')
  to get the type checking helpers
- Rename nodeUtil to nodeTypes since that is what it is for

Refs: nodejs/node#18415
jdalton pushed a commit to lodash/lodash that referenced this pull request Mar 23, 2018
- Use require('util').types instead of using process.binding('util')
  to get the type checking helpers
- Rename nodeUtil to nodeTypes since that is what it is for

Refs: nodejs/node#18415
addaleax added a commit to addaleax/node that referenced this pull request Mar 26, 2018
The old variants have been deprecated since b20af80.

Refs: nodejs#18415
trivikr pushed a commit that referenced this pull request Mar 28, 2018
The old variants have been deprecated since b20af80.

Refs: #18415

PR-URL: #19602
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Weijia Wang <starkwang@126.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request May 1, 2018
PR-URL: nodejs#18415
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request May 1, 2018
Provide public APIs for native typechecking that is actually useful.
The motivation for this is providing alternatives to userland
modules that would currently rely on `process.binding('util')`.

PR-URL: nodejs#18415
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request May 1, 2018
Refs: nodejs#18415
PR-URL: nodejs#19149
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request May 1, 2018
The old variants have been deprecated since b20af80.

Refs: nodejs#18415

PR-URL: nodejs#19602
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Weijia Wang <starkwang@126.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@BridgeAR BridgeAR mentioned this pull request May 1, 2018
4 tasks
MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
PR-URL: nodejs#18415
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
Provide public APIs for native typechecking that is actually useful.
The motivation for this is providing alternatives to userland
modules that would currently rely on `process.binding('util')`.

PR-URL: nodejs#18415
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
Refs: nodejs#18415
PR-URL: nodejs#19149
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@MylesBorins
Copy link
Contributor

Is this something we would want to backport to v8.x? Seems like a bunch of moving parts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-minor PRs that contain new features and should be released in the next minor version. util Issues and PRs related to the built-in util module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.