-
-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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
fix 5747 replace $root with mermaid/dist #5805
fix 5747 replace $root with mermaid/dist #5805
Conversation
|
✅ Deploy Preview for mermaid-js ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
commit: |
howdy folks! (esp. @knsv @sidharthv96) I've seen a couple releases go out, but still have been unable to kick the tires on any new features/fixes due to #5747. Please consider a review here (or the somewhat smaller #5798), so that downstreams using typescript can more easily start adopting the |
Hi @bollwyvl, the We came across https://github.com/Rich-Harris/dts-buddy while checking for alternatives, but it has a very prominent warning to not use it. |
Sure, I personally prefer the relative ones, and can prepare an alternate PR. I presumed there was some reason the |
Please see #5838 |
Yep. I've found pretty much anything "fancy," in a TypeScript build just makes a project harder to maintain over time, and much more complicated for downstreams. TS is still likely never going to land as a first-party citizen in the browser, but the ecosystem is big enough on the developer side to try to do it The Most Widely-Compatible Way whenever possible. Put differently: if fancy tool gains are great enough, they may be worth it for a delivered application built by a single organization... but maddening for downstreams when it happens in a library. For example, adopting |
📑 Summary
$root
withmermaid/dist
pnpm test:check:tsc
which builds the tarballs and uses them in a minimal TypeScript projectResolves #5747.
Includes #5798 (this would have also failed
tsc
, as therendering-utils/types.d.ts
would not be present).📏 Design Decisions
This relies on using a
tsconfig.json#/compilerOptions/paths
that exactly matches the structure of the as-distributed files, such that downstream packages won't have to add anything to their configuration (e.g. a special case for$root
,skipLibCheck
, etc). Note this only checks whether the packages can be imported, as adding much of anything else would likely make these baseline, language-level failures harder to debug.The added
script/tsc-check.ts
runs out-of-tree, and usesnpm
to avoid pollution from the build system, and cleans up after itself on the happy path, but leaves the files in place for interactive inspection in the event of a failure.The
parser
andzenuml
packages are commented out, as they presently fail in this setup, but as I don't use them, I have left this for someone who does to get them under control.📋 Tasks
Make sure you
MERMAID_RELEASE_VERSION
is used for all new features.pnpm changeset
and following the prompts. Changesets that add features should beminor
and those that fix bugs should bepatch
. Please prefix changeset messages withfeat:
,fix:
, orchore:
.