1

at first i need to say im pretty new to electron and node.js.

What i am doing is im trying to send an array with some directory data to my browser and this asynchronous (Error: array is empty).

The Problem should be my function 'scanDirectories(path)'. If i make it non recursive (scanDirectories(res) -> only return res) it works pretty fine for 1 level directories, but recursive it doesnt.

So i think i need to do it like my function above with returning a new promise? I tried it but can´t figure out how it works or my syntax should be.

Hope you can help me.
Simon

main.js:
calling function from ipcmain

ipcMain.on('fileTree', (event, arg) => {
  let fileDirectory = helperFile.getDirectories(arg);
  fileDirectory.then(function(result) {
    event.reply('fileTree', result);
  }, function(err) {
    console.log(err);
  })
})

files.js

const { readdir } = require('fs').promises;
const resolvePath = require('path').resolve;

module.exports = {
    getDirectories: async function(path) {
       return new Promise(function (resolve, reject) {
           try {
               resolve(scanDirectories(path));
           } catch {
               reject('Error');
           }
       });
    }
};

async function scanDirectories(path) {
    const dirents = await readdir(path, {withFileTypes: true});
    const files = dirents.map((dirent) => {
        const res = resolvePath(path, dirent.name);
        return dirent.isDirectory() ? scanDirectories(res) : res;
    });
    return Array.prototype.concat(...files);
}
Simon
  • 31
  • 4
  • Looks like a good question, but can you improve the formatting? – dwjohnston Oct 13 '19 at 09:03
  • Avoid the [`Promise` constructor antipattern](https://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it)! – Bergi Oct 13 '19 at 13:52

2 Answers2

1

You can try something like this where you generate an array of promises:

files.js

const { readdir } = require('fs').promises;
const resolvePath = require('path').resolve;

module.exports = {
  // This is an async function so don’t need internal promise
  getDirectories: async function(path) {
    try {
      const dirs = await scanDirectories(path);
      return dirs;
    }
    catch {
      throw new Error('Error');
    }
  }
};

async function scanDirectories(path) {
  const dirents = await readdir(path, {withFileTypes: true});

  // Generate array of promises
  const promises = dirents.map(dirent => {
    const res = resolvePath(path, dirent.name);
    return dirent.isDirectory()
      ? scanDirectories(res)
      : Promise.resolve(res);
  });

  // Await all promises
  const files = await Promise.all(promises);
  return Array.prototype.concat(...files);
}
Steve Holgado
  • 11,508
  • 3
  • 24
  • 32
0

If you call an async function without await, you receive a promise.

Your event handler handles this sort-of-OK with then (it has trouble with error handling), but your recursive call to scanDirectories does not.

The simplest way to wait for an async function to resolve is to use await.

So this change makes the recursive call properly:

return dirent.isDirectory() ? (await scanDirectories(res)) : res;

Note the addition of await.

However "Array.map" is not designed for use in async functions. It will call them synchronously and create an array of promises. Not what you want.

In addition, this code is unnecessarily complicated, wrapping a promise in a promise and using try/catch in a way that won't work:

    getDirectories: async function(path) {
       return new Promise(function (resolve, reject) {
           try {
               resolve(scanDirectories(path));
           } catch {
               reject('Error');
           }
       });
    }

Just call scanDirectories directly from your original event handler, and make the event handler an async function, so a lot of code just goes away.

In general: if you have to deal with async stuff, write an async function, and always await it in the function that calls it, even if that function is itself. You may write async functions anywhere, even if they are event handlers or Express routes or other contexts where the promise they resolve to won't be consumed.

Here is your original code simplified and corrected but working basically the same way:

ipcMain.on('fileTree', async (event, arg) => {
  try {
    event.reply('fileTree', await helperFile.scanDirectories(arg);
  } catch (e) {
    console.log(e);
  }
});

// files.js

const { readdir } = require('fs').promises;
const resolvePath = require('path').resolve;

module.exports = {
  scanDirectories: async function(path) {
    const dirents = await readdir(path, { withFileTypes: true });
    const files = [];
    for (const dirent of dirents) {
      const res = resolvePath(path, dirent.name);
      if (dirent.isDirectory()) {
        files = files.concat(await scanDirectories(res));
      } else {
         files.push(res);
      }
    });
    return files;
  }
}
Tom Boutell
  • 7,281
  • 1
  • 26
  • 23
  • 1
    This doesn't work, the `await scanDirectories` is not inside an `async function` - it's inside the `map` callback – Bergi Oct 13 '19 at 13:54
  • 1
    Oops yes. Map expects an ordinary sync function. I will correct my answer. "for...of" is safe to use with async/await so I generally recommend a solution based on it. – Tom Boutell Oct 13 '19 at 15:24
  • 1
    You can use promise-returning functions (including `async` ones) with `map` as well, you just get back an array of promises and would need to use `Promise.all`. Both a valid solutions. – Bergi Oct 13 '19 at 15:38
  • That's correct, but I wanted to present a solution that didn't require thinking quite as much about promises. Totally a stylistic difference though. – Tom Boutell Oct 13 '19 at 15:52