Converting to ts and promises?

[OmgImAlexis]

Wanted to discuss moving over to typescript and possibly a code cleanup as I'm noticing a lot of the code is heavily nested using a lot of callbacks.

[OmgImAlexis]

Here's an example with the audio.js converted over.

// ==================================================================================
// audio.js
// ----------------------------------------------------------------------------------
// Description:   System Information - library
//                for Node.js
// Copyright:     (c) 2014 - 2021
// Author:        Sebastian Hildebrandt
// ----------------------------------------------------------------------------------
// License:       MIT
// ==================================================================================
// 16. audio
// ----------------------------------------------------------------------------------

import { execShellCommand } from './common/exec-shell-command';
import { execWmicCommand } from './common/exec-wmic-command';
import { getValue } from './common/get-value';

const _platform = process.platform;
const _linux = (_platform === 'linux');
const _darwin = (_platform === 'darwin');
const _windows = (_platform === 'win32');
const _freebsd = (_platform === 'freebsd');
const _openbsd = (_platform === 'openbsd');
const _netbsd = (_platform === 'netbsd');

export const parseAudioType = (str: string, input: boolean | null, output: boolean | null) => {
  if (str.indexOf('speak') >= 0) return 'Speaker';
  if (str.indexOf('laut') >= 0) return 'Speaker';
  if (str.indexOf('loud') >= 0) return 'Speaker';
  if (str.indexOf('head') >= 0) return 'Headset';
  if (str.indexOf('mic') >= 0) return 'Microphone';
  if (str.indexOf('mikr') >= 0) return 'Microphone';
  if (str.indexOf('phone') >= 0) return 'Phone';
  if (str.indexOf('controll') >= 0) return 'Controller';
  if (str.indexOf('line o') >= 0) return 'Line Out';
  if (str.indexOf('digital o') >= 0) return 'Digital Out';
  if (output) return 'Speaker';
  if (input) return 'Microphone';
  return undefined;
};

export const getLinuxAudioPci = async () => {
  const result = [];
  try {
    const parts = await execShellCommand('lspci -v 2>/dev/null').then(stdout => stdout.split('\n\n'));
    for (let i = 0; i < parts.length; i++) {
      const lines = parts[i].split('\n');
      if (lines && lines.length && lines[0].toLowerCase().indexOf('audio') >= 0) {
        result.push({
          slotId: lines[0].split(' ')[0],
          driver: getValue(lines, 'Kernel driver in use', ':', true) || getValue(lines, 'Kernel modules', ':', true)
        });
      }
    }
    return result;
  } catch {
    return result;
  }
}

type AudioPCI = {
  slotId: any;
  driver: string;
};

const parseLinuxAudioPciMM = (lines: string[], audioPCI: AudioPCI[]) => {
  const slotId = getValue(lines, 'Slot');
  const pciMatch = audioPCI.find(item => item.slotId === slotId);
  const name = getValue(lines, 'SDevice');

  return {
    id: slotId,
    name,
    manufacturer: getValue(lines, 'SVendor'),
    revision: getValue(lines, 'Rev'),
    driver: pciMatch?.driver,
    default: null,
    channel: 'PCIe',
    type: parseAudioType(name, null, null),
    in: null,
    out: null,
    status: 'online'
  };
};

const parseDarwinChannel = (str: string) => {
  if (str.indexOf('builtin') >= 0) return 'Built-In';
  if (str.indexOf('extern') >= 0) return 'Audio-Jack';
  if (str.indexOf('hdmi') >= 0) return 'HDMI';
  if (str.indexOf('displayport') >= 0) return 'Display-Port';
  if (str.indexOf('usb') >= 0) return 'USB';
  if (str.indexOf('pci') >= 0) return 'PCIe';
  return undefined;
}

type AudioObject = {
  _name: string;
  coreaudio_device_transport: string;
  coreaudio_device_manufacturer: string;
  coreaudio_default_audio_input_device: string;
  coreaudio_default_audio_output_device: string;
  coreaudio_device_input: string;
  coreaudio_device_output: string;
};

const parseDarwinAudio = (audioObject: AudioObject, id: number) => {
  const name = audioObject._name;
  const channelStr = ((audioObject.coreaudio_device_transport || '') + ' ' + (name || '')).toLowerCase();

  return {
    id,
    name,
    manufacturer: audioObject.coreaudio_device_manufacturer,
    revision: null,
    driver: null,
    default: Boolean(audioObject.coreaudio_default_audio_input_device || '') || Boolean(audioObject.coreaudio_default_audio_output_device || ''),
    channel: parseDarwinChannel(channelStr),
    type: parseAudioType(name, Boolean(audioObject.coreaudio_device_input || ''), Boolean(audioObject.coreaudio_device_output || '')),
    in: Boolean(audioObject.coreaudio_device_input || ''),
    out: Boolean(audioObject.coreaudio_device_output || ''),
    status: 'online'
  };
};

const parseWindowsAudio = (lines: string[]) => {
  const status = getValue(lines, 'StatusInfo', '=');
  const name = getValue(lines, 'name', '=');

  return {
    id: getValue(lines, 'DeviceID', '='), // PNPDeviceID??
    name,
    manufacturer: getValue(lines, 'manufacturer', '='),
    revision: null,
    driver: null,
    default: null,
    channel: null,
    type: parseAudioType(name, null, null),
    in: null,
    out: null,
    status
  };
};

const nixAudio = async () => {
  const stdout = await execShellCommand('lspci -vmm 2>/dev/null');
  const audioPCI = await getLinuxAudioPci();
  const parts = stdout.toString().split('\n\n');
  const result = [];
  for (let i = 0; i < parts.length; i++) {
    const lines = parts[i].split('\n');
    if (getValue(lines, 'class', ':', true).toLowerCase().indexOf('audio') >= 0) {
      const audio = parseLinuxAudioPciMM(lines, audioPCI);
      result.push(audio);
    }
  }

  return result;
};

const darwinAudio = async () => {
  const stdout = await execShellCommand('system_profiler SPAudioDataType -json');
  const result = [];
  try {
    const outObj = JSON.parse(stdout.toString());
    if (outObj.SPAudioDataType && outObj.SPAudioDataType.length && outObj.SPAudioDataType[0] && outObj.SPAudioDataType[0]['_items'] && outObj.SPAudioDataType[0]['_items'].length) {
      for (let i = 0; i < outObj.SPAudioDataType[0]['_items'].length; i++) {
        const audio = parseDarwinAudio(outObj.SPAudioDataType[0]['_items'][i], i);
        result.push(audio);
      }
    }
  } catch {}
  return result;
}

const windowsAudio = async () => {
  const stdout = await execWmicCommand('path Win32_SoundDevice get /value');
  const parts = stdout.toString().split(/\n\s*\n/);
  const result = [];
  for (let i = 0; i < parts.length; i++) {
    if (getValue(parts[i].split('\n'), 'name', '=')) {
      result.push(parseWindowsAudio(parts[i].split('\n')));
    }
  }

  return result;
}

export const audio = new Promise(resolve => {
  process.nextTick(() => {
    switch (true) {
      case _linux || _freebsd || _openbsd || _netbsd:
        return resolve(nixAudio());
      case _darwin:
        return resolve(darwinAudio());
      case _windows:
        return resolve(windowsAudio());
      default:
        return resolve(null);
    }
  });
});

[sebhildebrandt]

@OmgImAlexis this is something that I have on top of my list for version 6. I started to make first conversions and of course there is plenty of room for code cleanup! Is this an urgent issue for you?

[OmgImAlexis]

@sebhildebrandt really glad to hear that. 😄

I have some holidays coming up and was thinking if all good I could refactor it during a few of those days. I have a project I'm working on and full typescript/promise support would really help but it's not urgent.

[sebhildebrandt]

@OmgImAlexis maybe let's do it this way: I will open a new V6 branch and share all what I did in this new branch. I do not want to waste your time / your holidays cleaning up my mess ;-) As I have a conference next week I will try to provide my converted code next weekend. OK for you?

[OmgImAlexis]

Sounds good to me. 👍

Could you close and ping me in this issue once that branch is up?

[OmgImAlexis]

@sebhildebrandt any updates on this? I'm now free from work for the next 15 days. 👍

[OmgImAlexis]

@sebhildebrandt could you setup the v6 branch today?

[sebhildebrandt]

@OmgImAlexis working on it - but I need to make some refactoring concerning wmic which I would like to finish first. I hope I'm not stretching your patience too far ...

[OmgImAlexis]

@sebhildebrandt no worries, just wanting to get working on it. 👍

[OmgImAlexis]

Guessing once that's it we'll do a code(feature) freeze until we've converted it to ts?

[sebhildebrandt]

@OmgImAlexis kind of ... I thought about that a lot the last few days and as I do not want to maintain two different versions this all needs a little bit of planning - which I am currently trying to do ;-)

[OmgImAlexis]

@sebhildebrandt any updates on the plan yet? 😃

[OmgImAlexis]

@sebhildebrandt apart from the WMIC changes is there anything else holding this back? Really looking forward to working on this.

[sebhildebrandt]

@OmgImAlexis ... I currently do a lot of cleanup and refactoring during this transitioning ... takes more time than expected ... I have to be very careful to consider all breaking changes that I had in mind NOW - otherwise it would again need a major version update.

[sebhildebrandt]

@OmgImAlexis I now decided to completely restructure the whole code (+ cleaning up/refactor), taking your idea of #604 into consideration. Currently the "new" code state is a mess ... but I will provide an intermediate state as soon as I have a clear new structure and the first few functions working again.

I will focus only on this refactoring for the new version now. The only thing I need to solve for the old version ist the bug that I possibly introduced when refactoring the graphics function to get proper UTF-8 output (#388)

[OmgImAlexis]

@sebhildebrandt got an update? 😃

[sebhildebrandt]

@OmgImAlexis I guess I now have a proper structure, did a lot of conversion already. But still a LOT to do. Tonight I will start testing the first batch (first 20 functions). I am pretty sure, that not all will work as expected ;-) As soon as at least all my partial converted functions are running I will create the v6 branch here. By Wednesday you should see a first partial version.

So as you mentioned to use this converted package in another project... what are the areas where you have a priority? To be honest: having a look on my long todo list for V6 (including refactoring, new functionality, testing, documentation, ...) I do not expect to have this new version published in the next few weeks. There are some areas, where I really need to make much more refactoring (not only TS conversion). E.g. network.js is one of the areas where I also need to change functionality.

[OmgImAlexis]

@sebhildebrandt I'm kind of surprised by that. I converted this whole library over in about a night. I have every function working as it did in the js version. 🤔

---

What I was initially hoping was to have a branch where we would work on this together, since the conversion didn't take me long I figured I'd redo it but this time in single commits so you could review.

---

I'm wanting to build out a stats monitoring package for all OSs and was going to use this for getting all the stats to send up to my cloud server. So.. "what are the areas where you have a priority" all of it? 😅

[sebhildebrandt]

@OmgImAlexis well, seems that I am much slower ;-) I was not aware that you did the whole thing on your side ...

I guess as I now completely restructured the code our versions will not match any longer ... really sorry about that. During the conversion I see a lot of things that needed to be changed and so I am not that sure if all works as expected on all platforms...

[OmgImAlexis]

I guess as I now completely restructured the code our versions will not match any longer

That's fine, it was more of a training exercise than anything. I do conversions of libraries quite frequently now so it's become quite quick for me to do so. Part of the reason I wanted to help with this so badly.

What's your suggestion for moving forward?

[sebhildebrandt]

@OmgImAlexis ... hmmm ... what if you then have a look at my new structure as soon as I have it published ... I definitely see that you are really experienced and I appreciate all your comments even if my suggested structure again needs refactoring. Maybe you also what to share your approach somewhere? What do you think?

The thing is ... I just don't want to take up too much of other people's precious resources. And that's why I really appreciate your commitment!

[OmgImAlexis]

hmmm ... what if you then have a look at my new structure as soon as I have it published

Do you think you could push a temp/test branch?

Maybe you also what to share your approach somewhere? What do you think?

Sure. I'd have to finish tidying it up. The main reason I hadn't opened a PR with it was it was rushed. I did it all at once and didn't commit any of it in smaller chunks. For the most part it's just the current code with types added and all the callbacks removed.

[sebhildebrandt]

@OmgImAlexis ... as soon as I also cleaned it up I will push it. And please do not out to much work into it ... I guess I will still need your resources anyway, once you have seen my proposal ;-)

[sebhildebrandt]

@OmgImAlexis I just pushed the current status to branch v6 ... have a look also at README.md to see what is missing.

[OmgImAlexis]

@sebhildebrandt looks good so far, can I make a few suggestions?

[OmgImAlexis]

Is there a reason the following.

export const audio = () => {
  return new Promise<AudioObject[] | null>(resolve => {
    process.nextTick(() => {
      switch (true) {
        case LINUX || FREEBSD || NETBSD:
          return resolve(nixAudio());
        case DARWIN:
          return resolve(darwinAudio());
        case WINDOWS:
          return resolve(windowsAudio());
        default:
          return resolve(null);
      }
    });
  });
};

Isn't written like this?

export const audio = async () => {
  await nextTick();
  switch (true) {
    case LINUX || FREEBSD || NETBSD:
      return nixAudio();
    case DARWIN:
      return darwinAudio();
    case WINDOWS:
      return windowsAudio();
    default:
      return null;
  }
};

With nextTick being the following function.

export const nextTick = () => new Promise<void>(resolve => {
  process.nextTick(() => {
    resolve();
  });
});

[sebhildebrandt]

@OmgImAlexis perfect ... will add this to all functions!

[OmgImAlexis]

I'd suggest unless you NEED to use new Promise avoid it and use async.

[OmgImAlexis]

Prefer filter/map over forEach.

For example the following

export const countLines = (lines: string[], startingWith = '') => {
  startingWith = startingWith || '';
  const uniqueLines = [];
  lines.forEach(line => {
    if (line.startsWith(startingWith)) {
      uniqueLines.push(line);
    }
  });
  return uniqueLines.length;
};

Can be written as.

export const countLines = (lines: string[], startingWith = '') => lines.filter(line => line.startsWith(startingWith)).length;

[OmgImAlexis]

Use Promise.allSettled instead of the promiseAll function from common/index.ts.

[OmgImAlexis]

Return early instead of using side effects.

export const toInt = (value: any) => {
  let result = parseInt(value, 10);
  if (isNaN(result)) {
    result = 0;
  }
  return result;
};

vs

export const toInt = (value: any) => {
  const result = parseInt(value, 10);
  if (isNaN(result)) return 0;
  return result;
};

[OmgImAlexis]

@sebhildebrandt can I make a PR to v6? There's quite a bit I would change honestly.

[OmgImAlexis]

Also incase you didn't know isPrototypePolluted can be bypassed.

[OmgImAlexis]

Why is noop(); used so much? I don't see any reason to be using it in the catch statements.

[OmgImAlexis]

Can I suggest using tsup instead of tsc?

[OmgImAlexis]

tslint should be removed and replaced with eslint.

[OmgImAlexis]

existsSync should never be used, instead the promise version of access should be imported from fs/promises.

For example.

import { existsSync } from "fs";
export const darwinXcodeExists = () => {
  const cmdLineToolsExists = existsSync('/Library/Developer/CommandLineTools/usr/bin/');
  const xcodeAppExists = existsSync('/Applications/Xcode.app/Contents/Developer/Tools');
  const xcodeExists = existsSync('/Library/Developer/Xcode/');
  return (cmdLineToolsExists || xcodeExists || xcodeAppExists);
};

vs

import { constants } from 'fs';
import { access } from 'fs/promises';

export const darwinXcodeExists = async () => {
  const results = await Promise.allSettled([
    access('/Library/Developer/CommandLineTools/usr/bin/', constants.F_OK),
    access('/Applications/Xcode.app/Contents/Developer/Tools', constants.F_OK),
    access('/Library/Developer/Xcode/', constants.F_OK)
  ]);

  // If at least one path fulfilled the promise xcode is installed
  return results.find(result => result.status === 'fulfilled') !== undefined;
};

[sebhildebrandt]

@OmgImAlexis nexTtick refactoring completed. Going through all others one by one ;-)

[OmgImAlexis]

@sebhildebrandt the Promise.allSettled isn't a straight conversion.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/allSettled

This is likely what you need. With Promise.all if one fails then it fails the whole Promise but with Promise.allSettled it returns an object for each with a status and a value.

const results = await Promise.allSettled([promise1, promise2]).then(results => results.map(result => result.status === 'fulfilled' ? result.value : null));

[OmgImAlexis]

Would it be okay to use kebab-case for filenames instead of camelCase as it makes things easier to read and avoids issues with case in/sensative file systems (macOS at times).

[OmgImAlexis]

stdout should already be a string when you get it from execCmd so I think the toString() call can be removed from lines like this let lines = stdout.toString().split('\n');

[OmgImAlexis]

I feel adding the nextTick in the windowsAudio function and just making that the export makes more sense than the current code. Unless there's a specific reason for having another function call windowsAudio?

Also if you export this as audio and only have a single export in index.ts you can use export * from './audio'; and then we can import the whole of windows as import * from './windows'; this would result in await windows.audio() working or import {audio} from './windows'; and then await audio();

export const windowsAudio = async () => {
  const stdout = await execCmd('path Win32_SoundDevice get /value');
  const parts = stdout.toString().split(/\n\s*\n/);
  const result = [];
  for (let i = 0; i < parts.length; i++) {
    if (getValue(parts[i].split(os.EOL), 'name', '=')) {
      result.push(parseAudio(parts[i].split('\n')));
    }
  }
  return result;
};

export const audio = () => {
  return new Promise<AudioObject[] | null>(resolve => {
    process.nextTick(() => {
      return resolve(windowsAudio());
    });
  });
};

[OmgImAlexis]

execCmd should export { stdout, stderr } so you'd use this const { stdout } = await execCmd('path Win32_SoundDevice get /value'); this would allow checking the stderr output if needed. This may result in us being able to show a better error for the user in the corresponding output.

[OmgImAlexis]

When cloning defaults the var shouldn't just be set to it as this can result in the original var being mutated.

const initValue = { randomArray: [0] };
const result = initValue;
result.randomArray.push(1);
console.log(initValue); // { randomArray: [ 0, 1 ] }

const initValue = { randomArray: [0] };
const result = JSON.parse(JSON.stringify(initValue));
result.randomArray.push(1);
console.log(initValue); // { randomArray: [ 0 ] }

[OmgImAlexis]

The following.

const result = {
    valueA: '',
    valueB: '',
};

should be used instead of setting each field individually. This makes things easier in Typescript.

[OmgImAlexis]

@sebhildebrandt sorry for all the feedback.

[OmgImAlexis]

You can see why I was hoping to do more of this myself.

[sebhildebrandt]

@OmgImAlexis THANK you for all your comments. Love to see that this improves the code base dramatically!!

Need to go through it ... as I am still refactoring some things. It would be fine if you wait with pull requests till I am through ...

And once again, sorry for having done it different ... really didn't know, that you went through a lot of thins already.

[OmgImAlexis]

Another thing is I think we should split the parsing from the retrieving. This would allow us to mock things quite easily by just passing in mocked stdout.

This would be replaced

export const bsdCpuCache = async () => {
  const result = initCpuCacheResult;
  const stdout = (await execCmd('export LC_ALL=C; dmidecode -t 7 2>/dev/null; unset LC_ALL')).toString();
  let cache: string[] = [];
  cache = stdout.split('Cache Information');
  cache.shift();
  for (let i = 0; i < cache.length; i++) {
    const lines = cache[i].split('\n');
    const cacheTypeParts = getValue(lines, 'Socket Designation').toLowerCase().replace(' ', '-').split('-');
    const cacheType = cacheTypeParts.length ? cacheTypeParts[0] : '';
    const sizeParts = getValue(lines, 'Installed Size').split(' ');
    let size = parseInt(sizeParts[0], 10);
    const unit = sizeParts.length > 1 ? sizeParts[1] : 'kb';
    size = size * (unit === 'kb' ? 1024 : (unit === 'mb' ? 1024 * 1024 : (unit === 'gb' ? 1024 * 1024 * 1024 : 1)));
    if (cacheType) {
      switch (true) {
        case cacheType === 'l1':
          result.l1d = size / 2;
          result.l1i = size / 2;
        case cacheType === 'l2': result.l2 = size;
        case cacheType === 'l3': result.l3 = size;
      }
    }
  }
  return result;
};

with this.

const parseCpuCache = (stdout: string, defaults = initCpuCacheResult) => {
  const [,...cache] = stdout.split('Cache Information');
  let l1d: number;
  let l1i: number;
  let l2: number;
  let l3: number;
  for (let i = 0; i < cache.length; i++) {
    const lines = cache[i].split('\n');
    const cacheTypeParts = getValue(lines, 'Socket Designation').toLowerCase().replace(' ', '-').split('-');
    const cacheType = cacheTypeParts.length ? cacheTypeParts[0] : '';
    const sizeParts = getValue(lines, 'Installed Size').split(' ');
    let size = parseInt(sizeParts[0], 10);
    const unit = sizeParts.length > 1 ? sizeParts[1] : 'kb';
    size = size * (unit === 'kb' ? 1024 : (unit === 'mb' ? 1024 * 1024 : (unit === 'gb' ? 1024 * 1024 * 1024 : 1)));
    if (cacheType) {
      switch (true) {
        case cacheType === 'l1':
          l1d = size / 2;
          l1i = size / 2;
        case cacheType === 'l2': l2 = size;
        case cacheType === 'l3': l3 = size;
      }
    }
  }
  return {
    ...defaults,
    l1d,
    l1i,
    l2,
    l3
  }
}

const cpuCache = async () => {
  const defaults = JSON.parse(JSON.stringify(initCpuCacheResult));
  const stdout = await execCmd('export LC_ALL=C; dmidecode -t 7 2>/dev/null; unset LC_ALL');
  return parseCpuCache(stdout, defaults);
};

[OmgImAlexis]

As much as I said filter/map should be used over forEach if you still just need a loop then use for (const value of values) {} or another similar for loop. Avoid forEach.