chore(node): declutter NodeJS namespace#44814
Closed
SimonSchick wants to merge 1 commit intoDefinitelyTyped:masterfrom
SimonSchick:chore/node-declutter-global
Closed
chore(node): declutter NodeJS namespace#44814SimonSchick wants to merge 1 commit intoDefinitelyTyped:masterfrom SimonSchick:chore/node-declutter-global
SimonSchick wants to merge 1 commit intoDefinitelyTyped:masterfrom
SimonSchick:chore/node-declutter-global
Conversation
Contributor
Author
|
@Flarna tagging ya for early 👀, I also need some help with the error showing up in |
Contributor
|
👋 Hi there! I’ve run some quick measurements against master and your PR. These metrics should help the humans reviewing this PR gauge whether it might negatively affect compile times or editor responsiveness for users who install these typings. Let’s review the numbers, shall we? color-support/v*Comparison details for color-support/* 📊
It looks like nothing changed too much. I won’t post performance data again unless it gets worse. depd/v*Comparison details for depd/* 📊
It looks like nothing changed too much. I won’t post performance data again unless it gets worse. deprecate/v*Comparison details for deprecate/* 📊
It looks like nothing changed too much. I won’t post performance data again unless it gets worse. derhuerst__cli-on-key/v*Comparison details for derhuerst__cli-on-key/* 📊
It looks like nothing changed too much. I won’t post performance data again unless it gets worse. gulp-zip/v*Comparison details for gulp-zip/* 📊
It looks like nothing changed too much. I won’t post performance data again unless it gets worse. hapi-pino/v*Comparison details for hapi-pino/* 📊
It looks like nothing changed too much. I won’t post performance data again unless it gets worse. node/v*Comparison details for node/* 📊
It looks like nothing changed too much. I won’t post performance data again unless it gets worse. pkgcloud/v*Comparison details for pkgcloud/* 📊
It looks like nothing changed too much. I won’t post performance data again unless it gets worse. semantic-release/v*Comparison details for semantic-release/* 📊
It looks like nothing changed too much. I won’t post performance data again unless it gets worse. signale/v*Comparison details for signale/* 📊
It looks like nothing changed too much. I won’t post performance data again unless it gets worse. undertaker/v*Comparison details for undertaker/* 📊
It looks like nothing changed too much. I won’t post performance data again unless it gets worse. webpackbar/v*Comparison details for webpackbar/* 📊
It looks like nothing changed too much. I won’t post performance data again unless it gets worse. x-ray/v*Comparison details for x-ray/* 📊
It looks like nothing changed too much. I won’t post performance data again unless it gets worse. |
Contributor
|
@SimonSchick Thank you for submitting this PR! This is a live comment which I will keep updated. This PR touches some part of DefinitelyTyped infrastructure, so a DT maintainer will need to review it. This is rare — did you mean to do this? Code ReviewsThis PR adds a new definition, so it needs to be reviewed by a DT maintainer before it can be merged.
Status
Once every item on this list is checked, I'll ask you for permission to merge and publish the changes. Diagnostic Information: What the bot saw about this PR{
"type": "info",
"now": "-",
"pr_number": 44814,
"author": "SimonSchick",
"owners": [
"Yavanosta",
"danny8002",
"BendingBender",
"Toilal",
"jacobbubu",
"dudeofawesome",
"robertbullen",
"saboya",
"todd",
"BlooJeans",
"GoPro16",
"Microsoft",
"DefinitelyTyped",
"jkomyno",
"a-tarasyuk",
"alvis",
"r3nya",
"btoueg",
"brunoscheufler",
"smac89",
"touffy",
"DeividasBakanas",
"eyqs",
"Flarna",
"Hannes-Magnusson-CK",
"KSXGitHub",
"hoo29",
"kjin",
"ajafff",
"islishude",
"mwiktorczyk",
"mohsen1",
"n-e",
"octo-sniffle",
"galkin",
"parambirs",
"eps1lon",
"SimonSchick",
"ThomasdenH",
"WilcoBakker",
"wwwy3y3",
"samuela",
"kuehlein",
"j-oliveras",
"bhongy",
"chyzwar",
"trivikr",
"nguymin4",
"yoursunny",
"qwelias",
"ExE-Boss",
"Ryan-Willpower",
"peterblazejewicz",
"addaleax",
"JasonHK",
"dantman",
"ScriptSmith",
"heuperman",
"lgaticaq",
"djcsdy",
"resir014",
"kingdaro",
"rjoydip",
"lookapanda",
"tkqubo",
"GiedriusGrabauskas",
"sh0ji",
"rynclark",
"mtraynham"
],
"dangerLevel": "Infrastructure",
"headCommitAbbrOid": "22c922e",
"headCommitOid": "22c922e7b2758479b928d750d23c9cfb8d6f938b",
"mergeIsRequested": false,
"stalenessInDays": 28,
"lastCommitDate": "2020-06-02T06:09:43.000Z",
"reopenedDate": "2020-05-21T23:24:47.000Z",
"lastCommentDate": "2020-06-30T02:45:06.000Z",
"reviewLink": "https://github.com/DefinitelyTyped/DefinitelyTyped/pull/44814/files",
"hasMergeConflict": false,
"authorIsOwner": true,
"isFirstContribution": false,
"popularityLevel": "Critical",
"anyPackageIsNew": true,
"packages": [
"color-support",
"depd",
"deprecate",
"derhuerst__cli-on-key",
"gulp-zip",
"hapi-pino",
"net-keepalive",
"node",
"pkgcloud",
"semantic-release",
"signale",
"undertaker",
"webpackbar",
"x-ray"
],
"files": [
{
"filePath": "notNeededPackages.json",
"kind": "infrastructure"
},
{
"filePath": "types/color-support/index.d.ts",
"kind": "definition",
"package": "color-support"
},
{
"filePath": "types/depd/index.d.ts",
"kind": "definition",
"package": "depd"
},
{
"filePath": "types/deprecate/index.d.ts",
"kind": "definition",
"package": "deprecate"
},
{
"filePath": "types/derhuerst__cli-on-key/index.d.ts",
"kind": "definition",
"package": "derhuerst__cli-on-key"
},
{
"filePath": "types/gulp-zip/index.d.ts",
"kind": "definition",
"package": "gulp-zip"
},
{
"filePath": "types/hapi-pino/index.d.ts",
"kind": "definition",
"package": "hapi-pino"
},
{
"filePath": "types/net-keepalive/index.d.ts",
"kind": "definition",
"package": "net-keepalive"
},
{
"filePath": "types/net-keepalive/net-keepalive-tests.ts",
"kind": "test",
"package": "net-keepalive"
},
{
"filePath": "types/net-keepalive/tsconfig.json",
"kind": "package-meta",
"package": "net-keepalive"
},
{
"filePath": "types/net-keepalive/tslint.json",
"kind": "package-meta",
"package": "net-keepalive"
},
{
"filePath": "types/node/buffer.d.ts",
"kind": "definition",
"package": "node"
},
{
"filePath": "types/node/console.d.ts",
"kind": "definition",
"package": "node"
},
{
"filePath": "types/node/domain.d.ts",
"kind": "definition",
"package": "node"
},
{
"filePath": "types/node/events.d.ts",
"kind": "definition",
"package": "node"
},
{
"filePath": "types/node/globals.d.ts",
"kind": "definition",
"package": "node"
},
{
"filePath": "types/node/http.d.ts",
"kind": "definition",
"package": "node"
},
{
"filePath": "types/node/process.d.ts",
"kind": "definition",
"package": "node"
},
{
"filePath": "types/node/stream.d.ts",
"kind": "definition",
"package": "node"
},
{
"filePath": "types/node/test/process.ts",
"kind": "test",
"package": "node"
},
{
"filePath": "types/node/ts3.2/base.d.ts",
"kind": "definition",
"package": "node"
},
{
"filePath": "types/node/ts3.2/globals.d.ts",
"kind": "definition",
"package": "node"
},
{
"filePath": "types/node/ts3.2/process.d.ts",
"kind": "definition",
"package": "node"
},
{
"filePath": "types/node/util.d.ts",
"kind": "definition",
"package": "node"
},
{
"filePath": "types/pkgcloud/index.d.ts",
"kind": "definition",
"package": "pkgcloud"
},
{
"filePath": "types/semantic-release/index.d.ts",
"kind": "definition",
"package": "semantic-release"
},
{
"filePath": "types/signale/index.d.ts",
"kind": "definition",
"package": "signale"
},
{
"filePath": "types/undertaker/index.d.ts",
"kind": "definition",
"package": "undertaker"
},
{
"filePath": "types/webpackbar/v2/index.d.ts",
"kind": "definition",
"package": "webpackbar"
},
{
"filePath": "types/x-ray/index.d.ts",
"kind": "definition",
"package": "x-ray"
}
],
"hasDismissedReview": false,
"ciResult": "pass",
"lastReviewDate": "2020-06-09T16:45:20.000Z",
"reviewersWithStaleReviews": [
{
"reviewedAbbrOid": "560fc46",
"reviewer": "ExE-Boss",
"date": "2020-05-29T19:13:52Z"
}
],
"approvalFlags": 4,
"isChangesRequested": false
} |
Contributor
People who would have been pingedYavanosta danny8002 BendingBender Toilal jacobbubu dudeofawesome robertbullen saboya todd BlooJeans GoPro16 hertzg Microsoft DefinitelyTyped jkomyno a-tarasyuk alvis r3nya btoueg brunoscheufler smac89 touffy DeividasBakanas eyqs Flarna Hannes-Magnusson-CK KSXGitHub hoo29 kjin ajafff islishude mwiktorczyk mohsen1 n-e octo-sniffle galkin parambirs eps1lon ThomasdenH WilcoBakker wwwy3y3 samuela kuehlein j-oliveras bhongy chyzwar trivikr nguymin4 yoursunny qwelias ExE-Boss Ryan-Willpower peterblazejewicz addaleax JasonHK dantman ScriptSmith heuperman lgaticaq djcsdy resir014 kingdaro rjoydip lookapanda tkqubo GiedriusGrabauskas sh0ji rynclark mtraynham |
Contributor
|
@SimonSchick Unfortunately, this pull request currently has a merge conflict 😥. Please update your PR branch to be up-to-date with respect to master. Have a nice day! |
|
Inspecting the JavaScript source for this package found some properties that are not in the .d.ts files. derhuerst__cli-on-key (unpkg)was missing the following properties:
hapi-pino (unpkg)was missing the following properties:
pkgcloud (unpkg)was missing the following properties:
signale (unpkg)was missing the following properties:
as well as these 2 other properties...packageConfiguration, configuration |
Contributor
Author
|
@danger-public I don't understand what this bot is doing at all. |
Member
|
@SimonSchick for the 'dt-danger': |
Contributor
Author
|
Hey @Flarna |
Contributor
|
@SimonSchick sorry, I'm on vacation... will take a while. |
ExE-Boss
reviewed
May 29, 2020
Contributor
|
@ExE-Boss Thank you for reviewing this PR! The author has pushed new commits since your last review. Could you take another look and submit a fresh review? |
Contributor
|
@SimonSchick Unfortunately, this pull request currently has a merge conflict 😥. Please update your PR branch to be up-to-date with respect to master. Have a nice day! |
orta
approved these changes
Jun 9, 2020
Collaborator
orta
left a comment
There was a problem hiding this comment.
This looks good to me, I've just asked in our DT chat room for one more look over it
Collaborator
|
OK, looks like we're a bit skeptical of a change this big not breaking quite a few builds (the Buffer changes, and interface merging being prominent examples) with the recommendations being either:
|
Contributor
Author
|
Can you elaborate on the concerns with the buffer changes here? |
Contributor
Author
|
@orta also which chat room, is it https://gitter.im/DefinitelyTyped/DefinitelyTyped? |
Contributor
Author
|
@orta ping In any case I'm fine with holding off these rather sweeping changes till v15 hits. |
Collaborator
|
Sorry, my GH notifications for the DT org are basically useless when I'm on rotation. The idea was that we'd expect there to be a bunch of broken apps and libraries breaking due to these changes, and while the idea is good making it a part of a major bump means people will be OK with that |
Contributor
|
@orta Looks like the bot got stuck in an infinite loop, again. |
Contributor
Author
|
Closing for now, will reopen with v15. |
This was referenced Jul 2, 2020
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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:
tslint.jsoncontaining{ "extends": "dtslint/dt.json" }. If for reason the any rule need to be disabled, disable it for that line using// tslint:disable-next-line [ruleName]and not for whole package so that the need for disabling can be reviewed.If removing a declaration:
notNeededPackages.json.This PR aims to declutter the
NodeJSnamespace andgloba.d.tsby moving definitions into their respective modules.This includes:
events,console,buffer,domain.Events
There used to (and still is)
NodeJS.EventEmitterthis type was only created so it could be used for other global definitions,EventEmittershould be imported fromeventsinstead, a compability alias has been provided.Process
All process related definitions have been moved to
process.d.ts, most popular definitions have compatilibity aliases (eg.NodeJS.ProcessEnvetc.), all others have been moved toprocessand must be imported from there.Caveat: I can't seem to augmentFixed.HRTimefor bigint usage anymoreStream
As evident by my refactors to other packages, there is a general confusion between
NodeJS.WriteStream/ReadStream(the TTY variant) andNodeJS.ReadableStream/WritatableStream(pure stream).I have removed the TTY aliases and changed all imports in third party packages to use
ttyinstead.Buffer
No visible changes for consumers.
There appears to be a bug in TS that causes
NodeJS.ArrayBufferViewto not be resolved correctly, I'm looking into filing an issue for this.Domain
By moving process definitions into their respective module,
NodeJS.Domainis no longer needed and was removed, useimport { Domain } from 'domain';instead.Console
NodeJS.ConsoleConstructorOptionsandNodeJS.ConsoleConstructorhave been moved into theconsolemodule.For the future
NodeJS.EventEmitterand eventually deprecate it?streamto avoid confusion.