DevelopingMagic / pdfassembler

MIT License
183 stars 34 forks source link

pdfObject is not a function #2

Open lukas-mertens opened 6 years ago

lukas-mertens commented 6 years ago

Hi! Thank you for developing pdfassembler, it's exactly what I need. I've got a problem with your minimal working example from the Readme.md. I use Browserify and Babel to support all browsers and to use

var PDFAssembler = require('pdfassembler').PDFAssembler;

on the client side.

This is my code:

    await Promise.all(pdfFiles.map(async (f) => { 
        const helloWorldPdf = {
            '/Root': {
              '/Type': '/Catalog',
              '/Pages': {
                '/Type': '/Pages',
                '/Count': 1,
                '/Kids': [ {
                  '/Type': '/Page',
                  '/MediaBox': [ 0, 0, 612, 792 ],
                  '/Contents': [ {
                    'stream': '1 0 0 1 72 708 cm BT /Helv 12 Tf (Hello world!) Tj ET'
                  } ],
                  '/Resources': {
                    '/Font': {
                      '/Helv': {
                        '/Type': '/Font',
                        '/BaseFont': '/Helvetica',
                        '/Subtype': '/Type1'
                      }
                    }
                  },
                } ],
              }
            }
          }
        const pdfAssemblerObj = new PDFAssembler(helloWorldPdf);
        pdfAssemblerObj
        .pdfObject()
        .then(function(pdf) {
            pdf['/Root']['/Pages']['/Kids'] = pdf['/Root']['/Pages']['/Kids'].slice(0, 1);
        });
        await pdfAssemblerObj.removeRootEntries();
        out_pdfs.push(await pdfAssemblerObj.assemblePdf(f.filename));
    }));

Whenever I try to execute it, it throws the error

Unhandled promise rejection
TypeError: pdfAssemblerObj.pdfObject is not a function

in firefox (just the beginning of the error) and

Uncaught (in promise) TypeError: pdfAssemblerObj.pdfObject is not a function
    at _callee16$ (bundle.js:55304)
    at tryCatch (polyfill.min.js:4)
    at Generator.invoke [as _invoke] (polyfill.min.js:4)
    at Generator.t.(:3000/anonymous function) [as next] (https://cdnjs.cloudflare.com/ajax/libs/babel-polyfill/6.26.0/polyfill.min.js:4:1671)
    at step (bundle.js:55383)
    at bundle.js:55383
    at new Promise (<anonymous>)
    at bundle.js:55383
    at bundle.js:55335
    at Array.map (<anonymous>)

in chrome. When I print out the pdfAssemblerObj using

const pdfAssemblerObj = new PDFAssembler(helloWorldPdf);
console.log(pdfAssemblerObj);

it gives me the object

{
  "pdfManager": null,
  "userPassword": "",
  "ownerPassword": "",
  "nextNodeNum": 1,
  "pdfTree": {
    "/Root": {
      "/Type": "/Catalog",
      "/Pages": {
        "/Type": "/Pages",
        "/Count": 1,
        "/Kids": [
          {
            "/Type": "/Page",
            "/MediaBox": [
              0,
              0,
              612,
              792
            ],
            "/Contents": [
              {
                "stream": "1 0 0 1 72 708 cm BT /Helv 12 Tf (Hello world!) Tj ET"
              }
            ],
            "/Resources": {
              "/Font": {
                "/Helv": {
                  "/Type": "/Font",
                  "/BaseFont": "/Helvetica",
                  "/Subtype": "/Type1"
                }
              }
            }
          }
        ]
      }
    }
  },
  "recoveryMode": false,
  "objCache": {},
  "objCacheQueue": {},
  "pdfManagerArrays": [],
  "pdfAssemblerArrays": [],
  "promiseQueue": {
    "options": {},
    "pendingPromises": 0,
    "maxPendingPromises": 1,
    "maxQueuedPromises": null,
    "queue": []
  },
  "indent": false,
  "compress": true,
  "encrypt": false,
  "groupPages": true,
  "pageGroupSize": 16,
  "pdfVersion": "1.7"
}

Did you change anything about the API? Or what did I do wrong?

Edit: I don't use TypeScript. Could that be a problem?

evanc commented 6 years ago

I ran into this as well, I just haven’t had a chance to make a pr- pdfObject is not a function (despite the sample code implying that it is), it’s a property that runs a getter method and returns a promise. Just do:

pdfAssemblerObj.pdfObject.then(...)

Several of the things you would expect to be methods are implemented as properties! Look in the source; if a “method” is declared prefixed with get and a space it’s actually a property.

Explanation of js getters, since they’re not very commonly used: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/get

lukas-mertens commented 6 years ago

Thank you, you saved my day! I made a pull request (#1), that adds an explanation to the Readme. That was the last thing I had to fix, I am now ready for production/open source 🎉

I won't close the issue for now, because I think it would be a good idea to switch to methods. What do you think?

dschnelldavis commented 6 years ago

I merged your pull request to fix the readme, @lukas-mertens - thanks!

lukas-mertens commented 6 years ago

No problem!

dschnelldavis commented 6 years ago

I'm reopening this because I think the comment about "Several of the things you would expect to be methods are implemented as properties!" is a valid criticism I'd like to address.

The library currently has three properties:

Personally, I don't have a preference about whether these are methods or properties. I think it does make sense to make them methods, primarily for consistency. On the other hand, changing them would break backwards compatibility for people already using the library and expecting them to be properties.

Perhaps a good transitional solution would be to replace them with a new functions with different names, like so:

I would update the documentation to inform people to use the functions, but also leave the old properties in place for a while so as to maintain backward compatibility for a few more versions.

Thoughts? Alternate suggestions?

lukas-mertens commented 6 years ago

Is for example the number of pages calculated when accessing the property numPages or is it calculated by some other function before that? Because a property implies that it's calculated before accessing the property, a function would imply that it's being calculated when calling the function. If countPages() would just return some pre-calculated value, I would just leave it as it is. But the documentation should be updated in that case:

Renaming the methods wouldn't be a good idea I think... I would do it like this:

<button>
Click me
</button>
<div>
Current Time (1): <span id="1"></span><br>
Current Time (2): <span id="2"></span>
</div>
var exampleObject = {
    getTime: function() {
    return new Date().toLocaleString();
  }
}

$("button").click(function() {
    $("#1").html(exampleObject.getTime);
    $("#2").html(exampleObject.getTime());
});

Try it here