feat(node): v13#40927
feat(node): v13#40927andrewbranch merged 2 commits intoDefinitelyTyped:masterfrom SimonSchick:feat/node-v13
Conversation
|
@SimonSchick Thank you for submitting this PR! Because this is a new definition, a DefinitelyTyped maintainer will be reviewing this PR in the next few days once the Travis CI build passes. In the meantime, if the build fails or a merge conflict occurs, I'll let you know. Have a nice day! |
|
@SimonSchick 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! |
|
@orta or anyone else, can you mark this as actually passing, the parallel npm install keeps breaking and it's incredibly frustrating. |
|
@Flarna for another round, nothing has really changed. |
|
Some dgram and customRequire test are missing in node12 folder. Also the #40912 merged a few minutes is missing. |
|
Hnnng. |
|
@Flarna can you elaborate on what is missing exactly? |
|
The dgram changes are not applicable since these are v13 specific. |
|
@Flarna can you perhaps make a PR against my branch, my head is spinning a little 😛 |
|
Nvm I think I got it together now. |
|
@SimonSchick 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! |
| declare module "module" { | ||
| export = NodeJS.Module; | ||
| import { URL } from "url"; | ||
| interface Module extends NodeJS.Module {} |
There was a problem hiding this comment.
Seems the move of Module causes problems in one of my applications.
I augment Module with internals but I'm not able to adapt it in a way compatible to your changes:
declare namespace NodeJS {
interface ModuleCache {
[resolvedModulePath: string]: NodeModule;
}
namespace Module {
function _load(request: string, parent: NodeModule | undefined, isMain?: boolean): any;
function _resolveFilename(request: string, parent: NodeModule | undefined): string;
var _cache: ModuleCache;
}
}
There was a problem hiding this comment.
I'm able to adapt my code by moving the augmentation into module "module" and change from import Module = require("module"); to import { Module } from "module";
declare module "module" {
namespace Module {
function _load(request: string, parent: NodeModule | undefined, isMain?: boolean): any;
function _resolveFilename(request: string, parent: NodeModule | undefined): string;
var _cache: NodeJS.ModuleCache;
}
}
|
@SimonSchick 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. |
|
@rbuckton plss |
|
@SimonSchick 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! |
|
@SimonSchick 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! |
|
No idea what the deal with the build failure with |
|
I just published |
|
I just published |
|
I just published |
|
I just published |
|
I just published |
|
I just published |
|
I just published |
|
I just published |
|
I just published |
|
Some other PRs had been merged since my last push, namely #41170 (cc @yahehe) and #41029 (cc @TooTallNate). I tried to avoid this by constantly rebasing but didn't have the time in the last couple days. |
|
@SimonSchick I’ll be around until ~3 PM PST today—if you need to do a time-sensitive update, ping me and I’ll prioritize it. |
|
@andrewbranch not until after christmas, if you have the time I suggest you simply apply the 2 commits manually/directly to the v12 folder, should be ~5-10min of work. |
|
There’s quite a long backlog of PRs to get through today, so I probably won’t get around to that. Trying to knock it down as low as I can before taking the next two days off. |
|
@andrewbranch nope, only PRs that affected node are: |
|
Sorry, I meant that I have a long backlog of PRs to review across all of DT 😄 |
|
Ah ok :) |
|
@SimonSchick Is the removal of |
Removed in DefinitelyTyped#40927, required by rxjs.
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.This ended up being quite a large PR and I want some feedback on if we can proceed with this or if this needs to be broken down further.
General list of changes:
constantsmodule to avoid duplication.NodeJSnamespace.clusterunique symbolEventEmitterclass with interface as it's purely virtual and only there for global declarations.extends EventEmitterorextends NodeJS.EventEmitter.Retry of #40438