Vite: Import preview runtime as ordinary module #29172

tobiasdiez · 2024-09-20T14:41:23Z

What I did

Currently, the preview runtime is manually exposed in the builder (in dev mode via express.static and in build mode via maual copying).

This has several problems:

Makes the whole setup complex and thus suceptable to breaking (as demonstrated by Basic Usage example from documentation not working: preview.js not found nuxt-modules/storybook#777 where the preview runtime cannot be found for some reason in dev mode)
The preview runtime has to be shipped in a minified version because it would end up on the server in exactly this version
But it may still contain too much code since not all if it may be used in a given storybook.

This PR improves the situation by just importing the preview runtime as an ordinary modue (for the vite builder). So it goes through the vite pipeline just as every other module. As a nice side-effect, express is now no longer needed for the vite builder.

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

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	77.7 MB	77.7 MB	0 B	1.32	0%
initSize	130 MB	130 MB	-33.2 kB	0.59	0%
diffSize	52.5 MB	52.4 MB	-33.2 kB	0.45	-0.1%
buildSize	6.83 MB	6.75 MB	-82.2 kB	-685.76	-1.2%
buildSbAddonsSize	1.51 MB	1.51 MB	0 B	-0.23	0%
buildSbCommonSize	195 kB	195 kB	0 B	-	0%
buildSbManagerSize	1.86 MB	1.86 MB	0 B	0.03	0%
buildSbPreviewSize	271 kB	0 B	-271 kB	-54020.65	🔰-Infinity%
buildStaticSize	0 B	0 B	0 B	-	-
buildPrebuildSize	3.84 MB	3.56 MB	-271 kB	-2259.5	🔰-7.6%
buildPreviewSize	3 MB	3.19 MB	189 kB	61805.81	🔺5.9%
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	7.2s	8.1s	886ms	-0.82	10.8%
generateTime	20s	23.3s	3.3s	0.57	14.2%
initTime	12.7s	15.5s	2.8s	0.56	18.2%
buildTime	7.9s	8.8s	881ms	-0.19	10%
testBuildTime	0ms	0ms	0ms	-	-
devPreviewResponsive	5.1s	4.7s	-395ms	-0.35	-8.3%
devManagerResponsive	3.6s	3.6s	-13ms	0.2	-0.4%
devManagerHeaderVisible	633ms	561ms	-72ms	-0.3	-12.8%
devManagerIndexVisible	693ms	588ms	-105ms	-0.8	-17.9%
devStoryVisibleUncached	1.7s	1.9s	188ms	1.07	9.5%
devStoryVisible	692ms	587ms	-105ms	-0.59	-17.9%
devAutodocsVisible	561ms	635ms	74ms	1.23	11.7%
devMDXVisible	511ms	616ms	105ms	0.95	17%
buildManagerHeaderVisible	520ms	601ms	81ms	0.18	13.5%
buildManagerIndexVisible	532ms	674ms	142ms	0.57	21.1%
buildStoryVisible	514ms	552ms	38ms	-0.22	6.9%
buildAutodocsVisible	458ms	459ms	1ms	-0.21	0.2%
buildMDXVisible	425ms	460ms	35ms	-0.12	7.6%

Greptile Summary

This pull request simplifies the handling of the preview runtime in the Vite builder for Storybook. Key changes include:

Imports preview runtime as an ordinary module instead of manual exposure
Removes 'express' dependency from '@storybook/builder-vite' package
Modifies Vite build configuration to remove './sb-preview/runtime.js' from external rollup options
Updates HMR handling for better Vite compatibility
Removes manual handling of preview runtime via express.static and file copying
Adds new exports for preview runtime and manager globals runtime in core package
Updates Vue component meta plugin to modify file exclusion patterns

These changes aim to streamline the setup, reduce complexity, and improve integration with Vite's build process.

greptile-apps

LGTM

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

nx-cloud · 2024-09-20T14:58:12Z

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 558d62d. 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.

ndelangen · 2024-10-07T07:52:40Z

I suspect we likely won't want to do this for 2 reasons:

The first being:

buildTime	8.9s	11.3s	2.3s	1.36	🔺21.1%

We prebundle, to minimize build-time, and optimize start-up time.
We know this causes a bit of a size increase, and it's likely not the most intuitive.. but in this case, we deem the trade-off worth it.

And the second being how this would make the vite builder work very differently from how other builders work, which will cause unexpected maintenance problems.

tobiasdiez · 2024-10-07T08:24:27Z

I suspect we likely won't want to do this for 2 reasons:

The first being:
buildTime 8.9s 11.3s 2.3s 1.36 🔺21.1%

Did you confirm this locally? The benchmark results are varying very much for my PRs, so I don't know how reliable these numbers are. My expectation would be that there is indeed a very small increase in the cold startup time (because the very first time vite has to process one more file) but afterwards there is a small improvement in the warm startup time (because vite can optimize the import, cache it etc).

And the second being how this would make the vite builder work very differently from how other builders work, which will cause unexpected maintenance problems.

Webpack would very much profit from the same optimization. I just don't have the knowledge to do this.

From the perspective of a downstream maintainer, this setup is a major headache and definily a source of "unexpected maintenance problems". For nuxt we essentially have 3 dev server running (nuxt, vite, express), all needing to be synchronized in subtitle ways to make the setup work. You might have a look at nuxt-modules/storybook#777 to get a feeling for these problems.

kasperpeulen · 2024-11-22T16:46:57Z

@tobiasdiez Will look next week what the real perf impact is of this PR.
I kind of like it. I think that (some) sourcemaps are also broken because of this prebundling hack.

Could you maybe describe explicitly the problem you are facing?

where the preview runtime cannot be found for some reason in dev mode
This sounds a bit vague.

tobiasdiez · 2024-11-23T13:21:59Z

Thanks @kasperpeulen for looking into this.

If I remember correctly, the problem was a bit vague in the first problem. The runtime.js/preview.jso or files loaded by it / related files were simply reported as missing or that they couldn't be loaded. Happened only in dev mode. I never figured out why it occurred, it appeared to me that vite had some troubles analyzing / transpiling all files. But maybe it was also simply an undesired effect of the various hacks we have in place (https://github.com/nuxt-modules/storybook/blob/736d46c89c978b872e4cec027d64b1a044589e83/packages/storybook-addon/src/preset.ts#L373 and https://github.com/nuxt-modules/storybook/blob/736d46c89c978b872e4cec027d64b1a044589e83/packages/storybook-addon/src/preset.ts#L299 being candidates).

I'm not even totally sure the issue is fixed with this PR but in either case it should make the setup simpler and more reliable.

…preview # Conflicts: # code/builders/builder-vite/package.json # code/builders/builder-vite/src/index.ts

code/core/scripts/prep.ts

code/lib/cli/package.json

storybook-pr-benchmarking · 2024-11-26T16:25:38Z

Package Benchmarks

^{Commit: 558d62d, ran on 3 December 2024 at 14:20:08 UTC}

The following packages have significant changes to their size or dependencies:

`@storybook/builder-vite`

	Before	After	Difference
Dependency count	6	6	0
Self size	468 KB	434 KB	🎉 -33 KB 🎉
Dependency size	832 KB	832 KB	0 B
Bundle Size Analyzer	Link	Link

`@storybook/experimental-nextjs-vite`

	Before	After	Difference
Dependency count	87	87	0
Self size	231 KB	231 KB	0 B
Dependency size	31.32 MB	31.28 MB	🎉 -33 KB 🎉
Bundle Size Analyzer	Link	Link

`@storybook/html-vite`

	Before	After	Difference
Dependency count	15	15	0
Self size	5 KB	5 KB	0 B
Dependency size	1.94 MB	1.90 MB	🎉 -33 KB 🎉
Bundle Size Analyzer	Link	Link

`@storybook/preact-vite`

	Before	After	Difference
Dependency count	13	13	0
Self size	6 KB	6 KB	0 B
Dependency size	1.34 MB	1.30 MB	🎉 -33 KB 🎉
Bundle Size Analyzer	Link	Link

`@storybook/react-native-web-vite`

	Before	After	Difference
Dependency count	102	102	0
Self size	41 KB	41 KB	0 B
Dependency size	17.88 MB	17.85 MB	🎉 -33 KB 🎉
Bundle Size Analyzer	Link	Link

`@storybook/react-vite`

	Before	After	Difference
Dependency count	87	87	0
Self size	12 KB	12 KB	0 B
Dependency size	16.03 MB	15.99 MB	🎉 -33 KB 🎉
Bundle Size Analyzer	Link	Link

`@storybook/svelte-vite`

	Before	After	Difference
Dependency count	134	134	0
Self size	22 KB	22 KB	0 B
Dependency size	36.40 MB	36.36 MB	🎉 -33 KB 🎉
Bundle Size Analyzer	Link	Link

`@storybook/sveltekit`

	Before	After	Difference
Dependency count	142	142	0
Self size	47 KB	47 KB	0 B
Dependency size	39.70 MB	39.66 MB	🎉 -33 KB 🎉
Bundle Size Analyzer	Link	Link

`@storybook/vue3-vite`

	Before	After	Difference
Dependency count	107	107	0
Self size	16 KB	16 KB	🎉 -16 B 🎉
Dependency size	42.57 MB	42.54 MB	🎉 -33 KB 🎉
Bundle Size Analyzer	Link	Link

`@storybook/web-components-vite`

	Before	After	Difference
Dependency count	16	16	0
Self size	5 KB	5 KB	0 B
Dependency size	1.97 MB	1.94 MB	🎉 -33 KB 🎉
Bundle Size Analyzer	Link	Link

kasperpeulen · 2024-12-03T12:29:21Z

code/core/src/preview/runtime.ts

+}
+
+// TODO: In the future, remove this call to make the module side-effect free
+setup();


Are you not calling this function twice now? Can we not just remove it, now we call it explicitly in codegen-modern-iframe-script.ts?

I see, we can not remove it because of webpack.

kasperpeulen

LGTM, one comment

kasperpeulen · 2024-12-03T13:28:33Z

code/core/src/preview/runtime.ts

+  global.addEventListener('error', (args: any) => {
+    const error = args.error || args;
+    if (error.fromStorybook) {
+      global.sendTelemetryError(error);
+    }
+  });
+  global.addEventListener('unhandledrejection', ({ reason }: any) => {
+    if (reason.fromStorybook) {
+      global.sendTelemetryError(reason);
+    }
+  });
+}


Especially those 2 seem problematic, when called twice.

Made sure we only call the listeners once

tobiasdiez · 2024-12-05T09:17:36Z

@kasperpeulen Thanks for the review!

Build(Vite): Import preview runtime as ordinary module

511a557

greptile-apps bot reviewed Sep 20, 2024

View reviewed changes

update lock file

25e9b55

tobiasdiez added the ci:normal label Sep 20, 2024

tobiasdiez added 4 commits September 20, 2024 23:51

fix build

290ff6a

fix build

25abe01

improve preview import

3e72c2f

Make sure runtime preview is executed

643002d

kasperpeulen added the bug label Sep 23, 2024

kasperpeulen assigned ndelangen Sep 23, 2024

Merge branch 'next' into vite-preview

9adfb41

tobiasdiez mentioned this pull request Oct 13, 2024

CLI: Re-Add Nuxt support #28607

Merged

8 tasks

tobiasdiez mentioned this pull request Nov 20, 2024

State of the project nuxt-modules/storybook#813

Open

vanessayuenn assigned kasperpeulen Nov 22, 2024

Merge remote-tracking branch 'origin/next' into fork/tobiasdiez/vite-…

f17152a

…preview # Conflicts: # code/builders/builder-vite/package.json # code/builders/builder-vite/src/index.ts

kasperpeulen reviewed Nov 26, 2024

View reviewed changes

code/core/scripts/prep.ts Show resolved Hide resolved

kasperpeulen reviewed Nov 26, 2024

View reviewed changes

code/lib/cli/package.json Show resolved Hide resolved

kasperpeulen reviewed Dec 3, 2024

View reviewed changes

kasperpeulen approved these changes Dec 3, 2024

View reviewed changes

kasperpeulen reviewed Dec 3, 2024

View reviewed changes

kasperpeulen added 3 commits December 3, 2024 14:47

Make listeners unique so that they are not fired twice

8664dcd

Expand TODO

124b196

Merge branch 'next' into vite-preview

558d62d

kasperpeulen merged commit 5414a86 into storybookjs:next Dec 3, 2024
56 checks passed

github-actions bot mentioned this pull request Dec 3, 2024

Release: Prerelease 8.5.0-alpha.16 #29789

Merged

6 tasks

shilman changed the title ~~Build(Vite): Import preview runtime as ordinary module~~ Vite: Import preview runtime as ordinary module Dec 3, 2024

JReinhold mentioned this pull request Dec 4, 2024

[Bug]: In Next project with the experimental-nextjs-vite we need to install @storybook/core in the user's package #29798

Closed

tobiasdiez deleted the vite-preview branch December 5, 2024 09:16

yannbf mentioned this pull request Dec 20, 2024

[Bug] revisit dependency optimization behavior #29812

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Vite: Import preview runtime as ordinary module #29172

Vite: Import preview runtime as ordinary module #29172

tobiasdiez commented Sep 20, 2024 •

edited by storybook-pr-benchmarking bot

Loading

greptile-apps bot left a comment

nx-cloud bot commented Sep 20, 2024 •

edited

Loading

ndelangen commented Oct 7, 2024

tobiasdiez commented Oct 7, 2024 •

edited

Loading

kasperpeulen commented Nov 22, 2024

tobiasdiez commented Nov 23, 2024

storybook-pr-benchmarking bot commented Nov 26, 2024 •

edited

Loading

kasperpeulen Dec 3, 2024

kasperpeulen Dec 3, 2024

kasperpeulen left a comment

kasperpeulen Dec 3, 2024

kasperpeulen Dec 3, 2024

tobiasdiez commented Dec 5, 2024

Vite: Import preview runtime as ordinary module #29172

Vite: Import preview runtime as ordinary module #29172

Conversation

tobiasdiez commented Sep 20, 2024 • edited by storybook-pr-benchmarking bot Loading

What I did

Checklist for Contributors

Testing

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

Manual testing

Documentation

Checklist for Maintainers

🦋 Canary release

Greptile Summary

greptile-apps bot left a comment

Choose a reason for hiding this comment

nx-cloud bot commented Sep 20, 2024 • edited Loading

☁️ Nx Cloud Report

ndelangen commented Oct 7, 2024

tobiasdiez commented Oct 7, 2024 • edited Loading

kasperpeulen commented Nov 22, 2024

tobiasdiez commented Nov 23, 2024

storybook-pr-benchmarking bot commented Nov 26, 2024 • edited Loading

Package Benchmarks

@storybook/builder-vite

@storybook/experimental-nextjs-vite

@storybook/html-vite

@storybook/preact-vite

@storybook/react-native-web-vite

@storybook/react-vite

@storybook/svelte-vite

@storybook/sveltekit

@storybook/vue3-vite

@storybook/web-components-vite

kasperpeulen Dec 3, 2024

Choose a reason for hiding this comment

kasperpeulen Dec 3, 2024

Choose a reason for hiding this comment

kasperpeulen left a comment

Choose a reason for hiding this comment

kasperpeulen Dec 3, 2024

Choose a reason for hiding this comment

kasperpeulen Dec 3, 2024

Choose a reason for hiding this comment

tobiasdiez commented Dec 5, 2024

tobiasdiez commented Sep 20, 2024 •

edited by storybook-pr-benchmarking bot

Loading

nx-cloud bot commented Sep 20, 2024 •

edited

Loading

tobiasdiez commented Oct 7, 2024 •

edited

Loading

storybook-pr-benchmarking bot commented Nov 26, 2024 •

edited

Loading

`@storybook/builder-vite`

`@storybook/experimental-nextjs-vite`

`@storybook/html-vite`

`@storybook/preact-vite`

`@storybook/react-native-web-vite`

`@storybook/react-vite`

`@storybook/svelte-vite`

`@storybook/sveltekit`

`@storybook/vue3-vite`

`@storybook/web-components-vite`