Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Add Promise as return type to save method #3272

Merged
merged 3 commits into from Oct 6, 2021
Merged

Fix: Add Promise as return type to save method #3272

merged 3 commits into from Oct 6, 2021

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.

None yet

3 participants