Skip to content

Better support for Loader.preLoaded() when used in combined components in node #1310

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

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

dpvc
Copy link
Member

@dpvc dpvc commented Jul 16, 2025

This PR fixes a problem when combined components are loaded via the loader.load configuration array in node applications and one of the preloaded components needs to perform additional loads (like when the output jax needs to load a font). In the past, the load of the combined component would complete before the extra components are loaded, and so MathJax could process the page with the wrong font, for example.

In the current develop branch, the code

#! node

import {init} from '@mathjax/src/source';

await init({
  loader: {load: ['tex-chtml']},
  output: {font: 'mathjax-tex'},
  options: {enableSpeech: false, enableBraille: false},
})

const node = await MathJax.tex2chtmlPromise('x');
console.log(MathJax.startup.adaptor.outerHTML(node));

produces

<mjx-container class="MathJax" jax="CHTML" overflow="overflow" display="true"><mjx-math data-latex="x" data-semantic-structure="0" display="true" style="margin-left: 0; margin-right: 0;" class=" NCM-N"><mjx-mi data-latex="x" data-semantic-type="identifier" data-semantic-role="latinletter" data-semantic-font="italic" data-semantic-id="0" data-semantic-attributes="latex:x" aria-level="0" data-speech-node="true"><mjx-c class="mjx-c1D465">𝑥</mjx-c></mjx-mi></mjx-math></mjx-container>

Note the NCM-N class on the mjx-math element indicating it is still using the mathjax-newcm font.

With this patch, it will produce

<mjx-container class="MathJax" jax="CHTML" overflow="overflow" display="true"><mjx-math data-latex="x" data-semantic-structure="0" display="true" style="margin-left: 0; margin-right: 0;" class=" TEX-N"><mjx-mi data-latex="x" data-semantic-type="identifier" data-semantic-role="latinletter" data-semantic-font="italic" data-semantic-id="0" data-semantic-attributes="latex:x" aria-level="0" data-speech-node="true"><mjx-c class="mjx-c1D465">𝑥</mjx-c></mjx-mi></mjx-math></mjx-container>

with the correct TEX-N class for mathjax-tex.

It took me several days to figure out a means of handling this cleanly. The Package object is quite complicated, and probably deserves a rewrite now that MathJax has matured since it was originally written, but that will have to wait for another release.

The process used here is to have Loader.load() keep track of any Loader.load() calls that occur before it completes. this is done using an array created in the load() call that is used to store the promises produces by any load() calls that are made while the first one is waiting to complete. We keep an array of these arrays so that if a nested load() call causes every more load() calls, they are stored for the nested call (so it knows when all the calls it initiated are resolved). They are also added to any higher-up parent load() call arrays so that those higher-up loads wait for all the lower-down ones as well. (This was a key step in getting this to work.) Then when the initial loading is complete, we check if any additional loads have been added to the nested list, and if so, we wait for those after clearing the list. If more loads occurred during that wait, we do it again, and keep doing so until no more loads were processed during the wait. Then we remove our nested list from the list of lists, and finally are able to resolve the load promise.

We could also remove our promise from any higher-up lists at that point, since we now know our load is complete, but since our promise will now be resolved, it doesn't really hurt to leave it in place as it will not add any delay to the Promise.all() for any of the higher up loads. So we don't take time to go through any of the higher lists to remove our promise. If you think that is a good idea, I can add that it.

Finally, the change to Package.ts is to combine the loading of any extraLoads components into a single Loader.load() call rather than making one for each extra load. That is a little more efficient.

It may be easier to look at the diff with spaces being ignored, as some of the changes are indentations.

@dpvc dpvc requested a review from zorkow July 16, 2025 15:26
@dpvc dpvc added this to the v4.0 milestone Jul 16, 2025
Copy link

codecov bot commented Jul 16, 2025

Codecov Report

Attention: Patch coverage is 95.06173% with 4 lines in your changes missing coverage. Please review.

Project coverage is 86.72%. Comparing base (4d1e195) to head (97e879e).

Files with missing lines Patch % Lines
ts/components/loader.ts 94.87% 4 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #1310   +/-   ##
========================================
  Coverage    86.71%   86.72%           
========================================
  Files          337      337           
  Lines        83979    84032   +53     
  Branches      4750     4756    +6     
========================================
+ Hits         72826    72879   +53     
  Misses       11130    11130           
  Partials        23       23           

☔ View full report in Codecov by Sentry.
📢 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.

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.

1 participant