DefinitelyTyped / DefinitelyTyped

The repository for high quality TypeScript type definitions.
Other
48.53k stars 30.15k forks source link

types/node: make util.inspect.custom a 'unique symbol', not only a 'symbol'? #30241

Closed emmanueltouzery closed 3 years ago

emmanueltouzery commented 6 years ago

In node, if we want custom rendering for custom objects in the REPL and in console.log, we used to have to override inspect(). In recent versions of node, we must instead override [util.inspect.custom], which is a symbol. See the documentation: https://nodejs.org/api/util.html#util_util_inspect_custom [update: better link]

The implementation of custom was changed recently as well: https://github.com/nodejs/node/commit/dadd6e16888baac8fd110432b81f3fd1237be3e1#diff-a43208147d795be6dd3517c53226e37dL402

I think that in the type definitions, the type of custom should not be symbol as it is now, but unique symbol. The reason being that we can't define class members with [util.inspect.custom] as it is, as the compiler says:

error TS1169: A computed property name in an interface must refer to an expression whose type is a literal type or a 'unique symbol' type.

My current workaround is to say...

import * as util from 'util';

// @ts-ignore
export const inspect: unique symbol = util.inspect.custom;

and later [inspect](): string. Even casting to any is not enough, I need @ts-ignore.

PS: I tried to make a PR and failed. It was https://github.com/DefinitelyTyped/DefinitelyTyped/pull/30237

SimonSchick commented 6 years ago

Not going to work given the fact that the node typings have to support a very old version of TS.

emmanueltouzery commented 6 years ago

it's really not rare that I wish that there was some #ifdef mechanism in typescript, with version constraints :-(

It's quite painful as it is now. One would wish to define the constant oneself, ignoring the one exported by node, but as you can see in the commit I pasted, they've been changing it through time.. Using the constant is the best way to be compatible with multiple node versions. Well, worst-case I guess the @ts-ignore solution works, but...

ZaneHannanAU commented 6 years ago

Ugh… At this point I think we should deprecate most old/unused/otherwise broken versions…

TheAkio commented 5 years ago

This seems to be fixed with newer node typings. I'm using @types/node with version 10.14.5 and I don't get this error anymore, but I got the error with some 10.12.X typings.

EDIT: Looked into the repository a bit. The updated definitions for TypeScript 3.1 originally (later they moved it to the 3.2 folder) were added on Febuary 7th:

https://github.com/DefinitelyTyped/DefinitelyTyped/blob/77a6c2a9706e17d83a3623f2b1686aafa383e27b/types/node/ts3.1/util.d.ts

orta commented 3 years ago

Hi thread, we're moving DefinitelyTyped to use GitHub Discussions for conversations the @types modules in DefinitelyTyped.

To help with the transition, we're closing all issues which haven't had activity in the last 6 months, which includes this issue. If you think closing this issue is a mistake, please pop into the TypeScript Community Discord and mention the issue in the definitely-typed channel.