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

removeAfterPrint doesn't wait for promise returned from print callback #616

Closed
ms-tng opened this issue Jun 6, 2023 · 3 comments · Fixed by #619
Closed

removeAfterPrint doesn't wait for promise returned from print callback #616

ms-tng opened this issue Jun 6, 2023 · 3 comments · Fixed by #619
Labels

Comments

@ms-tng
Copy link

ms-tng commented Jun 6, 2023

When removeAfterPrint is set to true and a custom print callback is provided, the iframe is already removed before the promise returned from the print callback resolves.

Minimal reproducing example:

const TestComponent = () => {
  const printRef = useRef();
  const print = useReactToPrint({
    content: () => printRef.current,
    print: (iframe) => new Promise(resolve => {
      setTimeout(() => {
        iframe.contentWindow.print();
        resolve();
      }, 0);
    }),
    removeAfterPrint: true,
  });

  return <div>
    <button onClick={print}>Print</button>
    <div ref={printRef}>Test</div>
  </div>;
};

This produces the error

Uncaught TypeError: Cannot read properties of null (reading 'print')

because iframe.contentWindow === null when iframe.contentWindow.print() is invoked.

As a workaround, one can set removeAfterPrint: false and remove the iframe manually in the print callback.

The reason for this behavior is that while here this.handleRemoveIframe() is only called when the promise resolves, here it is still called synchronously.

The same problem occurs with the onAfterPrint callback, which is also called without waiting for the promise to resolve.

@MatthewHerbst
Copy link
Owner

MatthewHerbst commented Jun 6, 2023

Hey, thanks for the detailed report, it's a good catch! I think the fix is straightforward: lines 146-150 need to moved to be within the different blocks, might make a helper for them. An await on print( would work too but I've been trying to avoid async/await to maintain old browser compatibility. The next major version of the package, which I expect to get out sometime around new years, will drop support for IE and for React 15.

I'll get a fix for this specific issue out later tonight!

@MatthewHerbst
Copy link
Owner

Fix should be available in v2.14.13, please let me know if you run into any issue with it.

Note that I also added in calling of onAfterPrint when the print promise resolves to match the behavior of the code when not using a custom print method, even though the author of a custom print method is fully capable of determining when printing has completed themselves since they control the promise resolve.

I also did some general package cleanup 🥳

@ms-tng
Copy link
Author

ms-tng commented Jun 7, 2023

Thank you for the quick fix! I just tested it, works perfectly. 👍

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

Successfully merging a pull request may close this issue.

2 participants