[Build Config] Emit shared modules once per package - #4233
Open
jeremywiebe wants to merge 3 commits into
Open
jeremywiebe wants to merge 3 commits into
jeremywiebe wants to merge 3 commits into
Conversation
Contributor
npm SnapshotWant to try this PR's changes before it merges? Comment |
Contributor
|
Size Change: -346 B (-0.07%) Total Size: 499 kB 📦 View Changed
ℹ️ View Unchanged
|
jaredly
reviewed
Sep 17, 2026
| @@ -0,0 +1,51 @@ | |||
| import {getEntryPoints} from "../get-entry-points"; | |||
Collaborator
There was a problem hiding this comment.
any chance these new files can be in typescript? now that we're on node24, node can run .ts files natively for the most part.
Collaborator
Author
There was a problem hiding this comment.
Eventually. I'd like to avoid pulling that in as something for this PR though. I have been working to move all of this repo to Vite v8 which uses Rolldown for its build infra in both server and bundle mode. In that set of changes, everything is TypeScript.
jeremywiebe
added a commit
that referenced
this pull request
Sep 21, 2026
…#4251) Math in various stories intermittently rendered with extra, or missing, space beneath the equation, which surfaced as unexplained snapshot changes on #4233. `TestMathjax` called `onRender` synchronously, so `Zoomable` measured the math before the MathJax web fonts had loaded and stored the taller fallback-font height. Nothing re-measures on font load, so the stale height stuck whenever the font fetch lost the race. `TestMathjax` now defers `onRender` until `document.fonts.ready` resolves. It forces a layout first because `fonts.ready` only waits for fonts the browser has already requested. The graphie test that shadowed `document.fonts` with an own property now uses the `jest.spyOn` getter pattern instead, same as the new `TestMathjax` test. Issue: LEMS-4615 ## Test plan: - `pnpm test` - Confirm the Chromatic build is stable (might need to land this before we start to see) Author: jeremywiebe Reviewers: jeremywiebe, benchristel, nishasy, mark-fitzgerald, handeyeco, catandthemachines Required Reviewers: Approved By: benchristel Checks: ⏭️ 2 checks have been skipped, ✅ 10 checks were successful Pull Request URL: #4251
handeyeco
approved these changes
Sep 22, 2026
Collaborator
Author
|
/snapshot |
jeremywiebe
force-pushed
the
jer/rollup-shared-chunks
branch
from
September 22, 2026 23:20
4915601 to
78a0727
Compare
Collaborator
Author
|
/snapshot |
… helper The Rollup config found entry points inline in getPackageInfo(), reading the exports map's non-standard `source` condition and taking each output path from that entry's `import`/`require` target. That logic is about to be needed for multi-entry builds with shared chunks, so pull it out. getEntryPoints() in config/build/get-entry-points.js returns a map of entry name to source file: one entry per exports sub-path that has a `source` condition, or a single `index` entry from the package's `source` field when there is no exports map. Sub-paths that resolve straight to a built asset (`./styles.css`) are skipped. getPackageInfo() now derives output paths by convention instead of from the exports map — `dist/<name>.js` for CJS, `dist/es/<name>.js` for ESM. That matches the `main`, `module` and `exports` targets of every package today, so `pnpm build` emits the same file set as before. Also adds tests for the helper and rewrites getPackageInfo()'s doc comment, which described output paths as coming from package.json.
Each entry point was built by its own Rollup config, so a module reachable from more than one of them was duplicated into every bundle that reached it, each copy with its own module state. For anything that holds state, such as a registry, that is a runtime bug and not just wasted bytes. All of a package's entry points are now inputs to a single Rollup config per format, with `dir` output instead of `file`: one file per entry (`[name].js`) plus `chunk-[name]-[hash].js` for anything shared between them. CJS still lands in `dist/` and ESM in `dist/es/`, so the `main`, `module` and `exports` targets of every package are unchanged. Worth knowing: this emits no chunks today. `perseus` and `math-input` are the only packages with more than one entry point, and their graphs are disjoint — the two modules that import `strings.ts` do so with `import type`, which is erased before Rollup sees it. The published output is therefore unchanged; the fix is structural, so that a module which becomes shared later is hoisted instead of silently duplicated. Adds tests that run the real generator against the real `packages/` folder, covering format selection via `--configFormats`, the output dirs, the file name patterns, and that a package's entry points all land in one config. The compressed-size CI job's `**/dist/es/*.js` pattern now also matches the shared chunks; its comment says so.
jeremywiebe
force-pushed
the
jer/rollup-shared-chunks
branch
from
September 23, 2026 18:59
78a0727 to
91560e5
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
Original attempt at moving to ESM-only was #4233
#4242 changes the project to ESM-only again, which leverages the changes from this PR.
Each entry point is currently built by its own Rollup config, so a module reachable from more than one of them is duplicated into every bundle that reaches it, each copy with its own module state. For anything that holds state, such as a registry, that is a runtime bug (one "global" becomes N, one for each entry point in the package).
Every one of each package's entry points are now inputs to a single Rollup config per format (CJS/ESM). We use
diroutput instead offileso that we still get one file per entry ([name].js) but alsochunk-[name]-[hash].jsfor anything shared between them.Important note: nothing emits any chunks today.
perseusandmath-inputare the only packages with more than one entry point, and their graphs are the two entry points have no overlapping imports. There are only sharedtypeimports, but the build erases those so it doesn't cause any "chunking."I've added tests that run the real generator against the real
packages/folder, covering both output formats, the output dirs, the file name patterns, and that a package's entry points all land in one config.The compressed-size CI job's
**/dist/es/*.jspattern now also matches the shared chunks; its comment says so.Issue: LEMS-2247
Test plan:
pnpm testpnpm typecheckpnpm buildand confirm each package still emitsdist/anddist/es/, with one file per entry point (index.js, plusstrings.jsforperseusandmath-input)chunk-*.jsfiles are produced - that's expected, since no package's entry points currently share a module