Skip to content

Conversation

malthoff
Copy link
Contributor

The types of the jsPDF.save method only mention jsPDF itself, but if returnPromise options is set to true the function returns a promise.

Thanks for contributing to jsPDF! Please follow our
Contribution Guidelines
when creating a pull request.

The types of the jsPDF.save method only mention jsPDF itself, but if returnPromise options is set to true the function returns a promise.
@Uzlopak
Copy link
Collaborator

Uzlopak commented Sep 27, 2021

Is it a Promise?

@malthoff
Copy link
Contributor Author

If you provide { returnPromise: true } as options in save, it returns a promise. But the types are not reflecting this change. It was introduced due to this discussion:
#540

@Uzlopak
Copy link
Collaborator

Uzlopak commented Sep 28, 2021

lol. Markup changed my question. My original question was:
Is it a Promise<void>?

So do you have to add <void> so that the typings don't throw in modern typescript?

@Uzlopak
Copy link
Collaborator

Uzlopak commented Sep 28, 2021

Screenshot_20210928-085655_Chrome

Also i think you can make it more typestrong if you do it like this:

save(filename: string, options: { returnPromise: true }): Promise<void>;
save(filename?: string): jsPDF;

@Uzlopak
Copy link
Collaborator

Uzlopak commented Sep 28, 2021

modified accordingly.

@malthoff
Copy link
Contributor Author

Not sure, as the result of saveAs is returned.

Code:

return new Promise(function(resolve, reject) {
        try {
          var result = saveAs(getBlob(buildDocument()), filename);
          if (typeof saveAs.unload === "function") {
            if (globalObject.setTimeout) {
              setTimeout(saveAs.unload, 911);
            }
          }
          resolve(result);
        } catch (e) {
          reject(e.message);
        }
      });

@Uzlopak
Copy link
Collaborator

Uzlopak commented Sep 28, 2021

Yeah but result in FileSaver.js is not return anything i think

@malthoff
Copy link
Contributor Author

ok, perfect. Thanks for adjusting

@Uzlopak
Copy link
Collaborator

Uzlopak commented Sep 28, 2021

@HackbrettXXX
I guess as maintainer, you want to decide regarding merging this PR.

Copy link
Collaborator

@HackbrettXXX HackbrettXXX left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I'll merge it.

@HackbrettXXX HackbrettXXX merged commit 99cbee4 into parallax:master Oct 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants