0

We're trying to convert the function PSRunner to a TypeScript function:

export function PSRunner(commands: string[]) {
  const self: {
    out: string[]
    err: string[]
  } = this
  const results: { command: string; output: any; errors: any }[] = []
  const child = spawn('powershell.exe', ['-Command', '-'])

  child.stdout.on('data', function (data) {
    self.out.push(data.toString())
  })
  child.stderr.on('data', function (data) {
    self.err.push(data.toString())
  })

  commands.forEach(function (cmd) {
    self.out = []
    self.err = []
    child.stdin.write(cmd + '\n')
    results.push({ command: cmd, output: self.out, errors: self.err })
  })
  child.stdin.end()
  return results
}

I'm a bit confused with how this works. An object literal is created called self. This is then populated with data coming from child.stdout and child.stderr. Later on, in the foreach, the this.out and this.err are set to an empty array again.

How can result then hold the values related to that one specific command? I would try to use a fat arrow function to avoid having to use this, but in this case it might be required?

There are also some TS errors with regards to not use any. But I would like to understand first how this works. Thank you for any clarifications.

DarkLite1
  • 13,637
  • 40
  • 117
  • 214
  • In short: yes, you can use an arrow function. The initial statement is just `self = this`, so it's aliasing it for the purpose of reusing `self` later. An arrow function for the callbacks will preserve the value of `this`. You change the function signature to `function PSRunner(this: { out: string[]; err: string[]: }, commands: string[])` and remove `self`. – VLAZ Mar 24 '21 at 09:59
  • _"Later on, in the foreach, the `this.out` and `this.err` are set to an empty array again"_ - You got the order wrong. The `child.stdout.on()` and `child.stderr.on()` calls add an event handler. The `.forEach()` is executed before any of the event handlers is triggered. – Andreas Mar 24 '21 at 10:02
  • It would be nicer to just have one argument for the function, being `commands`. Would it also work if we replace `self` with just an `const out:` and `const err`? – DarkLite1 Mar 24 '21 at 10:03
  • @Andreas so if there is output for a single command, it will not be tied to the output of that command in the array `results`, but only the last item in `results` will hold all errors and output for all commands? Because `self.out` and `self.err` are always reset until the last command. – DarkLite1 Mar 24 '21 at 10:08
  • 1
    If `.write()` causes the `data` event to fire then `output` and `errors` will only contain the "output" of the last command. If there isn't anything executed until `.end()` then all commands in `results` will have the output of all the commands (because they all reference the exact same array) – Andreas Mar 24 '21 at 10:14
  • @Andreas Thank you that helps :) So we can also remove `const self = this` and just use `commands.forEach(function(cmd) { let out=[]; let err=[]; child.stdin.write(cmd + '\n'); results.push({ command: cmd, output: out, errors: err })}` ? – DarkLite1 Mar 24 '21 at 10:17
  • 1
    @DarkLite1 the semantics are different, the functions are *not* equivalent. But whether that matters or not, I cannot say because I don't know how this function is being called. – VLAZ Mar 24 '21 at 10:18
  • This would create new arrays for every command and you won't get any results at all. With the doubts from my previous comment you might be able to replace `const self = this` with `const out = []; const err = [];`, use these in the `.forEach()` and (with an arrow function) with `this.out`/`this.err` in the event handlers. But why? The only real change I would move the `self.out = []; self.err = []` part in the `.forEach()` before it. One reset is enough. – Andreas Mar 24 '21 at 11:33
  • I just can't get my head around it. The event handlers `child.stdout.on` and `child.stderr.on` are adding to the same array. So it's not possible to keep output and error tied to the executed command when tere are multiple. Also, the last errors and outputs are only collected when executing `child.stdin.end()`. Feel free to post an answer with a function that does have a command with only its proper output and errors. – DarkLite1 Mar 24 '21 at 12:51

1 Answers1

0

Regarding the question about any, I wouldn't expect that to cause an actual error (by which I mean it won't transpile due to syntax errors), but I would expect a linter plugin to be complaining about it.

In general, you want to either use unknown instead of any if what you really mean is "I won't know until runtime what object actually gets bound to this type", because any will be far too lenient in letting you reference members that may or may not exist, whereas unknown is a little more strict (and type-safe).

Before resorting to unknown however, it's generally preferable to define an actual type, even something like Record<string, string> (an object with string properties mapping to string values). Or using a generic type if you need something a little more dynamic (although, not the case here).

In fact, recently typescript made a change where a caught exception is promoted to be unknown as opposed to the former any, due to type safety. https://www.typescriptlang.org/docs/handbook/release-notes/typescript-4-0.html#unknown-on-catch-clause-bindings

In general, avoid any, and any good linter will tell you the same. I assume that's what your error was about. Correct me if I was mistaken, though.

acat
  • 596
  • 6
  • 15