Improve the API unit-tests, and try to expose more API-functionality in the TypeScript definitions #14013
Conversation
…PDFDocumentLoadingTask`-instance This is similar to existing unit-tests, which checks for `PDFDocumentProxy`- and `PDFPageProxy`-instances.
…urns a `RenderTask`-instance This is similar to existing unit-tests, which checks for `PDFDocumentProxy`- and `PDFPageProxy`-instances.
While these types apparently makes sense in TypeScript environments, we really don't want to extend the *public* API by simply exporting the relevant classes directly in `src/pdf.js` (since they should never be called/initialized manually). Please see e.g. issue 12384 where this was first requested, and note that a possible work-around was also provided there. This patch simply implements that work-around[1], which will hopefully be helpful to TypeScript users. --- [1] Based on the discussion in PR 13957, the two previous patches appear to be necessary for this to actually work.
|
/botio unittest |
From: Bot.io (Linux m4)ReceivedCommand cmd_unittest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.241.84.105:8877/6a164cdc3eccb9d/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_unittest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.193.163.58:8877/8d66c9df0a29e47/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.241.84.105:8877/6a164cdc3eccb9d/output.txt Total script time: 2.84 mins
|
From: Bot.io (Windows)SuccessFull output at http://54.193.163.58:8877/8d66c9df0a29e47/output.txt Total script time: 6.64 mins
|
|
Given #13957 (comment), does the last commit make sense to include at this point in time? |
I really don't know, since we also have #12384 (comment) which explicitly recommended this approach. |
|
@tamuratak Would you perhaps be able to shed some light on if the third commit in this PR, Snuffleupagus@95057a4 to be precise, is working correctly given your suggestion from #12384 (comment) one the one hand but #13957 (comment) on the other hand that claims it doesn't have the desired effect? |
|
The following line does not work as expected. Line 21 in 95057a4 That is because Line 3140 in 95435ed For #13957, if we remove the const task: RenderTask = pageProxy.render(renderContext);const task = pageProxy.render(renderContext);
|
Huh, but it actually is thanks to the previous commit; please see https://github.com/mozilla/pdf.js/pull/14013/files#diff-082d6b37ad01db7ac97cc07c6ddb0dc52040484c5ef91b110b072f50144d9f39 Furthermore, we now have unit-tests that ensure that |
I am sorry I have missed that. I have confirmed that things work as expected. |
No worries; thank you for helping out with testing this! |
|
Let's do this now that we know it's working; thanks! |
|
Thank you for looking into this! |
Please refer to the individual commit messages for additional details.