Maintenance: Disable minification in build scripts

[tobiasdiez]

Closes #23970

What I did

Remove minification in build scripts so that the distributed files are human-readable. This makes debugging of storybook issues 1000 times easier. Since the storybook packages are passed through a bundler anyway, minimifaction doesn't provide any real benefit for production.

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

• stories
• unit tests
• integration tests
• end-to-end tests

Manual testing

Run the scripts/prepare/bundle.ts script on some packages and look at the output produced in dist.

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

Documentation

• Add or update documentation reflecting your changes
• If you are deprecating/removing a feature, make sure to update
  MIGRATION.MD

Checklist for Maintainers

• When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli/src/sandbox-templates.ts

• Make sure this PR contains one of the labels below:

  Available labels

  • bug: Internal changes that fixes incorrect behavior.
  • maintenance: User-facing maintenance tasks.
  • dependencies: Upgrading (sometimes downgrading) dependencies.
  • build: Internal-facing build tooling & test updates. Will not show up in release changelog.
  • cleanup: Minor cleanup style change. Will not show up in release changelog.
  • documentation: Documentation only changes. Will not show up in release changelog.
  • feature request: Introducing a new feature.
  • BREAKING CHANGE: Changes that break compatibility in some way with current major version.
  • other: Changes that don't fit in the above categories.

🦋 Canary release

This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

core team members can create a canary release here or locally with gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>

name	before	after	diff	z	%
createSize	0 B	0 B	0 B	-	-
generateSize	76.3 MB	76.3 MB	0 B	1.36	0%
initSize	167 MB	183 MB	15.5 MB	664.67	🔺8.5%
diffSize	91.1 MB	107 MB	15.5 MB	1924.12	🔺14.5%
buildSize	7.42 MB	8.78 MB	1.36 MB	423766.56	🔺15.5%
buildSbAddonsSize	1.61 MB	1.63 MB	19.3 kB	4925	1.2%
buildSbCommonSize	195 kB	195 kB	0 B	-	0%
buildSbManagerSize	2.29 MB	3.46 MB	1.18 MB	88946.97	🔺33.9%
buildSbPreviewSize	351 kB	507 kB	156 kB	54485.88	🔺30.7%
buildStaticSize	0 B	0 B	0 B	-	-
buildPrebuildSize	4.45 MB	5.8 MB	1.35 MB	210061.1	🔺23.3%
buildPreviewSize	2.97 MB	2.98 MB	11.7 kB	3644.35	0.4%
testBuildSize	0 B	0 B	0 B	-	-
testBuildSbAddonsSize	0 B	0 B	0 B	-	-
testBuildSbCommonSize	0 B	0 B	0 B	-	-
testBuildSbManagerSize	0 B	0 B	0 B	-	-
testBuildSbPreviewSize	0 B	0 B	0 B	-	-
testBuildStaticSize	0 B	0 B	0 B	-	-
testBuildPrebuildSize	0 B	0 B	0 B	-	-
testBuildPreviewSize	0 B	0 B	0 B	-	-

name	before	after	diff	z	%
createTime	25.8s	6.6s	-19s -186ms	-1.21	-287.6%
generateTime	20.8s	19.5s	-1s -378ms	-0.21	-7.1%
initTime	19s	16.4s	-2s -579ms	-0.41	-15.7%
buildTime	10.6s	13.4s	2.7s	1.32	🔺20.6%
testBuildTime	0ms	0ms	0ms	-	-
devPreviewResponsive	7.2s	7.6s	390ms	-0.65	5.1%
devManagerResponsive	4.7s	5.1s	382ms	0.07	7.5%
devManagerHeaderVisible	841ms	1s	250ms	2.85	🔺22.9%
devManagerIndexVisible	885ms	1.1s	241ms	2.7	🔺21.4%
devStoryVisibleUncached	1.3s	1.3s	-15ms	0.3	-1.1%
devStoryVisible	886ms	1.1s	242ms	2.59	🔺21.5%
devAutodocsVisible	773ms	977ms	204ms	2.33	🔺20.9%
devMDXVisible	658ms	846ms	188ms	1.1	22.2%
buildManagerHeaderVisible	705ms	913ms	208ms	1.93	🔺22.8%
buildManagerIndexVisible	713ms	915ms	202ms	1.89	🔺22.1%
buildStoryVisible	755ms	990ms	235ms	2.31	🔺23.7%
buildAutodocsVisible	607ms	959ms	352ms	2.95	🔺36.7%
buildMDXVisible	576ms	893ms	317ms	3.44	🔺35.5%

[greptile-apps]

PR Summary

• Disabled minifyIdentifiers and minifySyntax in /code/core/scripts/prep.ts
• Modified getESBuildOptions in /scripts/prepare/addon-bundle.ts to disable minification
• Updated getESBuildOptions in /scripts/prepare/bundle.ts to disable minification

_{3 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings}

[nx-cloud]

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 77d10d5. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution

---

✅ Successfully ran 1 target

• nx run-many -t build --parallel=3

---

Sent with 💌 from NxCloud.

[valentinpalkovic]

@ndelangen Your opinion is requested here.

[ndelangen]

It seems to be increasing the size of build storybook substantially @valentinpalkovic

[tobiasdiez]

I had accidentally also disabled the minification for the manager build. The numbers should be better now. Could you please trigger a new benchmark?

[ndelangen]

It auto triggers, the number do not look too great, they are increasing pretty significantly upwards.

[tobiasdiez]

I tried to reproduce this locally by running task.ts build --template react-vite/default-ts. However, there doesn't seem to be any different in the manager output between this branch and next. In both cases, the sb-manager folder contains mostly un-minimized code.

I would also argue that the distributed size is not super important, depending on the use case. If it's just used as an internal dev tool, then I would gladly have a slightly larger storybook dist then having to uncrypt errors such as

De66xnrA.js:5 [nuxt] error caught during app initialization TypeError: Cannot read properties of undefined (reading 'cdnURL')
    at e (BGOWv8qc.js:1:209)
    at b (BGOWv8qc.js:1:174)
    at mt (D8wL0etK.js:7:6272)
    at A (D8wL0etK.js:7:6458)
    at K (D8wL0etK.js:7:6492)
    at D8wL0etK.js:7:19552
    at r (De66xnrA.js:5:25148)
    at Object.runWithContext (CR8tLLMx.js:13:22037)
    at ze (De66xnrA.js:5:25187)
    at De66xnrA.js:5:23338

If the storybook is intended for a larger audience and has many views, one can activate code minimization of the bundler. The important point is that the end-user can decide which scenario he prefers.

[ndelangen]

@storybookjs/core what do we think?

Should we proceed with this, trade some perf for debug-ability?

[JReinhold]

Personally I'm very much in favor of this trade-off, and I know @IanVS has requested the same thing before.
After having worked with Svelte and SvelteKit which doesn't have any build step at all, I now see the light in how easy unminified code makes it to debug dependencies in node_modules, etc.

I don't know about you all, but multiple times a week I dive into a package in node_modules to understand and temporarily modify the behavior - this would make that a lot easier.

I don't understand why the build sizes are different though, I'd expect a built Storybook to maintain it's current size.

[JReinhold]

Thanks for the investigation @tobiasdiez. We've discussed this in the team and we don't think the timing of this is great. We're currently putting a lot of effort into making Storybook smaller and faster across the board, including minimising dependencies, ESM-only experiments, etc.

Not minifying the package would counter-act that, and we don't think that's the right trade-off right now. I think we should look into this again when we're happy with our size-reduction efforts in a few month, to see how much the difference is by then.

[tobiasdiez]

We're currently putting a lot of effort into making Storybook smaller and faster across the board, including minimising dependencies, ESM-only experiments, etc.

Happy to hear that this is a priority. Especially, ESM-first sounds like a very good idea.

Not minifying the package would counter-act that

I honestly don't agree here. Your efforts in using esm treeshaking and minimizing dependencies would still have a huge impact, regardless of minimizing the code or not.

If you look in the benchmark above you see that the for the user-code buildPreviewSize is (almost) unchanged. The size increase comes from the manager and preview. At least the latter is just copied verbatim

storybook/code/builders/builder-webpack5/src/index.ts

Lines 291 to 304 in 6388f93

	const previewResolvedDir = join(corePath, 'dist/preview');
	const previewDirOrigin = previewResolvedDir;
	const previewDirTarget = join(options.outputDir || '', `sb-preview`);
	const previewFiles = fs.copy(previewDirOrigin, previewDirTarget, {
	filter: (src) => {
	const { ext } = parse(src);
	if (ext) {
	return ext === '.js';
	}
	return true;
	},
	});

If it would pass through the normal build pipeline, then it would also be minified before deploying.

[JReinhold]

I honestly don't agree here. Your efforts in using esm treeshaking and minimizing dependencies would still have a huge impact, regardless of minimizing the code or not.

That's fair, and I can somewhat agree with that. But right now the priority is to get the size down, because it has continuously been a top complaint for years, that Storybook is "bloated". There are many ways to interpret and solve that, one of them being the package download size, that we want to focus on first.

Making it easier to debug Storybook is also a big win, but it only helps a tiny minority (including me) compared to the countless size complaints.

If it would pass through the normal build pipeline, then it would also be minified before deploying.

Yeah if we did end up not shipping minified code, we'd definitely need to minify everything during build, and there's a performance penalty to doing that we'd need to investigate as well. Could be very little though.