chore(mermaid): marked package as free of side-effects for improved tree shaking #5872

GauBen · 2024-09-17T16:33:50Z

📑 Summary

Hey!

I'm using mermaid on a svelte-kit website and the server bundle does include a bare import "mermaid"; because esbuild is not able to determine if this import can be entirely tree-shaken. This package.json directive is common among bundlers (webpack) and will ensure that bare mermaid imports are removed.

📏 Design Decisions

Describe the way your implementation works or what design decisions you made if applicable.

📋 Tasks

Make sure you

📖 have read the contribution guidelines
💻 have added necessary unit/e2e tests.
📓 have added documentation. Make sure MERMAID_RELEASE_VERSION is used for all new features.
🦋 If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

…ree shaking Hey! I'm using mermaid on a svelte-kit website and the server bundle does include a bare `import "mermaid";` because esbuild is not able to determine if this import can be entirely tree-shaken. This package.json directive is common among bundlers ([webpack](https://webpack.js.org/guides/tree-shaking/#mark-the-file-as-side-effect-free)) and will ensure that bare mermaid imports are removed.

changeset-bot · 2024-09-17T16:33:53Z

🦋 Changeset detected

Latest commit: b999978

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
mermaid	Patch

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

netlify · 2024-09-17T16:34:04Z

✅ Deploy Preview for mermaid-js ready!

Name	Link
🔨 Latest commit	`b999978`
🔍 Latest deploy log	https://app.netlify.com/sites/mermaid-js/deploys/66e9b1a07f775d0008c0cb72
😎 Deploy Preview	https://deploy-preview-5872--mermaid-js.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

pkg-pr-new · 2024-09-17T16:36:18Z

Open in Stackblitz

pnpm add https://pkg.pr.new/mermaid-js/mermaid@5872

pnpm add https://pkg.pr.new/mermaid-js/mermaid/@mermaid-js/layout-elk@5872

pnpm add https://pkg.pr.new/mermaid-js/mermaid/@mermaid-js/mermaid-zenuml@5872

pnpm add https://pkg.pr.new/mermaid-js/mermaid/@mermaid-js/parser@5872

commit: b999978

codecov · 2024-09-17T16:37:55Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 5.00%. Comparing base (a07f1f3) to head (b999978).

Additional details and impacted files

@@            Coverage Diff             @@
##           develop   #5872      +/-   ##
==========================================
- Coverage     5.00%   5.00%   -0.01%     
==========================================
  Files          337     338       +1     
  Lines        48209   48220      +11     
  Branches       576     551      -25     
==========================================
  Hits          2413    2413              
- Misses       45796   45807      +11

Flag	Coverage Δ
unit	`5.00% <ø> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 1 file with indirect coverage changes

argos-ci · 2024-09-17T16:46:31Z

The latest updates on your projects. Learn more about Argos notifications ↗︎

Build	Status	Details	Updated (UTC)
default (Inspect)	✅ No changes detected	-	Sep 17, 2024, 4:58 PM

GauBen · 2024-09-17T16:49:43Z

That's a lot of bots! 😂

sidharthv96 · 2024-09-18T09:12:30Z

I remember this was removed recently. Need to look into why.

GauBen · 2024-09-18T09:23:41Z

One reason that might prompt the removal of such a directive is that it will tree-shake bare imports that should have side effects, e.g. import "mermaid/themes/default.css";

I took a quick glance at the files in the package, none of them are css files. If some files are intentionally designed for bare imports, they can be made explicit with "sideEffetcts": ["**/*.css", "ie-fix.js"]

aloisklink

I believe the mermaid package does actually have side-effects for backwards compatibility.

If you run import 'mermaid';, it will render anything in the document with class="mermaid".

See https://mermaid.js.org/config/usage.html#using-mermaid-run

mermaid/packages/mermaid/src/mermaid.ts

Lines 265 to 284 in 571dfda

    
           /** 
        
            * ##contentLoaded Callback function that is called when page is loaded. This functions fetches 
        
            * configuration for mermaid rendering and calls init for rendering the mermaid diagrams on the 
        
            * page. 
        
            */ 
        
           const contentLoaded = function () { 
        
             if (mermaid.startOnLoad) { 
        
               const { startOnLoad } = mermaidAPI.getConfig(); 
        
               if (startOnLoad) { 
        
                 mermaid.run().catch((err) => log.error('Mermaid failed to initialize', err)); 
        
               } 
        
             } 
        
           }; 
        
           if (typeof document !== 'undefined') { 
        
             /*! 
        
              * Wait for document loaded before starting the execution 
        
              */ 
        
             window.addEventListener('load', contentLoaded, false); 
        
           }

Maybe we could do something like:

// TODO: replace `./src/mermaid.ts` with the actual generated file in `./dist`
"sideEffects": ["./src/mermaid.ts"]

I don't think anywhere else in mermaid has side-effects, other than that above line of code.

GauBen · 2024-09-19T12:48:13Z

I believe the mermaid package does actually have side-effects for backwards compatibility.

If you run import 'mermaid';, it will render anything in the document with class="mermaid".

See mermaid.js.org/config/usage.html#using-mermaid-run

mermaid/packages/mermaid/src/mermaid.ts

Lines 265 to 284 in 571dfda

/**

* ##contentLoaded Callback function that is called when page is loaded. This functions fetches

* configuration for mermaid rendering and calls init for rendering the mermaid diagrams on the

* page.

*/

const contentLoaded = function () {

if (mermaid.startOnLoad) {

const { startOnLoad } = mermaidAPI.getConfig();

if (startOnLoad) {

mermaid.run().catch((err) => log.error('Mermaid failed to initialize', err));

}

}

};

if (typeof document !== 'undefined') {

/*!

* Wait for document loaded before starting the execution

*/

window.addEventListener('load', contentLoaded, false);

}

Maybe we could do something like:
// TODO: replace `./src/mermaid.ts` with the actual generated file in `./dist`
"sideEffects": ["./src/mermaid.ts"]
I don't think anywhere else in mermaid has side-effects, other than that above line of code.

Oooh I see, I missed this one, I thought since the full ESM rewrite mermaid was free of side effects.

I'm not sure if this the direction that mermaid intends to take, but I would recommend making it free of side effects for its default export, and have an alternate entry point to run on page load:

{ // package.json
  exports: {
    ".": "./dist/core.mjs",
    "./auto": "./dist/auto.mjs"
  }
}

and

// auto.mjs
import mermaid from './core.mjs';

const contentLoaded = function () {
  if (mermaid.startOnLoad) {
    const { startOnLoad } = mermaidAPI.getConfig();
    if (startOnLoad) {
      mermaid.run().catch((err) => log.error('Mermaid failed to initialize', err));
    }
  }
};

window.addEventListener('load', contentLoaded, false);

I have seen this pattern a few times in the past, it's great because it allows for both CDN and bundler usage to the best of their capabilities.

If you like this approach, I'd love to work on it with you!

aloisklink · 2024-09-19T15:38:29Z

Oooh I see, I missed this one, I thought since the full ESM rewrite mermaid was free of side effects.

😢 I wish we thought to change this then! It would have been a good time to do it. We could have even kept the old IIFE version still using side-effects, while only the new ESM version didn't have side-effects.

I'm not sure if this the direction that mermaid intends to take, but I would recommend making it free of side effects for its default export, and have an alternate entry point to run on page load: [...]

I like the idea, but for backwards compatibility, I think we may have to do it the other way around. E.g. the default import 'mermaid' will still have side-effects, but users could do an import mermaid from 'mermaid/name-to-still-be-determined' if they don't want to have side-effects.

I did a quick search of 'https://cdn.jsdelivr.net/npm/mermaid/+esm' on GitHub Code Search (which will also resolve to the latest version, since they're not using mermaid@version), and it looks like 384 files are using the default ESM import, without being pinned to any version of mermaid.

But then again, there are 70k files using https://cdn.jsdelivr.net/npm/mermaid/dist, so maybe it's okay to remove side-effects from the default import in a new major release (e.g. Mermaid v12), as long as we make sure all the current entry-points in the dist/mermaid.* folder still have side-effects?

@sidharthv96, your thoughts?

GauBen · 2024-09-19T16:09:37Z

I like the idea, but for backwards compatibility, I think we may have to do it the other way around. E.g. the default import 'mermaid' will still have side-effects, but users could do an import mermaid from 'mermaid/name-to-still-be-determined' if they don't want to have side-effects.

I like this option, it does not even require a major version

Maybe we could go even further in this tree-shaking-ability rabbit hole by exposing diagrams separately?

import { createMermaid } from 'mermaid/tree-shakable';
import flowChart from 'mermaid/tree-shakable/flowChart';

const mermaid = createMermaid({ diagrams: [flowChart] });
mermaid.run({ nodes: ... });

I know that mermaid has code splitting, it works to reduce the page weight for diagrams that are not needed, but the resulting bundle still includes all possible diagrams.

aloisklink · 2024-09-23T14:03:24Z

Maybe we could go even further in this tree-shaking-ability rabbit hole by exposing diagrams separately?

import { createMermaid } from 'mermaid/tree-shakable';
import flowChart from 'mermaid/tree-shakable/flowChart';

const mermaid = createMermaid({ diagrams: [flowChart] });
mermaid.run({ nodes: ... });

We've had similar discussions in #4120 and #4616 about splitting up mermaid into multiple sub-packages.

Then, we'd have something like:

import { createMermaid } from '@mermaid-js/mermaid';
import flowChart from '@mermaid-js/flowchart';
 
const mermaid = createMermaid({ diagrams: [flowChart] });
mermaid.run({ nodes: ... });

Then, the default mermaid package would still have all diagram types for backwards compatibility, but anybody that wants smaller packages/more control would be able to manually install the smaller versions (and it would npm installs much faster for them too, since right now the main mermaid package is 60 MB!!)

GauBen · 2024-09-23T14:31:18Z

Then, we'd have something like:

import { createMermaid } from '@mermaid-js/mermaid';
import flowChart from '@mermaid-js/flowchart';

If that's a direction you're willing to take, I'd be really happy to contribute!

Plus it will make community-developed diagrams feel more integrated, which is always good for open source projects

Create khaki-needles-hope.md

c924a6f

[autofix.ci] apply automated fixes

b999978

aloisklink requested changes Sep 19, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(mermaid): marked package as free of side-effects for improved tree shaking #5872

chore(mermaid): marked package as free of side-effects for improved tree shaking #5872

GauBen commented Sep 17, 2024

changeset-bot bot commented Sep 17, 2024 •

edited

Loading

netlify bot commented Sep 17, 2024 •

edited

Loading

pkg-pr-new bot commented Sep 17, 2024 •

edited

Loading

codecov bot commented Sep 17, 2024 •

edited

Loading

argos-ci bot commented Sep 17, 2024 •

edited

Loading

GauBen commented Sep 17, 2024

sidharthv96 commented Sep 18, 2024

GauBen commented Sep 18, 2024

aloisklink left a comment •

edited

Loading

GauBen commented Sep 19, 2024 •

edited

Loading

aloisklink commented Sep 19, 2024

GauBen commented Sep 19, 2024 •

edited

Loading

aloisklink commented Sep 23, 2024

GauBen commented Sep 23, 2024

	/**
	* ##contentLoaded Callback function that is called when page is loaded. This functions fetches
	* configuration for mermaid rendering and calls init for rendering the mermaid diagrams on the
	* page.
	*/
	const contentLoaded = function () {
	if (mermaid.startOnLoad) {
	const { startOnLoad } = mermaidAPI.getConfig();
	if (startOnLoad) {
	mermaid.run().catch((err) => log.error('Mermaid failed to initialize', err));
	}
	}
	};

	if (typeof document !== 'undefined') {
	/*!
	* Wait for document loaded before starting the execution
	*/
	window.addEventListener('load', contentLoaded, false);
	}

chore(mermaid): marked package as free of side-effects for improved tree shaking #5872

Are you sure you want to change the base?

chore(mermaid): marked package as free of side-effects for improved tree shaking #5872

Conversation

GauBen commented Sep 17, 2024

📑 Summary

📏 Design Decisions

📋 Tasks

changeset-bot bot commented Sep 17, 2024 • edited Loading

🦋 Changeset detected

netlify bot commented Sep 17, 2024 • edited Loading

✅ Deploy Preview for mermaid-js ready!

pkg-pr-new bot commented Sep 17, 2024 • edited Loading

codecov bot commented Sep 17, 2024 • edited Loading

Codecov Report

argos-ci bot commented Sep 17, 2024 • edited Loading

GauBen commented Sep 17, 2024

sidharthv96 commented Sep 18, 2024

GauBen commented Sep 18, 2024

aloisklink left a comment • edited Loading

Choose a reason for hiding this comment

GauBen commented Sep 19, 2024 • edited Loading

aloisklink commented Sep 19, 2024

GauBen commented Sep 19, 2024 • edited Loading

aloisklink commented Sep 23, 2024

GauBen commented Sep 23, 2024

changeset-bot bot commented Sep 17, 2024 •

edited

Loading

netlify bot commented Sep 17, 2024 •

edited

Loading

pkg-pr-new bot commented Sep 17, 2024 •

edited

Loading

codecov bot commented Sep 17, 2024 •

edited

Loading

argos-ci bot commented Sep 17, 2024 •

edited

Loading

aloisklink left a comment •

edited

Loading

GauBen commented Sep 19, 2024 •

edited

Loading

GauBen commented Sep 19, 2024 •

edited

Loading