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

Is workbook.commit() still a promise or not #548

Closed
Sjonnie2nd opened this issue Apr 18, 2018 · 2 comments
Closed

Is workbook.commit() still a promise or not #548

Sjonnie2nd opened this issue Apr 18, 2018 · 2 comments

Comments

@Sjonnie2nd
Copy link

The docs mention:
// Finished the workbook.
workbook.commit().then(function() { /* the stream has been written */ });

I used it like:
book.commit().then(() => response.end())

After I upgraded types/exceljs to ^0.5.2 typescript warned me for commit() not giving back a promise.

So I changed it to:
book.commit();
response.end();
After this things went wrong because response.end() was breaking the stream too early.
After removing response.end() it worked fine. Perhaps commit() called response,end() itself.

So 3 questions:

  • Need the docs be adjusted: commit() no longer brings a response?
  • Or is types/exceljs wrong here?
  • Does commit() really end the response?
@carboneater
Copy link
Contributor

@Sjonnie2nd
exceljs seems to provide its own index.d.ts now, where workbook.commit() returns void rather than Promise, as documented and in @types/exceljs

Looking at the code, that looks like an issue with the new integrated index.d.ts, because the underlying function for workbook.commit(), or at least stream.xlsx.workbook-writer.js, still handles the job through promises and returns a resolved promise when the workbook.commit() is done...

@guyonroche
Copy link
Collaborator

@carboneater 's PR has been published as v1.4.12

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants