Support node16 module resolution

[remcohaszing]

📑 Summary

The node16 module resolution requires imports to use the .js file extension in type definitions. This also needs to be the case for dependencies. This fixes usage of Mermaid with projects that use "module": "node16".

📏 Design Decisions

@rollup/plugin-typescript is needed to make this work with the Vite setup used by Mermaid.

The module option for Mermaid internally is set to nodenext. This is needed to support .json imports. Note that setting module to node16 or nodenext implies a matching moduleResolution value.

📋 Tasks

Make sure you

• 📖 have read the contribution guidelines
• 💻 have added unit/e2e tests (if appropriate)
• 📓 have added documentation (if appropriate)
• 🔖 targeted develop branch

[remcohaszing]

This almost fixes it, but I can’t get the Cypress tests to succeed.

[aloisklink]

We've had issues with accidentally missing the .js extensions before (see #4170), so having TypeScript check this for us would be really helpful.

Thanks for making this PR, I've been working on something similar (see #4217), but you managed to make a PR before I did 😄 My implementation didn't need @rollup/plugin-typescript, since Vite/TypeScript handles it for you if you use the .ts extension (allowed using the --allowImportingTsExtensions flag in TypeScript v5.0 RC).

Because you've made your PR first, you're welcome to take my commits from #4217 if helps you get the E2E and Cypress tests working.

[remcohaszing]

allowImportingTsExtensions is nice for application code, but it shouldn’t be used for libraries. It emits the .ts extension in the output, both JavaScript and type definitions, which should never be the case. TypeScript shows an error when this option is set in tsconfig.json, but noEmit is missing.

[remcohaszing]

I think the main problem with the e2e tests is that it imports TypeScript source code, which it can’t resolve anymore. The only solution I can some up with, is to test against the built version of mermaid instead of the source code.

[aloisklink]

allowImportingTsExtensions is nice for application code, but it shouldn’t be used for libraries. It emits the .ts extension in the output, both JavaScript and type definitions, which should never be the case.

The output JavaScript is fine, because TypeScript isn't compile/emit those. Instead, Mermaid uses Vite to transpile TypeScript files.

However, you're right that it emits the .ts extension in the .d.ts type definitions. That seems to be a pretty major problem, since it breaks the type definitions for downstream users, unless they switch to TypeScript v5.0 too, and that would be a breaking change.

I'll close my PR!

I think the main problem with the e2e tests is that it imports TypeScript source code, which it can’t resolve anymore. The only solution I can some up with, is to test against the built version of mermaid instead of the source code.

It might even speed up the E2E tests, since loading bundles in browsers (even on localhost) is normally quite a bit faster.

[remcohaszing]

That seems to be a pretty major problem, since it breaks the type definitions for downstream users, unless they switch to TypeScript v5.0 too

Even then it would be a problem. .ts imports are not supposed to be published.

---

@aloisklink Am I correct that you’re not a member of the Mermaid team? It’s nice to get some feedback, but I prefer to get confirmation from the Mermaid team before I start modifying the end-to-end tests.

[aloisklink]

@aloisklink Am I correct that you’re not a member of the Mermaid team? It’s nice to get some feedback, but I prefer to get confirmation from the Mermaid team before I start modifying the end-to-end tests.

I'm a member of the Mermaid team, but I spend more time maintaining the mermaid-js/mermaid-cli project, rather than the core Mermaid project.

If you're planning on making a lot of modifications, it's probably worth getting confirmation from somebody more familiar with the E2E tests/Vite than me, e.g. @sidharthv96.

(@sidharthv96 also has a WIP PR that replaces Vite with ESBuild that may help: #4109)

We may also be able to just wait for vitejs/vite#8993 (comment) to be added to Vite. They're planning on adding .js -> .ts resolving for imports, which might fix the E2E issues you're having, but that comment was a few months ago, so we might be waiting a while.

[sidharthv96]

@aloisklink is indeed one of our very valuable core team members.

---

Many E2E tests already use the compiled bundle via import mermaid from './mermaid.esm.mjs';, so adopting that shouldn't be an issue.

---

@aloisklink, Regarding #4109, we might have to close it as we might be adding UMD support back due to #4148.
Also, I'll be on limited availability for 2 weeks.

[remcohaszing]

@aloisklink is indeed one of our very valuable core team members.

Ah right, sorry, I misread the Collaborator tag as Contributor.

---

An example of a problematic import is

mermaid/cypress/platform/bundle-test.js

Line 1 in 2dd906d

import mermaid from '../../packages/mermaid/src/mermaid';

.
Would it be best to wait for the UMD bundle to be restored then?

[aloisklink]

I misread the Collaborator tag as Contributor.

Sorry! That's actually my fault, I never noticed that I had my organization visibility setting for @mermaid-js on private. It should say (Member) now.

(That might explain why I sometimes see @github staff members being confused that people can't see their (Staff) badge.)

---

An example of a problematic import is

mermaid/cypress/platform/bundle-test.js

Line 1 in 2dd906d

import mermaid from '../../packages/mermaid/src/mermaid';

I think the purpose of bundle-test.js is to test how third-party bundles will handle importing mermaid, so it's probably best to switch that one for import mermaid from './mermaid.core.mjs' since that's what's listed in Mermaid's package.json file:

mermaid/packages/mermaid/package.json

Line 11 in 2dd906d

"import": "./dist/mermaid.core.mjs",

All of the other Cypress test files can be switched to import mermaid from './mermaid.esm.mjs' since all the Cypress tests should support ESM.

(In case you're curious about the differences, ./mermaid.esm.mjs should have all of Mermaid's dependencies (i.e. lodash-es) bundled in, so it can be used directly from a CDN. While mermaid.core.mjs doesn't bundle any dependencies, so that downstream users can bundle their own dependencies)

---

Would it be best to wait for the UMD bundle to be restored then?

My gut feeling is that adding a UMD/IIFE bundle should just be a single option in Vite, so I don't think it will cause any merge conflicts with this PR. But you're welcome to wait if you want to be 100% sure there won't be any issues.

[remcohaszing]

I figured, if you really want to end to end test, you can just use import mermaid from 'mermaid'. That would also ensure the export map works. Unfortunately that didn’t work though, so I used '../../packages/mermaid/dist/mermaid.esm.mjs' everywhere.

There are also some commented imports. I decided not to touch those.

Anyway, tests passed! 😄

---

As a downstream user I’m super happy with the bundled version of Mermaid. It allows me to run Mermaid using Puppeteer / Playwright, and then use the results in Node.js. 😄

[aloisklink]

Looks basically great to me!

I tested with git switch develop && pnpm run e2e && git switch fix-remcohaszing/node16-module-resolution && pnpm run e2e and it looks like all of the E2E test snapshots are the same too.

---

Minor change:, can you switch the imports in .html files from import mermaid from './mermaid.esm.mjs'; instead of import mermaid from '../../packages/mermaid/dist/mermaid.esm.mjs'.

It's mainly just to make things consistent with all of the existing Cypress tests:

mermaid/cypress/platform/class.html

Line 116 in 7647ae3

import mermaid from './mermaid.esm.mjs';

I believe you can do the same thing with import exampleDiagram from '../../packages/mermaid-example-diagram/dist/mermaid-example-diagram.core.mjs';, since it's listed in the Vite server config (see below), but no existing tests do it, so it's up to you:

mermaid/.vite/server.ts

Lines 17 to 18 in 7647ae3

	app.use(express.static('./packages/mermaid/dist'));
	app.use(express.static('./packages/mermaid-example-diagram/dist'));

---

As a downstream user I’m super happy with the bundled version of Mermaid. It allows me to run Mermaid using Puppeteer / Playwright, and then use the results in Node.js.

I think Mermaid v10 made life a bit harder, because now the bundle is ~40 different big files, so I support adding back a single-file bundle UMD/IIFE bundle!

By the way, we've recently added a Node.JS API to the mermaid-cli project for rendering Mermaid diagrams using Puppeteer if you don't want to handle all the bundling yourself, see https://github.com/mermaid-js/mermaid-cli#use-nodejs-api. We still haven't figured out a good way to get fontawesome icons bundled in offline .svg files, so it isn't perfect unfortunately.

[remcohaszing]

I think Mermaid v10 made life a bit harder, because now the bundle is ~40 different big files, so I support adding back a single-file bundle UMD/IIFE bundle!

By the way, we've recently added a Node.JS API to the mermaid-cli project for rendering Mermaid diagrams using Puppeteer if you don't want to handle all the bundling yourself, see https://github.com/mermaid-js/mermaid-cli#use-nodejs-api. We still haven't figured out a good way to get fontawesome icons bundled in offline .svg files, so it isn't perfect unfortunately.

I meant to say I’m happy with the ESM bundle actually. :)

Although using ESM in the browser does cause some CSP issues when using a file:// URL. So I’ll probably switch to the UMD bundle if that becomes available again.

---

I maintain remark-mermaidjs and gatsby-remark-mermaid. I intend to update those to the latest versions of mermaid, and create a rehype plugin, as that is a better place for making such tranformations.

I want to support both the remark and rehype plugin in both Node.js and the browser. In order to support both, I’m going to publish a package mermaid-isomorphic. This will provide an an interface something like this:

interface MermaidRenderer {
  render(diagrams: string[], config: MermaidConfig): SettledPromise<string>[]
}

declare function createMermaidRenderer(): MermaidRenderer;

Such a factory-like function is needed to provide an efficient interface for Node.js, as it will contain logic for starting a browser when necessary, reusing it when still available, tearing it down when it’s unused, and processing multiple diagrams in one call. This logic is not provided by mermaid-cli. I imagine mermaid-cli could use this package under the hood though.

This is getting a bit off-topic for the PR shough. If this is something you are interested to support officially / cooperate on this, please let me know via Twitter DM / email or something. :)

[aloisklink]

This PR looks perfect to me now!

FYI, Sidharth's is busy for the next few weeks, but since they're the most knowledgeable on Vite/TypeScript, this PR probably won't get merged until they've had a look a it.

[sidharthv96]

Do we need this block?

[remcohaszing]

Yes, according to https://vitejs.dev/config/shared-options.html#resolve-extensions, ths allows some extensions by default, which we don’t want.

[remcohaszing]

Is there anything blocking this? The problem is it touches all imports. So the longer we wait, the more merge conflicts will arise.

[sidharthv96]

Hey @remcohaszing, If you can sync this once, I'll take a look and merge.
I didn't find anything major to change.

[remcohaszing]

Done!

[sidharthv96]

Thank you :)