-
Notifications
You must be signed in to change notification settings - Fork 30.5k
node: revert node: prefixed modules
#52595
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
node: revert node: prefixed modules
#52595
Conversation
|
@SimonSchick Thank you for submitting this PR! This is a live comment which I will keep updated. This PR might touch some part of DefinitelyTyped infrastructure, so a DT maintainer will need to review it. This is rare — did you mean to do this? 4 packages in this PRNote: this PR touches too many files, and I didn't see all of them!
Code ReviewsBecause this is a widely-used package, a DT maintainer will need to review it 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. InactiveThis PR has been inactive for 7 days. Diagnostic Information: What the bot saw about this PR{
"type": "info",
"now": "-",
"pr_number": 52595,
"author": "SimonSchick",
"headCommitOid": "792e6796526a04a4fb1a9825ffc4176840c466e5",
"lastPushDate": "2021-04-26T22:58:43.000Z",
"lastActivityDate": "2021-04-28T05:07:18.000Z",
"maintainerBlessed": false,
"hasMergeConflict": false,
"isFirstContribution": false,
"tooManyFiles": true,
"popularityLevel": "Critical",
"pkgInfo": [
{
"name": "aws-lambda-fastify",
"kind": "edit",
"files": [
{
"path": "types/aws-lambda-fastify/aws-lambda-fastify-tests.ts",
"kind": "test"
}
],
"owners": [
"kentakang"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Well-liked by everyone"
},
{
"name": "body",
"kind": "edit",
"files": [
{
"path": "types/body/test/body-tests.ts",
"kind": "test"
}
],
"owners": [
"SachinShekhar"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Well-liked by everyone"
},
{
"name": "isomorphic-git__lightning-fs",
"kind": "edit",
"files": [
{
"path": "types/isomorphic-git__lightning-fs/index.d.ts",
"kind": "definition"
}
],
"owners": [
"Progyan1997"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Well-liked by everyone"
},
{
"name": "node",
"kind": "edit",
"files": [
{
"path": "types/node/assert.d.ts",
"kind": "definition"
},
{
"path": "types/node/async_hooks.d.ts",
"kind": "definition"
},
{
"path": "types/node/buffer.d.ts",
"kind": "definition"
},
{
"path": "types/node/child_process.d.ts",
"kind": "definition"
},
{
"path": "types/node/cluster.d.ts",
"kind": "definition"
},
{
"path": "types/node/console.d.ts",
"kind": "definition"
},
{
"path": "types/node/constants.d.ts",
"kind": "definition"
},
{
"path": "types/node/crypto.d.ts",
"kind": "definition"
},
{
"path": "types/node/dgram.d.ts",
"kind": "definition"
},
{
"path": "types/node/dns.d.ts",
"kind": "definition"
},
{
"path": "types/node/dns/promises.d.ts",
"kind": "definition"
},
{
"path": "types/node/domain.d.ts",
"kind": "definition"
},
{
"path": "types/node/events.d.ts",
"kind": "definition"
},
{
"path": "types/node/fs.d.ts",
"kind": "definition"
},
{
"path": "types/node/fs/promises.d.ts",
"kind": "definition"
},
{
"path": "types/node/http.d.ts",
"kind": "definition"
},
{
"path": "types/node/http2.d.ts",
"kind": "definition"
},
{
"path": "types/node/https.d.ts",
"kind": "definition"
},
{
"path": "types/node/inspector.d.ts",
"kind": "definition"
},
{
"path": "types/node/module.d.ts",
"kind": "definition"
},
{
"path": "types/node/net.d.ts",
"kind": "definition"
},
{
"path": "types/node/node-tests.ts",
"kind": "test"
},
{
"path": "types/node/os.d.ts",
"kind": "definition"
},
{
"path": "types/node/path.d.ts",
"kind": "definition"
},
{
"path": "types/node/perf_hooks.d.ts",
"kind": "definition"
},
{
"path": "types/node/process.d.ts",
"kind": "definition"
},
{
"path": "types/node/punycode.d.ts",
"kind": "definition"
},
{
"path": "types/node/querystring.d.ts",
"kind": "definition"
},
{
"path": "types/node/readline.d.ts",
"kind": "definition"
},
{
"path": "types/node/repl.d.ts",
"kind": "definition"
},
{
"path": "types/node/scripts/generate-inspector/inspector.d.ts.template",
"kind": "package-meta",
"suspect": "edited"
},
{
"path": "types/node/stream.d.ts",
"kind": "definition"
},
{
"path": "types/node/stream/promises.d.ts",
"kind": "definition"
},
{
"path": "types/node/string_decoder.d.ts",
"kind": "definition"
},
{
"path": "types/node/test/assert.ts",
"kind": "test"
},
{
"path": "types/node/test/async_hooks.ts",
"kind": "test"
},
{
"path": "types/node/test/buffer.ts",
"kind": "test"
},
{
"path": "types/node/test/child_process.ts",
"kind": "test"
},
{
"path": "types/node/test/cluster.ts",
"kind": "test"
},
{
"path": "types/node/test/constants.ts",
"kind": "test"
},
{
"path": "types/node/test/crypto.ts",
"kind": "test"
},
{
"path": "types/node/test/dgram.ts",
"kind": "test"
},
{
"path": "types/node/test/dns.ts",
"kind": "test"
},
{
"path": "types/node/test/events.ts",
"kind": "test"
},
{
"path": "types/node/test/fs.ts",
"kind": "test"
},
{
"path": "types/node/test/global.ts",
"kind": "test"
},
{
"path": "types/node/test/http.ts",
"kind": "test"
},
{
"path": "types/node/test/http2.ts",
"kind": "test"
},
{
"path": "types/node/test/module.ts",
"kind": "test"
},
{
"path": "types/node/test/net.ts",
"kind": "test"
},
{
"path": "types/node/test/os.ts",
"kind": "test"
},
{
"path": "types/node/test/path.ts",
"kind": "test"
},
{
"path": "types/node/test/perf_hooks.ts",
"kind": "test"
},
{
"path": "types/node/test/process.ts",
"kind": "test"
},
{
"path": "types/node/test/querystring.ts",
"kind": "test"
},
{
"path": "types/node/test/readline.ts",
"kind": "test"
},
{
"path": "types/node/test/repl.ts",
"kind": "test"
},
{
"path": "types/node/test/stream.ts",
"kind": "test"
},
{
"path": "types/node/test/string_decoder.ts",
"kind": "test"
},
{
"path": "types/node/test/timers.ts",
"kind": "test"
},
{
"path": "types/node/test/timers_promises.ts",
"kind": "test"
},
{
"path": "types/node/test/tls.ts",
"kind": "test"
},
{
"path": "types/node/test/tty.ts",
"kind": "test"
},
{
"path": "types/node/test/url.ts",
"kind": "test"
},
{
"path": "types/node/test/util.ts",
"kind": "test"
},
{
"path": "types/node/test/v8.ts",
"kind": "test"
},
{
"path": "types/node/test/vm.ts",
"kind": "test"
},
{
"path": "types/node/test/wasi.ts",
"kind": "test"
},
{
"path": "types/node/test/worker_threads.ts",
"kind": "test"
},
{
"path": "types/node/test/zlib.ts",
"kind": "test"
},
{
"path": "types/node/timers.d.ts",
"kind": "definition"
},
{
"path": "types/node/timers/promises.d.ts",
"kind": "definition"
},
{
"path": "types/node/tls.d.ts",
"kind": "definition"
},
{
"path": "types/node/trace_events.d.ts",
"kind": "definition"
},
{
"path": "types/node/ts3.6/assert.d.ts",
"kind": "definition"
},
{
"path": "types/node/ts3.6/node-tests.ts",
"kind": "test"
},
{
"path": "types/node/ts3.6/tslint.json",
"kind": "package-meta",
"suspect": "not [the expected form](https://github.com/DefinitelyTyped/DefinitelyTyped#user-content-linter-tslintjson) and not moving towards it"
},
{
"path": "types/node/tty.d.ts",
"kind": "definition"
},
{
"path": "types/node/url.d.ts",
"kind": "definition"
},
{
"path": "types/node/util.d.ts",
"kind": "definition"
},
{
"path": "types/node/v12/assert.d.ts",
"kind": "definition"
},
{
"path": "types/node/v12/async_hooks.d.ts",
"kind": "definition"
},
{
"path": "types/node/v12/buffer.d.ts",
"kind": "definition"
},
{
"path": "types/node/v12/child_process.d.ts",
"kind": "definition"
},
{
"path": "types/node/v12/cluster.d.ts",
"kind": "definition"
},
{
"path": "types/node/v12/console.d.ts",
"kind": "definition"
},
{
"path": "types/node/v12/constants.d.ts",
"kind": "definition"
},
{
"path": "types/node/v12/crypto.d.ts",
"kind": "definition"
},
{
"path": "types/node/v12/dgram.d.ts",
"kind": "definition"
},
{
"path": "types/node/v12/dns.d.ts",
"kind": "definition"
},
{
"path": "types/node/v12/domain.d.ts",
"kind": "definition"
},
{
"path": "types/node/v12/events.d.ts",
"kind": "definition"
},
{
"path": "types/node/v12/fs.d.ts",
"kind": "definition"
},
{
"path": "types/node/v12/http.d.ts",
"kind": "definition"
},
{
"path": "types/node/v12/http2.d.ts",
"kind": "definition"
},
{
"path": "types/node/v12/https.d.ts",
"kind": "definition"
},
{
"path": "types/node/v12/inspector.d.ts",
"kind": "definition"
}
],
"owners": [
"Microsoft",
"DefinitelyTyped",
"jkomyno",
"alvis",
"r3nya",
"btoueg",
"smac89",
"touffy",
"DeividasBakanas",
"eyqs",
"Hannes-Magnusson-CK",
"KSXGitHub",
"hoo29",
"kjin",
"ajafff",
"islishude",
"mwiktorczyk",
"mohsen1",
"n-e",
"galkin",
"parambirs",
"eps1lon",
"SimonSchick",
"ThomasdenH",
"WilcoBakker",
"wwwy3y3",
"samuela",
"kuehlein",
"bhongy",
"chyzwar",
"trivikr",
"nguymin4",
"yoursunny",
"qwelias",
"ExE-Boss",
"Ryan-Willpower",
"peterblazejewicz",
"addaleax",
"JasonHK",
"victorperin",
"ZYSzys"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Critical"
}
],
"reviews": [
{
"type": "approved",
"reviewer": "SachinShekhar",
"date": "2021-04-28T05:07:18.000Z",
"isMaintainer": false
},
{
"type": "approved",
"reviewer": "G-Rath",
"date": "2021-04-28T03:14:00.000Z",
"isMaintainer": false
},
{
"type": "approved",
"reviewer": "Progyan1997",
"date": "2021-04-27T11:25:40.000Z",
"isMaintainer": false
}
],
"ciResult": "pass"
} |
|
🔔 @kentakang @SachinShekhar @Progyan1997 @microsoft @DefinitelyTyped @jkomyno @alvis @r3nya @btoueg @BrunoScheufler @smac89 @Touffy @DeividasBakanas @eyqs @Hannes-Magnusson-CK @KSXGitHub @hoo29 @kjin @ajafff @islishude @mwiktorczyk @mohsen1 @n-e @galkin @parambirs @eps1lon @ThomasdenH @WilcoBakker @wwwy3y3 @samuela @kuehlein @bhongy @chyzwar @trivikr @nguymin4 @yoursunny @qwelias @ExE-Boss @Ryan-Willpower @peterblazejewicz @addaleax @JasonHK @victorperin @ZYSzys — please review this PR in the next few days. Be sure to explicitly select |
|
Note that I’d prefer if we tried moving the |
|
@SimonSchick The CI build failed! Please review the logs for more information. Once you've pushed the fixes, the build will automatically re-run. Thanks! |
|
👋 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? node/v15.0Comparison details for node/15.0 📊
First off, note that the system varied slightly between these two runs, so you’ll have to take these measurements with a grain of salt. It looks like nothing changed too much. I won’t post performance data again unless it gets worse. node/v15.0Comparison details for node/15.0 📊
It looks like nothing changed too much. I won’t post performance data again unless it gets worse. node/v15.0These typings are for a version of node that doesn’t yet exist on master, so I’ve compared them with v14.14. Comparison details for node/15.0 📊
First off, note that the system varied slightly between these two runs, so you’ll have to take these measurements with a grain of salt. It looks like nothing changed too much. I won’t post performance data again unless it gets worse. |
|
I tried this previously but saw no change on resolve order based on declaration order but you're welcome to check this yourself. Furthermore I think it would make more sense to wait for official support for module aliases from the TS team, the |
|
@SimonSchick The CI build failed! Please review the logs for more information. Once you've pushed the fixes, the build will automatically re-run. Thanks! |
|
@SimonSchick The CI build failed! Please review the logs for more information. Once you've pushed the fixes, the build will automatically re-run. Thanks! |
|
Updated numbers for you here from 8f728c8. aws-lambda-fastify/v1.4Comparison details for aws-lambda-fastify/1.4 📊
First off, note that the system varied slightly between these two runs, so you’ll have to take these measurements with a grain of salt. It looks like nothing changed too much. I won’t post performance data again unless it gets worse. body/v5.1Comparison details for body/5.1 📊
First off, note that the system varied slightly between these two runs, so you’ll have to take these measurements with a grain of salt. It looks like nothing changed too much. I won’t post performance data again unless it gets worse. isomorphic-git__lightning-fs/v4.4Comparison details for isomorphic-git__lightning-fs/4.4 📊
First off, note that the system varied slightly between these two runs, so you’ll have to take these measurements with a grain of salt. Looks like there were a couple significant differences—take a look at worst-case duration for getting completions at a position to make sure everything looks ok. node/v15.0Comparison details for node/15.0 📊
First off, note that the system varied slightly between these two runs, so you’ll have to take these measurements with a grain of salt. It looks like nothing changed too much. I won’t post performance data again unless it gets worse. node/v15.0Comparison details for node/15.0 📊
It looks like nothing changed too much. I won’t post performance data again unless it gets worse. node/v15.0These typings are for a version of node that doesn’t yet exist on master, so I’ve compared them with v14.14. Comparison details for node/15.0 📊
It looks like nothing changed too much. I won’t post performance data again unless it gets worse. |
|
Updated numbers for you here from d217990. Nice job, these numbers look better. aws-lambda-fastify/v1.4Comparison details for aws-lambda-fastify/1.4 📊
It looks like nothing changed too much. I won’t post performance data again unless it gets worse. body/v5.1Comparison details for body/5.1 📊
It looks like nothing changed too much. I won’t post performance data again unless it gets worse. isomorphic-git__lightning-fs/v4.4Comparison details for isomorphic-git__lightning-fs/4.4 📊
It looks like nothing changed too much. I won’t post performance data again unless it gets worse. node/v15.0Comparison details for node/15.0 📊
First off, note that the system varied slightly between these two runs, so you’ll have to take these measurements with a grain of salt. It looks like nothing changed too much. I won’t post performance data again unless it gets worse. node/v15.0Comparison details for node/15.0 📊
It looks like nothing changed too much. I won’t post performance data again unless it gets worse. node/v15.0These typings are for a version of node that doesn’t yet exist on master, so I’ve compared them with v14.14. Comparison details for node/15.0 📊
First off, note that the system varied slightly between these two runs, so you’ll have to take these measurements with a grain of salt. It looks like nothing changed too much. I won’t post performance data again unless it gets worse. |
0xTheProDev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 for Lighting FS changes. 💯
|
I just published |
|
I just published |
|
I just published |
|
I just published |
|
@SimonSchick @elibarzilay Uh, is this supposed to cause applications using these prefixes to break on a minor upgrade of the types ( This is causing errors: These were working just fine before. cc @rbuckton |
|
Since the Edit: It has been backported: #52595 (comment) |
|
As someone who has used these type definitions in the last week and auto-imports of the |
|
@rbuckton is there an approach that would achieve both? Eg. all 3 of:
Or is this microsoft/TypeScript#42764 ? |
node: prefixes are no longer working due to DefinitelyTyped/DefinitelyTyped#52595
|
For those who are looking to ban all new versions of https://github.com/karlhorky/test-renovate-config-ban-types-versions/blob/main/renovate.json See an example of this config autoclosing the upgrade here: karlhorky/test-renovate-config-ban-types-versions#2 |
|
@rbuckton @SimonSchick What about the following as a temporary solution: Release Then Node 16 users who want to use the prefixes still have a latest major version that they can install (which also matches the Node version number). And until a better solution is in place, anyone who doesn't want the prefixes can stay on the 12.x, 14.x or 15.x lines. |
|
Update: there is now a PR for Node v16: #52594 Hopefully the |
|
So looks like the https://nodejs.org/api/esm.html -> link "node: Imports" (links with anchors don't scroll properly right now) |
|
So maybe now this "revert" PR can be reverted and the feature can be back :) |
|
It's supported when outputting to ESM, but not to CommonJS. That was the issue I (and I think most others) had with the In Node 16 |
Ah, good point! Didn't catch this detail. |
|
|

Please fill in this template.
npm test <package to test>.Select one of these and delete the others:
If changing an existing definition:
Revert of #51107 as discussed we should bring this back once microsoft/TypeScript#42764 has matured.