Allow renderActions to return promises and use that to better handle retries - #1540
Open
dpvc wants to merge 2 commits into
Open
Allow renderActions to return promises and use that to better handle retries#1540dpvc wants to merge 2 commits into
dpvc wants to merge 2 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1540 +/- ##
===========================================
- Coverage 86.93% 86.90% -0.03%
===========================================
Files 388 388
Lines 87571 87680 +109
Branches 3290 4978 +1688
===========================================
+ Hits 76129 76201 +72
- Misses 11442 11459 +17
- Partials 0 20 +20 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Member
Author
|
I accidentally created the PR without the initial message. I have edited it in above. |
zorkow
approved these changes
Aug 7, 2026
zorkow
left a comment
Member
There was a problem hiding this comment.
Three jsdoc issues. Otherwise fine.
| */ | ||
| public prev: ListItem<DataClass> = null; | ||
|
|
||
| public get isEnd() { |
| return a < b; | ||
| } | ||
|
|
||
| public first(): ListItem<DataClass> { |
| */ | ||
| public convert(document: MathDocument<N, T, D>, end: number = STATE.LAST) { | ||
| document.renderActions.renderConvert(this, document, end); | ||
| return document.renderActions.renderConvert(this, document, end); |
Member
There was a problem hiding this comment.
This will now require a returns statement in the overridden jsdoc, i.e, in the MathItem interface.
Member
There was a problem hiding this comment.
Probably also needs the return type.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The current
renderActionslist runs synchronously, but can throw a retry error when an asynchronous action is needed, and that error can be caught byhandleRetriesFor()to rerun the render-action list after the asynchronous action is complete. The various entries in the list each have a flag in the document's process-bits object that they use to tell if they have already been run or not, and they skip their action if they have been run. When one that hasn't completed is reached, it runs again, usually running a command on each of the math items on the page. Each math item marks its own state to tell when its action has been performed. That way, when a later math item causes a retry to occur, the math items that have already been handled can be skipped.That all works, but is a bit complicated, and there is a performance hit for handling the retries when you have to loop through the actions again and through the already processed math items for a suspended action. It would be nice to be able to use promises to make this work more efficiently.
This PR introduces a backward-compatible way to accomplish that. The
retryAfter()function can now include a function that is called when the promise is complete, so that the render actions can be suspended and then restarted at the point where they left off, rather than running through the entire action list and beginning of the math list again. ThehandleRetriesFor()function uses these restart functions when they exist, or re-runs the complete code that it received, as usual, when they don't. That means that existing code will continue to work without change, but can take advantage of the new promise-based restarts that have been added into the compile and typeset functions (which are the main ones that cause restarts), and new user-defined render actions can use promises to handle tasks that need them. (I have wished for this several times in the past.)Details
The most critical changes are in the
util/Retries.tsfile (at the bottom of the diff). Here, theRetryErroradds a newcodefield that includes the code to call in order to restart the process where it left off. This is supplies as part of theretryAfter()call.The
handleRetriesFor()function now works as follows (it may be easier to view this file directly rather than use the diff, it has been heavily revised):This way, the actions can restart where they left off without having to go back and rerun earlier actions, even when there are multiple retries required.
The other critical changes are in
core/MathDocument.ts, where the render-action processing is changed to handle return values that are promises, and where the compile and typeset actions have been refactored to be able to restart in the middle of the math list without having to go back and skip any earlier math items that were already processed.We define a new
RenderResulttype for convenience, and simplify the function that creates the render actions. TherenderDoc()andrenderMath()(used to process a complete document, or a single math item) both get a new parameter,i, that is the index into the render-action list of where to start, and after calling the action, if it returns a promise, we do a retry that continues the process from that point on.The
renderConvert()function gets a similar parameter and handles promises like above. It also now returns the mathtypesetRootorroot, depending on which is is defined.The
compile()andtypeset()methods are refactored so that they can be restarted using the new promise approach. The main work is done by thecompileAction()andtypesetAction()functions, which are the ones used by therenderDoc()functions. TherenderMath()function uses the originalcompile()andtypeset()functions, passing themfalseas their parameter. That is used in the action functions to determine if a restart promise should be used (when only compiling or typesetting outside of the render-action list, the original behavior is used).A try-catch structure is used to check if the
compileMath()call returns a promise, and if we are in a render action and the error is a retry error, we add athento the promise that finishes the compile or typeset action. The handling of recompiled expressions (due to references to labels that aren't defined yet) is factored out into a separate function, since that must also handle promise return values.In
core/MathItem.ts, thecompile()function returns therenderConvert()value, which will be the typeset root or the MathML root.The
core/MathList.tsfile now exports aMathListtype for convenience.The
util/LinkedList.tsfile now addsisEndandfirst()functions, for convenience.Finally, a number of new tests are added to the
Retries.test.jsto test the new functionality, and to simplify existing tests usingasyncandawaitrather than.then().