0

I'm making a build script for my angular app in node. Please have a look at the snippet:

const fs = require('fs-extra');

const dev = process.argv[2] === 'dev';
const folder = process.argv[3];

if (folder && fs.existsSync(`./projects/${folder}`)) {
    
    const execSync = require('child_process').execSync;
    // ng build --prod --output-hashing=none OR ng build --source-map --output-hashing=none
    
    let command;
    if (dev) {
        command = 'ng build --source-map --output-hashing=none ' + folder;
    } else {
        command = 'ng build --prod --output-hashing=none ' + folder;
    }

    // execSync(command, {stdio:[0, 1, 2]});

    (async function build()
        {
            
            const files = [
            ];
            const { promisify } = require('util')

            const getFiles = async () => {
                try {
                    const readdir = promisify(fs.readdir);
                    await readdir(`./dist/${folder}`, {withFileTypes:true}, (err, elements) => {
                        //handling error
                        if (err) {
                            return console.error('Unable to scan directory: ' + err);
                        } else {
                            elements.forEach(async element => {
                                if( !element.isDirectory() && /.*-es2015.js$/.test(element.name) ) {
                                    files.push(`./dist/${folder}/${element.name}`);
                                    console.log(`Pushing file: ./dist/${folder}/${element.name}`);
                                }
                            });
                        }
                    });
                } catch (err) {
                    console.error(err);
                }
            }
            
            await getFiles();

            // We need a random number for voiding the cache with every new build
            const random = [...Array(10)].map(()=>(c = (r = Math.random()).toString(36)[2]) && r>.5 ? c.toUpperCase():c ).join('');

            // create directory if doesnt exists (not needed anymore): await fs.ensureDir(`../js/${folder}/dist`)
            if (!dev && files.length) {
                const concat = require('concat');
                await concat(files, `./dist/${folder}/concatenated.${random}.js`);
            }
            console.log('Build complete');
        }
    )();
    
} else if (folder && !fs.existsSync(`projects/${folder}`)) {
    console.log('Specified destination folder does not exists as a project');
}
else {
    console.log('Please specify a destination folder such as app-name');
}

Well, the mysterious is that just after await getFiles() call, the execution halts, no error neither message anywhere is shown. I'm getting crazy investigating this.

Can anybody spot the issue? Thanks

manu
  • 187
  • 1
  • 16
  • 1
    IF you promisify `readdir`, then you don’t also use a callback. You should use `asynchronous/await` or `then()/catch()` instead. – Randy Casburn Nov 26 '20 at 22:26
  • Thank @RandyCasburn for your answer. I tried before without promisify and with await / async. As a result the code after getFiles were executed before getFiles itself. Now after getFiles nothing else is executed, that's the weird thing. Maybe I did it the wrong way. Could you please post an example? Your help is much appreciated – manu Nov 26 '20 at 22:30
  • Indeed you should be using `const elements = await await readdir(`./dist/${folder}`, {withFileTypes:true});` - the callback is never called. Also you probably don't need to make a `getFiles` helper function - just call it directly. The actual problem appears to be the `elements.forEach(async element => {…})` though which won't work. – Bergi Nov 26 '20 at 22:39

1 Answers1

0

The main issue in your code is that you are not promisfying the readdir correctly.

Try this:

(async () => {
    try {
        const readdir = require('util').promisify(require('fs').readdir);
        const elements = await readdir(`./dist/${folder}`, { withFileTypes: true });
        await Promise.all(
            elements.map(async (element) => {
                if (!element.isDirectory() && /.*-es2015.js$/.test(element.name)) {
                    files.push(`./dist/${folder}/${element.name}`);
                    console.log(`Pushing file: ./dist/${folder}/${element.name}`);
                }
            })
        );
    } catch (error) {
        console.error('Unable to scan directory: ' + err);
    }
})();

You can of course keep your forEach while omitting the async instead of the map + async + Promise.all. The difference is is that the one I suggest is faster since it utilizes concurrency while forEach would work sequentially! But either one would work!

basickarl
  • 37,187
  • 64
  • 214
  • 335
  • @jfriend00 forEach is if he omits the async, you are correct however if the async stays it's not sequential. Sorry if I was unclear about that. Updated the answer. – basickarl Nov 26 '20 at 22:55
  • "*the one I suggest is faster since it utilizes concurrency*" - it doesn't if the callback isn't doing anything asynchronous. Your `map` is sequential as well. – Bergi Nov 27 '20 at 09:45
  • @Bergi Are you meaning that `element.isDirectory()` are all blocking each other? If so then you would be correct (I haven't checked). – basickarl Nov 27 '20 at 10:31
  • Yes, it's a synchronous method. – Bergi Nov 27 '20 at 10:45
  • @Bergi I understand that but if you have several of those running in their own processes will they block each other? – basickarl Nov 27 '20 at 10:54
  • Not certain which meaning of "process" you refer to here, but there are no processes in the code. – Bergi Nov 27 '20 at 10:57
  • @Bergi I used the term "process" in regards to traditional concurrency (in node as we know everything runs on a single event loop). When forEach is called with async, it fires off every callback nearly at the same time as shown here https://jsfiddle.net/wms1g0vc/22/ run that and you will see that forEach does not wait until the previous callback is resolved, hence it is a concurrent operation happening, it fires them off synchronously (one after the other), but it does not wait until as said the previous fired callback to finish. – basickarl Nov 28 '20 at 15:25
  • @Bergi https://jsfiddle.net/wms1g0vc/42/ in that example you see a non async callback being used and that does wait for the previous callback to finish. – basickarl Nov 28 '20 at 16:06
  • Yes, `forEach` fires them off synchronously and ignores their results. If the callback does something asynchronous, like the `setTimeout` in your fiddle, these tasks will run concurrently. But in the code in your answer, there is nothing asynchronous, nothing is being `await`ed, and all the code in the `map` runs sequentially exactly like in your second fiddle. – Bergi Nov 28 '20 at 17:27