Skip to content

Use retryAfter() and handleRetriesFor() from mathjax so they are easier to override - #1547

Open
dpvc wants to merge 1 commit into
developfrom
update/retries
Open

Use retryAfter() and handleRetriesFor() from mathjax so they are easier to override#1547
dpvc wants to merge 1 commit into
developfrom
update/retries

Conversation

@dpvc

@dpvc dpvc commented Aug 9, 2026

Copy link
Copy Markdown
Member

This PR moves the references to retryAfter() and handleRetriesFor() to the version in the mathjax variable rather than from the Retries.ts module directly. This makes it easier to override them. I have an example of making MathJax load tex extensions synchronously that would be helped by this, as that means there would be fewer methods that need to be overridden. Together with the synchronous loading of dynamic font data and of entity definitions, this would allow MathJax to be run synchronously in node applications, where synchronous file loading can be achieved.

Note that asyncLoad uses mathjax.asyncLoad internally, so we don't have to use mathjax for that.

@dpvc
dpvc requested a review from zorkow August 9, 2026 11:45
@dpvc dpvc added this to the v4.2 milestone Aug 9, 2026
@codecov

codecov Bot commented Aug 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 40.00000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.92%. Comparing base (6c2be35) to head (75043d5).

Files with missing lines Patch % Lines
ts/core/MathDocument.ts 0.00% 3 Missing ⚠️
ts/output/common/FontData.ts 0.00% 3 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1547      +/-   ##
===========================================
- Coverage    86.92%   86.92%   -0.01%     
===========================================
  Files          390      390              
  Lines        87853    87852       -1     
  Branches      4984     4984              
===========================================
- Hits         76370    76369       -1     
  Misses       11464    11464              
  Partials        19       19              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@zorkow zorkow left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have two concerns about the new async loading. Please have a look.

dynamic.promise = Promise.resolve();
try {
mathjax.asyncLoad(this.dynamicFileName(dynamic));
asyncLoad(this.dynamicFileName(dynamic));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This now returns a promise regardless of if it fails or not. So the try...catch does not really have an effect on this. But that means that dynamic.failed is never set. That would mean the file is set to loaded
even if the promise failed.

Comment thread ts/util/Entities.ts
if (!loaded[file]) {
loaded[file] = true;
const promise = mathjax.asyncLoad(`./util/entities/${file}.js`);
const promise = asyncLoad(`./util/entities/${file}.js`);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, should the promise failure here not be handled explicitly?

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

Successfully merging this pull request may close these issues.

2 participants