Skip to content

Conversation

@dpvc
Copy link
Member

@dpvc dpvc commented Dec 31, 2025

In Windows, importing a file from an absolute path requires the path to include the file:// protocol (that is not needed in unix and MacOS). This PR modifies the context.path() function that adjusts the Windows \ to / to also add the needed file:// protocol for absolute references. It also adds tests to get full coverage of the changes. Finally, it adjusts the asyncLoad/system.ts file to include file:// (though the system.ts file is a legacy one that isn't really used any longer since node handles ESM loading natively).

Resolves issue mathjax/MathJax#3481.

@dpvc dpvc requested a review from zorkow December 31, 2025 18:12
@dpvc dpvc added this to the v4.1.1 milestone Dec 31, 2025
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.

I am getting tests errors. But maybe I am running it wrong on Windows.

? file.replace(/\\/g, '/').replace(/^\//, '')
: file;
? 'file://' + file.replace(/\\/g, '/').replace(/^\//, '')
: file.replace(/^\//, 'file:///');
Copy link
Member

Choose a reason for hiding this comment

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

This now leads to an error in Context-node.test.ts:

Received: "file:///C:/test.js"

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, I forgot about this test (I didn't run it on windows, and that test is only performed there).

I fixed the test. I had to merge your fix/windows-tests branch to get the tests to run on windows, but that is still listed as a draft PR. Do you want to make it a full PR so we can include it in 4.1.1?

@zorkow
Copy link
Member

zorkow commented Jan 8, 2026

I've approved, but please remove that superfluous /.

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.

3 participants