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

Fix \require to properly handle retries in dependencies. (mathjax/MathJax#3170) #1050

Merged
merged 2 commits into from
Feb 12, 2024

Conversation

dpvc
Copy link
Member

@dpvc dpvc commented Feb 6, 2024

When \require{} is used to load an extension, then if the extension has preprocessors, that means the the expression needs to be re-typeset so that the preprocessors will be run. This is handled by calling mathjax.retryAfter() during the RegisterExtension() function.

If the extension has other extensions as dependencies, then they may also cause re-typesetting if they have preprocessors. That means the RegisterDependencies() function may throw a retry error, which stops the processing of the dependencies, and prevents the remainder of the RegisterExtension() function from completing.

This occurs, for example, when \require{textcomp} is processed, as textcomp depends on textmacros, and the latter has preprocessors, so retryAfter() is called when the textmacros dependency is registered. That means the RegisterExtension() for the textcomp will be interrupted at the RegisterDependencies() call, and the rest won't be processed. Because textcomp has already been pushed onto the required array, the retype setting will not run the main code for RegisterExtension, and textcomp won't be properly added to the configuration. That means that the macros won't be defined, leading to unexpected errors reporting undefined macros.

This PR resolves the problem by trapping the retry errors in the RegisterDependencies() function and returning either null if there were none, or a promise that resolves when all the dependencies are loaded. The RegisterExtension function then either waits for these those promises and adds the main extension, restarting the typesetting after that, or if there are no dependency promises, it adds the extension immediately and continues processing the expression. To make that easier, the main part of RegisterExtension has been broken out into a new ProcessExtension() function that can be called in either situation.

Resolves issue mathjax/MathJax#3170.

@dpvc dpvc requested a review from zorkow February 6, 2024 20:56
@dpvc dpvc added this to the v4.0 milestone Feb 6, 2024
@dpvc
Copy link
Member Author

dpvc commented Feb 6, 2024

Viewing without whitespace differences may make the changes clearer.

@pkra
Copy link
Contributor

pkra commented Feb 7, 2024

This is a random question that I've been meaning to ask - apologies for abusing this PR to do so:

Is it correct that an extension doesn't have to do anything beyond importing another extension to indicate that it has a dependency?

(IIRC in v2 we had to do some extra legwork to alert MathJax to such dependencies.)

@dpvc
Copy link
Member Author

dpvc commented Feb 7, 2024

Is it correct that an extension doesn't have to do anything beyond importing another extension to indicate that it has a dependency?

Unfortunately, no. The dependencies are defined in the components/mjs/dependencies.js file, though it is possible to use the MathJax configuration to add additional dependency definitions.

Also unfortunately, it is not possible for the the extension's component file to define the dependencies itself, because those have to be in place before the extension is loaded (so that the dependencies will be loaded first). It would be possible, I suppose, to have a small extension/def.js file that adds dependencies to the configuration so that you could do

<script>
MathJax = {
...
}
</script>
<script src="<URL-fo-extension>/def.js"></script>
<script defer src="https://cdn.jsdelivr.net/npm/mathjax/tex-svg.js"></script>

where def.js did something like

if (!window.MathJax) window.MathJax = {};
if (!MathJax.loader) MathJax.loader = {};
if (!MathJax.loader.dependencies) MathJax.loader.dependencies = {};
MathJax.loader.dependencies['[path]/extension'] = ['input/tex-base', '[tex]/dep1', '[tex]/dep2'];

where path is the name of the path you are using to load the extension, and extension is the name of the extension. This will make sure that, if [path]/extension is loaded, the dependencies will be loaded first.

The def.js file could even do

if (!MathJax.loader.paths) MathJax.loader.paths = {};
if (!MathJax.loader.paths['path']) {
  MathJax.loader.paths['path'] = '<URL-to-extension>';
}
if (!MathJax.tex) MathJax.tex = {};
if (!MathJax.tex.packages) MathJax.tex.packages = {};
if (Array.isArray(MathJax.tex.packages)) {
  MathJax.tex.packages.push('[path]/extension');
} else {
  if (!MathJax.tex.packages['[+]']) MathJax.tex.packages['[+]'] = [];
  MathJax.tex.packages['[+]'].push('[path]/extension');
}

to configure the loading of the extension and adding it to the the package list. (This code is untested, so it might need tweaking.) That way, the user only needs to load defs.js after the script the sets the MathJax configuration in order to load and install your extension (without needing to set loader.load or tex.packages explicitly).

This does mean there is an extra (synchronous) file download, which is not great, but you could use defer to help improve the situation, as long as you use defer (and not async) with the script that loads tex-svg.js.

The reason that the dependencies must be loaded first is that this is needed if an extension shares code with another extension. For example, if extension A has a class ClassA and extension B imports ClassA and makes a subclass ClassB, then ClassA has to be available at the time extension B is loaded so that the subclass can be created. The sharing is done through the MathJax._ object. Extension A will store a pointer to ClassA in MathJax._.input.tex.extension.ClassA (or some similar location) when the extension's component file loads its lib/extension.js file, and the webpacking of extension B will replace the input/tex/extension/extension.js (or wherever ClassA comes from) with a file that exports ClassA from MathJax._.input.tex.extension.ClassA. In order for that to work, MathJax._.input.tex.extension.ClassA has to already be in place; i.e., extension A has to already be loaded. Because ClassA is used when extension B is initially compiled and run by the browser, the loading of extension A has to be before that, and so running the code for extension B can't be what causes extension A to load, as that would be too late. So the configuration for the dependencies has to be done before extension B is loaded. Sigh.

@pkra
Copy link
Contributor

pkra commented Feb 7, 2024

Thanks for clarifying, Davide. I'm sorry for making you write up such a complete response. There's never been a problem in production so I stopped worrying about it some time ago.

@dpvc
Copy link
Member Author

dpvc commented Feb 7, 2024

No problem. Since you are running your code server-side, and not through webpacked components in the browser, you don't need to worry about the dependencies, as there is no MathJax._ sharing going on and regular import commands will work without all the component magic for the separate webpacked files that need to share common code.

@pkra
Copy link
Contributor

pkra commented Feb 7, 2024

Right I was actually thinking of client-side use. The specific situation is just far less complex so the dependencies always load first anyway.

Copy link
Member

@zorkow zorkow left a comment

Choose a reason for hiding this comment

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

lgtm.

@dpvc dpvc merged commit f798364 into develop Feb 12, 2024
@dpvc dpvc deleted the issue3170 branch February 12, 2024 16:04
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.

None yet

3 participants