DELETE FILES with Node.js

I'm trying to delete some files and then show a message.

EXPECTED OUTPUT

File deleted

Folder Cleared!!!

ACTUAL OUTPUT

Folder Cleared!!!

File deleted

The current code is:

function clearConverted() {
      const resp = new Promise(async (resolve) => {

        const converted = glob.sync('./converted/*.mp4');


        if (converted.length) {
          const fs = require('fs');
          const promises = converted.map(v => {

            fs.unlink(v, () => console.log('File deleted'))
          })
          Promise.all(promises);
        } else {
          console.log('No files to delete');
        }
        resolve();
      })
      resp.then(console.log('Folder Cleared!!!'))
    }

May you help me?

Upvotes: 2

Views: 2382

Answers (2)

Mulan
Mulan

Reputation: 135217

Per my original comments on your question, you have heaps to fix with your code -

const { unlink } =
  require("fs").promises  // <-- fsPromises

function clearConverted() {
  const converted =
    glob.sync("./converted/*.mp4")

  if (converted.length === 0)
    return Promise.resolve([])

  const promises = 
    converted.map(v => unlink(v).then(_ => v))

  return Promise.all(promises)
}

clearConverted() // <-- returns a promise!
  .then(deletedFiles => console.log("done! deleted files:", deletedFiles))

// done! deleted files: ./converted/foo.mp4, ./converted/bar.mp4

And see how we removed console.log side effects from the function? This allows our function to collect meaningful data, like the file names, and return a list of deleted files. Because the console.log effect is now outside of clearConverted we can change it, if we wish.

For example, we could simply display the count of deleted files, in a less verbose program -

clearConverted()
  .then(deletedFiles =>
    console.log("done! deleted %d files.", deletedFiles.length)
  )

// done! deleted 9 files.

And we can do more. An obvious improvement now is to make clearConverted a generic function which accepts the path as an argument -

function unlinkFiles (path = "") { // <-- generic name and path parameter
  const files =
    glob.sync(path)

  if (files.length === 0)
    return Promise.resolve([])
  else
    return Promise.all(files.map(f => unlink(f).then(_ => f)))
}

unlinkFiles("./converted/*.mp4") // <-- supply path at call site
  .then(deletedFiles => console.log("done! deleted files:", deletedFiles))
  .catch(console.error) // <-- don't forget to catch Promises too

Modern features like async allow us to skip some of the ceremony around Promises -

async function unlinkFiles (path = "") { // <-- async keyword
  const files =
    glob.sync(path)

  return files.length === 0
     ? []  // <-- automatically wrapped in a Promise
     : Promise.all(files.map(f => unlink(f).then(_ => f)))
}

Now if you wanted, you could make the function accept a glob result instead of a path -

const unlinkFiles = async (files = []) => // <-- arrow function
  files.length === 0
    ? []
    : Promise.all(files.map(f => unlink(f).then(_ => f)))

unlinkFiles(glob.sync("./converted/*.mp4")) // <-- use glob as input
  .then(console.log, console.error)

When you detangle the wires, programming can be fun and easy. Sadly, languages like JavaScript also make it easy to shoot yourself in the foot, so there's a lot of misery before enlightenment.


I have other answers involving the fs module and Promises. These additional code examples may help provide additional insight -

Upvotes: 6

Arlind
Arlind

Reputation: 397

When you call Promise.all add .then method there, something likethis:

Promise.all(promises).then((response) => console.log("Folder Cleared")).catch((error) => console.log(error)) 

And also when you require modules or dependencies, you declare them at the top of the file and not inside functions or loops.

Upvotes: 0

Related Questions