fix(parser): bundle langium and chevrotain inside @mermaid-js/parser#7658
Conversation
Use `@microsoft/api-extractor` to bundle the TypeScript `.d.ts` types for `@mermaid-js/parser`. In a future commit, we want to bundle `langium`, which would need us to bundle `langium`'s types as well. Bundling reduces the size of our `dist/` folder, and makes it more obvious which of our types are external. I've made this as a `prepack` step, so that it doesn't affect the majority of mermaid developers when they run `pnpm install`. It's only when we publish the package that we'd bundle the code. This also means it will be tested by the `pnpm run test:check:tsc` test that we have.
Bundle langium and chevrotain in the `@mermaid-js/parser` package, so they're no longer dependencies. This has the following benefits: 1. Chevrotain v11.1.1 has a pin on lodash-es v4.17.23. There are a couple of CVEs/alerts on that version, and chevrotain will not make a new v11 release since those alerts don't affect chevrotain, see Chevrotain/chevrotain#2186 2. Langium v4 raises an install warning on Node.JS v20.0, which is causing issues for some of mermaid's users, even if this code only runs in the browser. I'm using `api-extractor` to bundle the types for this. We're still keeping the `@chevrotatin/types` package as a dependency, since `api-extractor` can't seem to handle it, and it's only used for types.
✅ Deploy Preview for mermaid-js ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
🦋 Changeset detectedLatest commit: 675a64c The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@mermaid-js/examples
mermaid
@mermaid-js/layout-elk
@mermaid-js/layout-tidy-tree
@mermaid-js/mermaid-zenuml
@mermaid-js/parser
@mermaid-js/tiny
commit: |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #7658 +/- ##
==========================================
- Coverage 3.31% 3.31% -0.01%
==========================================
Files 539 540 +1
Lines 56719 56760 +41
Branches 824 825 +1
==========================================
+ Hits 1880 1881 +1
- Misses 54839 54879 +40
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
ashishjain0512
left a comment
There was a problem hiding this comment.
[sisyphus-bot]
Nice tightly-scoped fix — silencing the chevrotain/lodash CVE noise and the langium-on-Node-20 warning by bundling at the package boundary is exactly the right shape for this. The motivation is well-documented in the PR body and the changeset.
What's working well
- 🎉 [praise] Diagnosis is precise and the writeup links upstream context — the chevrotain v11/lodash situation (Chevrotain/chevrotain#2186) and the langium-v4-on-Node-20 thread are both linked, which means anyone tracing this fix in six months will find the upstream story instead of guessing why we own a rollup of langium types now.
- 🎉 [praise]
api-extractor.jsonconfiguration is thoughtful. EnablingdtsRolluponly, disabling the other outputs we don't need, and explicitly silencingae-forgotten-exportandae-missing-release-tag(with the inline comment explaining that langium-generated files don't carry TSDoc tags) — that's the right level of curation for a generated codebase. Same applies totsdocMessageReporting.default: "none"with the "we don't have control over bundled types" comment. - 🎉 [praise]
localBuild: !process.env.CIis the right knob. Warnings are fatal in CI and non-fatal locally, which keeps publish gates strict without making developer iteration miserable. And thesucceededcheck inprepack.ts:30-35throws a clear "N errors and N warnings" message rather than swallowing failures.
Nits
- 🟢 [nit] Cleanup loop sort comment. In
prepack.ts:53, the directories are sorted-then-reversed so children get removed before their parents. The comment is there ("delete subdirectories before their parents") but the sort being a deepest-first pass is the load-bearing detail — worth pulling that intuition into the comment, e.g.// Sort lexicographically and reverse so deeper paths come first. Minor readability thing.
Suggestions
- 💡 [suggestion] Type-identity loss for downstream consumers. Once this lands, anyone who uses
@mermaid-js/parserand langium directly in the same project gets two distinct copies ofAstNode— structural identity still works, but nominalinstanceofchecks across the boundary won't. Since@mermaid-js/parseris mostly an internal package consumed bymermaiditself, this is fine, but it might be worth a one-liner in the parser package's README ("consumers should not also depend onlangiumdirectly; the types are bundled here") so anyone who hits this in the future has a fast answer. - 💡 [suggestion] Forward-port plan. Since this targets
release/11.15.0, it'll need a follow-up merge intodevelop(or a cherry-pick) to keep the branches in sync. Mostly a process note — mentioning explicitly so it doesn't fall off the radar after this lands. - 💡 [suggestion] Smoke test that the rollup actually exports what consumers expect.
tsc-check.tsflipping@mermaid-js/parserfrom skipped to checked goes a long way here, but a small end-to-end check that imports a few key symbols fromdist/src/index.d.tsafterprepackruns (e.g., as part of CI) would catch silent regressions if api-extractor's rollup behavior changes. Not blocking —tsc-check.tscovers most of this.
Security
No XSS or injection surface — this PR is purely build configuration and a publish-time script. No runtime code changes, no DOM/SVG output paths touched. Skipping the sub-agent pass since there's nothing for it to analyze.
LGTM, happy to see this one land. 🙌
📑 Summary
Note
I'm targeting the
release/11.15.0branch, instead ofdevelop.Bundle langium and chevrotain in the
@mermaid-js/parserpackage, so they're no longer dependencies.This has the following benefits:
I'm using
api-extractorto bundle the types for this. We're still keeping the@chevrotatin/typespackage as a dependency, sinceapi-extractorcan't seem to handle it, and it's only used for types.📏 Design Decisions
Describe the way your implementation works or what design decisions you made if applicable.
📋 Tasks
Make sure you
pnpm run test:check:tsctestMERMAID_RELEASE_VERSIONis used for all new features.pnpm changesetand following the prompts. Changesets that add features should beminorand those that fix bugs should bepatch. Please prefix changeset messages withfeat:,fix:, orchore:.