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

Add versions to component files and check them when loaded #738

Merged
merged 7 commits into from
Feb 25, 2022

Conversation

dpvc
Copy link
Member

@dpvc dpvc commented Aug 18, 2021

This PR adds support for version checking so that components know the version that was used to build them, and the component loader can check the component version to make sure it is the same as the one loading it. If there is a mismatch, a (non-fatal) warning is issued. (In the future, this could be extended to make better choices about whether the version mismatch should be fatal or not.) The warnings can be suppressed via the versionWarnings: false option to the loader configuration.

The idea is that, now that we are going to have separate repositories for fonts and TeX extensions, and people are going to be making their own extensions, we need a means of checking whether the extensions can work with the vision of MathJax that is loading them. This is a first step in that direction, which at least indicates when a version mismatch occurs.

Note that most of the files in this PR are only changed to include an ID to use with the versions, so can be gone over quickly.

@dpvc dpvc requested a review from zorkow August 18, 2021 15:20
…le type to be passed (for potential future use).
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.

An alternative could be to import the package.json and read the version from there. This way there is no need to keep both in sync.

ts/components/version.ts Outdated Show resolved Hide resolved
* @author [email protected] (Davide Cervone)
*/

export const VERSION = '3.2.0';
Copy link
Member

Choose a reason for hiding this comment

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

This could be computed from the package.json.

Copy link
Member Author

Choose a reason for hiding this comment

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

This PR is about having the webpacked components be able to tell what version they were built under, so the version needs to be part of the packed component file. The main reason for this is to handle third-party components that may not be part of our repository, so that if someone loads an extension from another location (something like the xypic extension that is packaged and distributed separately), there is a check that it is compatible with the running version of MathJax.

So either the complete package.json would have to be included in the webpacked component file (so that its value could be determined at run time), or the lookup would have to be part of the component build process, say in the webpack.config.js file, and the version would be passed into the code as a variable (I'm sure that can be done, but I'll have to look up how that works). But if that is how it is done, then there would have to be an alternative path for use in node (and the lab) that looks it up from the package.json file and makers it available to the components that are being loaded from source (rather than from component packages); that is, for packages loaded from components/src like the lab does, or like the node examples do by default when the --dist option isn't present.

That can be done I suspect, but it makes things much more complicated. Or am I misunderstanding the comment?

Currently, the mathjax.js file has a hard-coded version number. That has simply been moved to this version.ts file instead, so this PR is no worse than the current situation.

Copy link
Member Author

Choose a reason for hiding this comment

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

See my comments below for the solution I was able to work out.

Base automatically changed from build-tools to develop February 22, 2022 13:25
@dpvc dpvc added this to the 3.2.1 milestone Feb 22, 2022
@dpvc
Copy link
Member Author

dpvc commented Feb 24, 2022

OK, I've updated webpack.common.js to look up the version from the package.json file, and version.ts to either use that value (when in a webpacked version), or look up the version from package.json directly itself if run in node. The lookup in version.ts is removed by webpack's tree shaking, so it doesn't enlarge the resulting file (yay!). This works everywhere expect in the lab, where you need to define the global PACKAGE_VERSION = "3.2.0" or maybe v3-lab instead of 3.2.0.

So I think this does what you want without adversely affecting the webpacked versions. So good suggestion.

@dpvc dpvc requested a review from zorkow February 24, 2022 14:41
@zorkow
Copy link
Member

zorkow commented Feb 25, 2022

Yes, that's exactly what I had in mind: Version should be looked up during webpacking and is then set in stone.

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.

Another advantage is that we can now simply use npm version for version bumps.

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