@types/node: Add missing methods to Console, plus docs#25327
@types/node: Add missing methods to Console, plus docs#253272 commits merged intoDefinitelyTyped:masterfrom
Conversation
|
If there's a better way to handle "this method only works with |
typescript-bot
added
the
Popular package
|
@thw0rted Thank you for submitting this PR! 🔔 @parambirs @tellnes @WilcoBakker @octo-sniffle @smac89 @Flarna @mwiktorczyk @wwwy3y3 @DeividasBakanas @kjin @alvis @OliverJAsh @eps1lon @Hannes-Magnusson-CK @jkomyno @ajafff @hoo29 @n-e - please review this PR in the next few days. Be sure to explicitly select If no reviewer appears after a week, a DefinitelyTyped maintainer will review the PR instead. |
| /** | ||
| * The `console.dirxml()` function is an alias for {@link console.log()}. | ||
| */ | ||
| dirxml(...data: any[]): void; |
There was a problem hiding this comment.
Why is there a difference in the type definition of dirxml to log if one is just an alias of the other?
There was a problem hiding this comment.
This is based on the Node docs, which say that the method signature is ...data <any>, and that it "calls console.log() passing it the arguments received". The official spec agrees with Node's definition, even though the MDN page says it's "non-standard" and only takes exactly one argument. Of course, logically foo(...everything: any[]) works identically to foo(first?: any, ...others: any[]) (from the caller's perspective) so it's kind of academic.
All that to say: I copied it from other similar methods (like debug) and probably should have used the original docs from Node,
This method calls console.log() passing it the arguments received. Please note that this method does not produce any XML formatting.
There was a problem hiding this comment.
I found following in NodeJS Source:
Console.prototype.info = Console.prototype.log;
Console.prototype.dirxml = Console.prototype.log;```
So seems there is really no difference between them.
But you are right, for the user it's don't care which variant is used here in the typings.
|
@thw0rted The Travis CI build failed! Please review the logs for more information. Once you've pushed the fixes, the build will automatically re-run. Thanks! |
|
Looks like your Travis is jacked, the failures are unrelated to these changes. Who can address? |
|
Hey @Flarna and others watching, I have a DT / node question and figured it probably wasn't worth opening a new issue / PR until I understand why things are the way they are. For a long time, Node's typings have included a If my guess is right, I'll make a new PR (maybe after this one merges successfully?) that does this. |
|
@thw0rted I haven't seen anything from you in a while and this PR currently has problems that prevent it from being merged. The PR will be closed tomorrow if there aren't new commits to fix the issues. |
|
A definition owner has approved this PR ⭐️. A maintainer will merge this PR shortly. If it shouldn't be merged yet, please leave a comment saying so and we'll wait. Thank you for your contribution to DefinitelyTyped! |
ghost
merged commit …ed#25327) * Add missing methods to node Console, plus docs * Update Node console dirxml per review comment
3 participants
Please fill in this template.
npm test.)npm run lint package-name(ortscif notslint.jsonis present).Select one of these and delete the others:
If changing an existing definition:
node 9.X, node 10.X