Stop bundling _any_ mathquill styles/fonts - let host app do that! #2385

jeremywiebe · 2025-04-11T20:59:59Z

Summary:

Fixes Perseus so that it doesn't ship Mathquill CSS with absolute font paths (ie /fonts/Symbola.ttf). This includes a bump to a new version of Mathquill.

This PR also updates the @khanacademy/math-input package to bundle all of mathquill (fonts, CSS, etc). This makes bundling things down the line much simpler. I couldn't find any apps consuming Perseus that actually used Mathquill directly, so this also aligns with that usage (consumer doesn't have to do anything special to accommodate Mathquill).

Also verified Storybook loads the Symbola font properly!

Issue: LEMS-3019

Test plan:

Update webapp with a test version of Perseus from this PR.
Build webapp - OK!
Run type checking and tests - OK!
Check webapp CSS and build output to ensure the fonts are "bundled" as assets and the CSS has correct paths for font files - OK!

github-actions · 2025-04-11T21:01:40Z

Size Change: +41.6 kB (+9.84%) ⚠️

Total Size: 465 kB

Filename	Size	Change
`packages/math-input/dist/es/index.js`	98.6 kB	+41.5 kB (+72.67%)	🆘
`packages/perseus/dist/es/index.js`	199 kB	+116 B (+0.06%)

ℹ️ View Unchanged

Filename	Size
`packages/kas/dist/es/index.js`	20.7 kB
`packages/keypad-context/dist/es/index.js`	1 kB
`packages/kmath/dist/es/index.js`	5.98 kB
`packages/math-input/dist/es/strings.js`	1.61 kB
`packages/perseus-core/dist/es/index.js`	18.4 kB
`packages/perseus-editor/dist/es/index.js`	88.5 kB
`packages/perseus-linter/dist/es/index.js`	7.05 kB
`packages/perseus-score/dist/es/index.js`	9.04 kB
`packages/perseus-utils/dist/es/index.js`	403 B
`packages/perseus/dist/es/strings.js`	6.13 kB
`packages/pure-markdown/dist/es/index.js`	1.22 kB
`packages/simple-markdown/dist/es/index.js`	6.71 kB

_{compressed-size-action}

github-actions · 2025-04-11T21:02:05Z

npm Snapshot: Published

Good news!! We've packaged up the latest commit from this PR (60e9460) and published it to npm. You
can install it using the tag PR2385.

Example:

pnpm add @khanacademy/perseus@PR2385

If you are working in Khan Academy's webapp, you can run:

./dev/tools/bump_perseus_version.js -t PR2385

…to control bundling and font resolution

…ymbola hash on the SVG font

jeremywiebe · 2025-04-16T00:01:50Z

packages/math-input/less/main.less

@@ -1,2 +1,2 @@
 @import "./overrides.less";
-@import (inline) "../node_modules/mathquill/build/mathquill.css";
+@import "mathquill/build/mathquill.css";


This is the correct import syntax to resolve a CSS/Less file out of an dependant npm package.

jeremywiebe · 2025-04-16T00:02:54Z

packages/math-input/package.json

        "@khanacademy/keypad-context": "workspace:*",
        "@khanacademy/perseus-core": "workspace:*",
-        "@khanacademy/perseus-utils": "workspace:*",
-        "mathquill": "file:../../vendor/mathquill-v1.0.0.tgz"


We move this to be in devDependencies because we now bundle everything from mathquill into @khanacademy/math-input. This relieves client apps from needing to depend on mathquill directly (most client apps don't use mathquill directly if they're using @khanacademy/math-input)

jeremywiebe · 2025-04-16T00:04:04Z

packages/perseus/src/styles/perseus-renderer.less

 }

-@import "../../../math-input/less/main.less";
+@import (reference) "@khanacademy/math-input/styles.css";


This becomes a (reference) now to avoid the situation where an app using math-input directly would "double" the math-input CSS because it's integrated into the Perseus CSS also.

This means that consuming apps will need to ensure that math-input CSS is properly imported, but that's a one-line change and allows for deduplication of math-input CSS.

jeremywiebe · 2025-04-16T00:05:02Z

vendor/mathquill-v1.0.2.tgz

This is not the final build yet. I'll bump this PR once more when the Mathquill PR lands and a new release is done.

…d .tgz

jeremywiebe · 2025-04-16T17:28:00Z

vendor/mathquill-v1.0.0.tgz

We can just reference the Git tag for the release on Github.

jeremywiebe · 2025-04-16T17:28:59Z

packages/math-input/.eslintrc.js

-                // note(matthewc): I changed this in a time pinch and I'm not sure
-                // it's right so I made a ticket to circle back to it
-                // https://khanacademy.atlassian.net/browse/LC-864
-                devDependencies: ["**/*.stories.tsx", "**/*.test.tsx"],


@handeyeco Weird. I can remove this in this PR and all linting passes. 🤷‍♂️

Copilot

Pull Request Overview

This PR stops bundling Mathquill CSS and font assets with Perseus so that the consuming app manages these resources. Key changes include:

Removing a devDependencies configuration (and its associated comments) from the ESLint configuration.
Updating the Rollup configuration to resolve and bundle Mathquill font assets with a custom URL resolver.
Bumping the version and updating the changeset to document the change.

Reviewed Changes

Copilot reviewed 4 out of 9 changed files in this pull request and generated 1 comment.

File	Description
packages/math-input/.eslintrc.js	Removed extraneous devDependencies configuration and associated comments.
config/build/rollup.config.js	Introduced a new resolve function using URL parsing to bundle Mathquill font assets.
.changeset/tame-grapes-ring.md	Added version bump and summary for stopping bundled Mathquill CSS/font assets.

Files not reviewed (5)

package.json: Language not supported
packages/math-input/less/main.less: Language not supported
packages/math-input/package.json: Language not supported
packages/perseus/src/styles/perseus-renderer.less: Language not supported
pnpm-lock.yaml: Language not supported

Comments suppressed due to low confidence (1)

packages/math-input/.eslintrc.js:10

Removing the devDependencies configuration might unintentionally affect linting of test and story files if they are not otherwise excluded. Verify that these files are still handled appropriately in the overall ESLint setup.

devDependencies: ["**/*.stories.tsx", "**/*.test.tsx"],

Copilot · 2025-04-16T22:26:46Z

config/build/rollup.config.js

+                        return {
+                            from: targetFile,
+                            source: fs.readFileSync(targetFile),


Consider adding error handling for fs.readFileSync to handle cases where a font file may be missing, which would otherwise cause a runtime error during the build.

Suggested change

return {

from: targetFile,

source: fs.readFileSync(targetFile),

let fileSource;

try {

fileSource = fs.readFileSync(targetFile);

} catch (error) {

console.error(`Error reading file: ${targetFile}`);

console.error(error.message);

throw error;

}

return {

from: targetFile,

source: fileSource,

It's safe to throw here as Rollup will capture the Error and fail the build with a sensible error.

jeremywiebe · 2025-04-16T22:33:19Z

config/build/rollup.config.js

+                        // Some inputUrl's have a hash (eg.
+                        // font/Symbola.svg#Symbola). We use the URL object to
+                        // ensure we only have the file path and nothing else.
+                        const url = new URL(inputUrl, "http://example.com");


I used the URL object here instead of string searching just to ensure we handle things correctly.

jeremywiebe · 2025-04-16T22:34:36Z

package.json

        "@storybook/react-vite": "^8.6.11",
        "@swc-node/register": "^1.10.10",
        "@swc/core": "1.11.13",
+        "@swc/helpers": "^0.5.17",


I saw warnings that a few files couldn't resolve this module. I'm not sure why it didn't happen earlier, but adding this to ensure our builds are correct.

jeresig

Great - thank you for reworking this, these changes look good to me!

handeyeco · 2025-04-17T14:48:23Z

.changeset/tame-grapes-ring.md

+"@khanacademy/math-input": minor
+---
+
+Stop bundling mathquill CSS - enables consuming app to control bundling and font resolution


I'm guessing we need to let Learning Equality know?

Actually I'm a little confused by this changeset. Here you're saying "enables consuming app to control bundling and font resolution" but in the comment below you say "This resolve is for the fonts in Mathquill. We want to bundle them into the math-input package so that consumers of @khanacademy/math-input don't need to also depend on the mathquill package." The two comments seem at odds, right?

It gives consumers control by bundling the Mathquill fonts into the @khanacademy/math-input package complete with correct CSS files point to the bundled font files using relative paths.

The custom resolve was for Mathquill fonts, but after you asked, I found I was able to simplify things and didn't need a custom resolver (the built-in one handles things fine). And that was exactly the motivation for updating Mathquill! Thanks.

handeyeco · 2025-04-17T14:50:31Z

packages/math-input/.eslintrc.js

-                // note(matthewc): I changed this in a time pinch and I'm not sure
-                // it's right so I made a ticket to circle back to it
-                // https://khanacademy.atlassian.net/browse/LC-864
-                devDependencies: ["**/*.stories.tsx", "**/*.test.tsx"],


@benchristel

This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @khanacademy/[email protected] ### Major Changes - [#2398](#2398) [`45635f7ef`](45635f7) Thanks [@benchristel](https://github.com/benchristel)! - Remove ReadonlyArray types from `@khanacademy/perseus-core` in favor of mutable arrays. Users should define separate readonly types if desired. ## @khanacademy/[email protected] ### Minor Changes - [#2385](#2385) [`a7f293aab`](a7f293a) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Stop bundling mathquill CSS - enables consuming app to control bundling and font resolution ### Patch Changes - Updated dependencies \[[`45635f7ef`](45635f7)]: - @khanacademy/[email protected] - @khanacademy/[email protected] ## @khanacademy/[email protected] ### Patch Changes - Updated dependencies \[[`45635f7ef`](45635f7)]: - @khanacademy/[email protected] ## @khanacademy/[email protected] ### Patch Changes - Updated dependencies \[[`45635f7ef`](45635f7)]: - @khanacademy/[email protected] ## @khanacademy/[email protected] ### Patch Changes - [#2404](#2404) [`457c9b818`](457c9b8) Thanks [@ivyolamit](https://github.com/ivyolamit)! - Add codeowners for radio widget - Updated dependencies \[[`45635f7ef`](45635f7), [`a7f293aab`](a7f293a)]: - @khanacademy/[email protected] - @khanacademy/[email protected] - @khanacademy/[email protected] - @khanacademy/[email protected] - @khanacademy/[email protected] - @khanacademy/[email protected] ## @khanacademy/[email protected] ### Patch Changes - Updated dependencies \[[`45635f7ef`](45635f7), [`457c9b818`](457c9b8), [`a7f293aab`](a7f293a)]: - @khanacademy/[email protected] - @khanacademy/[email protected] - @khanacademy/[email protected] - @khanacademy/[email protected] - @khanacademy/[email protected] - @khanacademy/[email protected] - @khanacademy/[email protected] ## @khanacademy/[email protected] ### Patch Changes - Updated dependencies \[[`45635f7ef`](45635f7)]: - @khanacademy/[email protected] ## @khanacademy/[email protected] ### Patch Changes - Updated dependencies \[[`45635f7ef`](45635f7)]: - @khanacademy/[email protected] - @khanacademy/[email protected] ## @khanacademy/[email protected] ### Patch Changes - Updated dependencies \[[`45635f7ef`](45635f7), [`a7f293aab`](a7f293a)]: - @khanacademy/[email protected] - @khanacademy/[email protected] - @khanacademy/[email protected] - @khanacademy/[email protected] Author: khan-actions-bot Reviewers: jeremywiebe Required Reviewers: Approved By: jeremywiebe Checks: ⏭️ 1 check has been skipped, ✅ 4 checks were successful Pull Request URL: #2405

jeremywiebe self-assigned this Apr 11, 2025

jeremywiebe added the olc-5.0.9a72e label Apr 11, 2025

jeremywiebe added 5 commits April 14, 2025 15:31

Stop bundling _any_ mathquill styles/fonts - let host app do that!

dd46e1e

docs(changeset): Stop bundling mathquill CSS - enables consuming app …

041cf94

…to control bundling and font resolution

WIP

244028f

Use new build of mathquill (v1.0.2)

58b47fc

Fix build so mathquill fonts are bundled!

acf44e1

jeremywiebe force-pushed the jer/mathquill-relative-font-paths branch from 2ce7101 to acf44e1 Compare April 15, 2025 19:04

jeremywiebe added 5 commits April 15, 2025 13:59

Address stopship

65203fc

Move mathquill dep to be a devDependency (because we bundle it)

357c027

Fix build so mathquill is not marked as external and we retain the #S…

0c069ea

…ymbola hash on the SVG font

Whew, hope this works now

74b367e

Fix rollup-plugin-auto-external to be exactly what it was before

6db55ec