easy-pdf-merge icon indicating copy to clipboard operation
easy-pdf-merge copied to clipboard

Support for Promises

Open shirshak55 opened this issue 6 years ago • 9 comments

I think promises are better now isn't it?

shirshak55 avatar May 04 '19 18:05 shirshak55

Yes, we would love to get PRs for promise support! cc @g0ddish

karuppiah7890 avatar May 05 '19 14:05 karuppiah7890

Here's my first crack at a promised based PDFMerge...needs to be thoroughly tested!

PDFMerge-Promises.txt

ksumarine avatar Jun 12 '19 21:06 ksumarine

Hmm I think its fine for now though its just an improvement.

shirshak55 avatar Jun 13 '19 04:06 shirshak55

@ksumarine Thanks for spending time on adding promise support. I just checked it out. Could you raise a PR with this code? I just have one comment for now, I see that there's only Promise support in the code that you posted, this will break code for people already using the library.

To tackle this, we can either release a major version change. Or we could support both callbacks and promises.

I would like to choose the latter. I remember seeing some libraries support this by doing - if callback func is passed, then use that, if not return promises.

Also, one more thing I noticed is you have used the default opts value in the function parameter definition, that's great. Just one thing, the author who wrote the code for opts has also considered the case where people call the function with only source files array, destination and callback (as 3rd arg) with no opts, the 3rd arg was considered as callback, and then used default opts. Could you consider these cases by looking at the current code? 😄 I would be happy to help if you have any questions regarding this

karuppiah7890 avatar Jun 27 '19 15:06 karuppiah7890

@karuppiah7890 you got it! Sorry for the delay, I'm at work. I can try to work on this tonight and attempt a PR (this would be my first for someone else's project).

You raise good concerns that I hadn't considered when I tried the promises. I will do my best to account for those situations. Please let me know if I didn't get it right or please send suggestions if you have any.

I appreciate your help!

ksumarine avatar Jun 27 '19 21:06 ksumarine

Hello bro,

If you don't mind I used hummus pdf as it required something like apache pdf box and turns out it also worked really well without using any dependencies.

Can u check that too may be it will be more useful too you ?

Here is how i merged using hummus js

import fs from 'fs'
import path from 'path'
import HummusRecipe from 'hummus-recipe'
import { consoleMessage as m } from 'scrapper-tools'

const fsPromises = fs.promises
async function merger(inputFolder = pdfDir, outputFile = outputFilePath) {
    const files = fs.readdirSync(inputFolder)
    let temp = []

    files.forEach((file) => {
        temp.push(file)
    })

    temp.sort((a, b) => {
        let anum = parseInt(a.split('.pdf')[0])
        let bnum = parseInt(b.split('.pdf')[0])

        return anum - bnum
    })

    let f = []
    temp.forEach((file) => {
        f.push(path.join(inputFolder, file))
    })
    console.log(f)
    const outputToMerge = path.join(outputFile + '_copy.pdf')
    const outputToMergePath = path.join(__dirname, '/../front.pdf')
    m.success('PDF Merge', 'Copying Cover File Temporarily')
    await fsPromises.copyFile(outputToMergePath, outputToMerge)

    m.success('PDF Merge', 'Starting To Merge')
    const pdfDoc = new HummusRecipe(outputToMerge, outputFile)
    f.forEach((v, i) => {
        m.info('Merge', 'File Index', i)
        pdfDoc.appendPage(v)
    })
    pdfDoc.endPDF()
}

shirshak55 avatar Jun 28 '19 10:06 shirshak55

Please don't mind extra code as i wrote it for my own purpose :)

shirshak55 avatar Jun 28 '19 10:06 shirshak55

@shirshak55 thanks for the info. I will check out the package 😄

karuppiah7890 avatar Jun 29 '19 15:06 karuppiah7890

Seems a common strategy for this is to return a promise if no callback function is supplied.

Example (stream-to-array):

If callback is not defined, then it returns a promise. (docs | source code)

It's slightly obfuscated in the code but it still serves as an example.

carmanchris31 avatar Aug 30 '19 23:08 carmanchris31