2
\$\begingroup\$

I'm using the following code to unzip file which is working OK. I'm getting file with the request (express) and unzip it in specified folder

_unzip: function (req) {
 return new Promise((resolve, reject) =>{
 var noOfFiles = 0;
 let filePath = path.join(req.file.destination, req.file.filename);
 logger.info("Unzip filePath: " + filePath);
 if (req.file) {
 yauzl.open(filePath, function (err, zipFile) {
 if (err) throw err;
 zipFile.on('entry', (entry) =>{
 //console.log(entry.fileName);
 if (/\/$/.test(entry.fileName)) {
 return;
 }
 zipFile.openReadStream(entry, (err, readStream) => {
 if (err) {
 logger.info(err);
 reject(err);
 } else {
 // ensure parent directory exists, and then:
 let destinationFileName = "./" + entry.fileName;
 let directoryName = path.dirname(destinationFileName);
 if (!fs.existsSync(directoryName)) {
 mkdirp(directoryName, (err) =>{
 if (err) {
 logger.info(err);
 reject(err);
 } else {
 writeIntoFile(readStream, destinationFileName, reject);
 noOfFiles++;
 }
 });
 } else {
 writeIntoFile(readStream, destinationFileName, reject);
 noOfFiles++;
 }
 }
 });
 }).once('error', (err) =>{
 logger.info(err);
 reject(err);
 }).once('close', () =>{
 logger.info("Unpacked " + noOfFiles + " files");
 resolve();
 });
 });
 }
 });
}

https://github.com/thejoshwolfe/yauzl

200_success
146k22 gold badges190 silver badges479 bronze badges
asked Nov 7, 2016 at 15:54
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$
  • Check for req.file before using its properties
  • Return/reject early instead of nesting a big else clause
  • Since you're using ES2015 syntax do it consistently:
    • template strings via backticks
    • arrow functions in callbacks that don't utilize this of the callee
    • string methods with self-explanatory names like endsWith instead of regexps
    • let instead of var (there was one in your code)
  • Deduplicate reject+logger combo and writeIntoFile
  • Use single or double quotes consistently
return new Promise((resolve, reject) => {
 if (!req || !req.file) {
 __rejectAndLog('No file provided');
 return;
 }
 let numberOfFiles = 0;
 let filePath = path.join(req.file.destination, req.file.filename);
 logger.info(`Unzip filePath: ${filePath}`);
 yauzl.open(filePath, (err, zipFile) => {
 if (err) {
 __rejectAndLog(err);
 return;
 }
 zipFile.on('entry', entry => __processEntry(zipFile, entry))
 .once('error', __rejectAndLog)
 .once('close', () => {
 logger.info(`Unpacked ${numberOfFiles} files`);
 resolve();
 });
 });
 function __processEntry(zipFile, entry) {
 if (entry.fileName.endsWith('/')) {
 return;
 }
 zipFile.openReadStream(entry, (err, readStream) => {
 if (err) {
 __rejectAndLog(err);
 return;
 }
 let destinationFileName = './' + entry.fileName;
 let directoryName = path.dirname(destinationFileName);
 if (fs.existsSync(directoryName)) {
 __writeFile();
 return;
 }
 mkdirp(directoryName, (err) => {
 if (err) {
 __rejectAndLog(err);
 } else {
 __writeFile();
 }
 });
 function __writeFile() {
 writeIntoFile(readStream, destinationFileName, reject);
 numberOfFiles++;
 }
 });
 }
 function __rejectAndLog(err) {
 logger.info(err);
 reject(err);
 }
});
  • entry-processing function is extracted to make the zipFile flow more obvious
  • __ prefix is used for subfunctions following your naming of _unzip
answered Nov 9, 2016 at 8:57
\$\endgroup\$
2
  • \$\begingroup\$ Thanks I've tried your proposal and I'm getting error TypeError: Cannot read property 'openReadStream' of undefined this is since it doesn't have the zipFile entry , any idea? \$\endgroup\$ Commented Nov 13, 2016 at 9:52
  • \$\begingroup\$ Well, obviously I overlooked zipFile variable scope. I've fixed the code, but you really should be able to see what is the issue, moreover my answer was an example not intended to be used as-is because I didn't test it. \$\endgroup\$ Commented Nov 13, 2016 at 13:56

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.