process.stdout should be WritableStream | tty.WriteStream
Opened this issue · 23 comments
From https://nodejs.org/api/tty.html#tty_tty:
When Node.js detects that it is being run inside a text terminal ("TTY") context, the process.stdin will, by default, be initialized as an instance of tty.ReadStream and both process.stdout and process.stderr will, by default be instances of tty.WriteStream. The preferred method of determining whether Node.js is being run within a TTY context is to check that the value of the process.stdout.isTTY property is true:
$ node -p -e "Boolean(process.stdout.isTTY)" true $ node -p -e "Boolean(process.stdout.isTTY)" | cat false
It's currently not possible to check process.stdout.isTTY because it is only typed as WritableStream. It should be WritableStream | tty.WriteStream.
The alternative check process.stdout instanceof tty.WriteStream is not possible either, because tty only exports interfaces, not the classes / constructors.
The same applies to process.stdin/tty.ReadStream
Seems like a good change. I think both the read and write streams should just be updated to the tty interfaces and not use streams directly.
They are totally different classes though. tty streams inherit from net.Socket
Ok, then what I should have said is to create new interfaces for this. I don't think you'd want a union here, TypeScript would still complain it doesn't exist until you do a type assertion then.
Edit: Sorry, you're right. Read the example wrong. You're definitely welcome to do a PR to quickly update as well 😄
Ok, so it turns out this is a real PITA to fix - any ideas? The issue is that the tty, net, stream, etc. are all modules by process it global. I wish there was a nice way in TypeScript to mix global with modules (like declare var process: Process from 'process' or something).
@blakeembrey shouldn't this work?
import {Process} from 'process';
declare const process: Process;Assuming Process is an interface
No. You can't do that because the second you put import at the top-level, it makes the definition an external module format and not global. That's sadly just the rules TypeScript uses.
And the other way around?
declare namespace NodeJS {
export interface Process {}
}
declare module 'process' {
type Process = NodeJS.Process;
export {Process};
}Yes, that's possible - but it requires moving all of stream, net, tty (just the pieces in use here) into the global NodeJS scope. It's actually the trick that's currently in use today, I just didn't want to make that change right away because it is a huge change.
And you cannot use import inside a namespace, right? I think that is the fundamental limitation here.
Correct 👍 But this isn't just in the namespace, process is a variable that needs declaring at the top-level. There's a few fundamental limitations here. I'm not sure, but maybe we can propose a change in TypeScript to curb this because it definitely isn't the first time.
My initial thought is a TypeScript directive that'll ensure the file is always considered global or external. /cc @mhegazy @RyanCavanaugh How feasible would it be to allow import at the top-level if there was something to declare this file as a global declaration?
@blakeembrey It could be a new triple-slash directive.
Alternatively, it would also be enough if we could import inside namespaces. Then we could declare the globals by referencing the type from the namespace.
declare const process: NodeJS.Process;
declare namespace NodeJS {
export {Process} from 'process';
}I don't understand the question -- TypeScript always considers files as global or module depending on presence of top-level import / export declarations. You can use declare global { if you want something to go into the global scope from within a module file.
So we would change the whole declaration file to a module, but then put the majority of it inside declare global. Interesting! Might make the whole code way more structured because we can put stream stuff inside the actual 'stream' module, tty stuff inside 'tty', process inside 'process' etc.
@blakeembrey Could we even make individual files for the core node modules that are purely external? And then have one index that imports everything and declares the globals?
@RyanCavanaugh And that's exactly our problem. We need to share an interface globally declare var process: Process and within modules declare module "process". The pain here comes from the fact that some things have a very big module dependency chain to work (E.g. tty -> net -> stream) which all have to be moved global just to make process work. However, if we could import from a module into the global, this would be possible without polluting the global namespace with a ton of things that don't exist.
@felixfbecker Unfortunately that's not possible either, because we need to put declare module "stream" so TypeScript can recognise them. So either way, all the files would need to be global.
Using declare global may be possible with 2.0. Might be worth trying but I think the issue here is how TypeScript would treat this file. Since it's now external, I believe the global declaration would only take effect after the externalised node.d.ts module is imported. Is that correct @RyanCavanaugh?
@RyanCavanaugh I just tried out your suggestion, but it doesn't work:
declare module 'process' {
export interface Process {}
}
import {Process} from 'process';
declare global {
const process: Process; // Error: Module augmentation cannot introduce new names in the top level scope.
}@felixfbecker I think the other issue with that is that import at the top-level makes the file external, which results in declare module "process" being an augmentation instead of a declaration of the original module. This, in turn, seems to result in errors with "process" not being found to import from to start with. At least on the latest nightly (which 2.0 does fix the module augmentation error you see) occurs with test.d.ts(4,23): error TS2307: Cannot find module 'process'..
That error can be circumvented by doing:
process.d.ts
export interface Process {}node.d.ts
import {Process} from './process';
declare global {
const process: Process; // Error: Module augmentation cannot introduce new names in the top level scope.
}The error I mentioned remains though.
@felixfbecker Correct, but then there's no "process" global module declared, only a ./process external module. The error you mentioned is fixed in 2.0.
The only way around all this may be to combine module files, references and imports. E.g. Write declare "process" {} in it's own file, use it as a reference, then import { Process } from 'process' like your example above.
Edit: However, none of this fixes the concerns mentioned in #19 (comment) because having node.d.ts as an external module probably won't be used how we want it to be.
@blakeembrey Besides the error that is fixed in TS2.0, I found a solution:
process.d.ts
declare module 'process' {
export interface Process {}
}node.d.ts
/// <reference path="./process.d.ts" />
import {Process} from 'process';
declare global {
declare const process: Process;
}Yes, that's the proposal I suggested above. But it does not fix the concern I expressed (also above).