Skip to content
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

Vite: Import preview runtime as ordinary module #29172

Merged
merged 11 commits into from
Dec 3, 2024
1 change: 0 additions & 1 deletion code/builders/builder-vite/input/iframe.html
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@
<!-- [BODY HTML SNIPPET HERE] -->
<div id="storybook-root"></div>
<div id="storybook-docs"></div>
<script type="module" src="./sb-preview/runtime.js"></script>
<script type="module" src="/virtual:/@storybook/builder-vite/vite-app.js"></script>
</body>
</html>
6 changes: 1 addition & 5 deletions code/builders/builder-vite/src/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,7 @@ export async function build(options: Options) {
outDir: options.outputDir,
emptyOutDir: false, // do not clean before running Vite build - Storybook has already added assets in there!
rollupOptions: {
external: [
// Do not try to bundle the Storybook runtime, it is copied into the output dir after the build.
'./sb-preview/runtime.js',
/\.\/sb-common-assets\/.*\.woff2/,
],
external: [/\.\/sb-common-assets\/.*\.woff2/],
},
...(options.test
? {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,15 @@ export async function generateModernIframeScriptCode(options: Options, projectRo
* @todo Inline variable and remove `noinspection`
*/
const code = `
import { composeConfigs, PreviewWeb, ClientApi } from 'storybook/internal/preview-api';
import { setup } from '@storybook/core/preview/runtime';
import '${SB_VIRTUAL_FILES.VIRTUAL_ADDON_SETUP_FILE}';
setup();
import { composeConfigs, PreviewWeb, ClientApi } from 'storybook/internal/preview-api';
import { importFn } from '${SB_VIRTUAL_FILES.VIRTUAL_STORIES_FILE}';
${getPreviewAnnotationsFunction}
window.__STORYBOOK_PREVIEW__ = window.__STORYBOOK_PREVIEW__ || new PreviewWeb(importFn, getProjectAnnotations);
Expand Down
31 changes: 2 additions & 29 deletions code/builders/builder-vite/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// noinspection JSUnusedGlobalSymbols
import { cp, readFile } from 'node:fs/promises';
import { join, parse } from 'node:path';
import { readFile } from 'node:fs/promises';

import { NoStatsForViteDevError } from 'storybook/internal/server-errors';
import type { Middleware, Options } from 'storybook/internal/types';
Expand Down Expand Up @@ -61,15 +60,6 @@ export const start: ViteBuilder['start'] = async ({
}) => {
server = await createViteServer(options as Options, devServer);

const previewResolvedDir = join(corePath, 'dist/preview');
router.use(
'/sb-preview',
sirv(previewResolvedDir, {
maxAge: 300000,
dev: true,
immutable: true,
})
);
router.use(iframeMiddleware(options as Options, server));
router.use(server.middlewares);

Expand All @@ -85,22 +75,5 @@ export const start: ViteBuilder['start'] = async ({
};

export const build: ViteBuilder['build'] = async ({ options }) => {
const viteCompilation = viteBuild(options as Options);

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

const [out] = await Promise.all([viteCompilation, previewFiles]);

return out;
return viteBuild(options as Options);
};
2 changes: 0 additions & 2 deletions code/builders/builder-vite/src/vite-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ export async function createViteServer(options: Options, devServer: Server) {

const config = {
...commonCfg,
// Needed in Vite 5: https://github.com/storybookjs/storybook/issues/25256
assetsInclude: getAssetsInclude(commonCfg, ['/sb-preview/**']),
// Set up dev server
server: {
middlewareMode: true,
Expand Down
15 changes: 6 additions & 9 deletions code/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,12 @@
"import": "./dist/cli/bin/index.js",
"require": "./dist/cli/bin/index.cjs"
},
"./preview/runtime": {
"import": "./dist/preview/runtime.js"
},
"./manager/globals-runtime": {
"import": "./dist/manager/globals-runtime.js"
},
"./package.json": "./package.json"
},
"main": "dist/index.cjs",
Expand All @@ -192,15 +198,6 @@
"core-server": [
"./dist/core-server/index.d.ts"
],
"core-server/presets/common-preset": [
"./dist/core-server/presets/common-preset.d.ts"
],
"core-server/presets/common-manager": [
"./dist/core-server/presets/common-manager.d.ts"
],
"core-server/presets/common-override-preset": [
"./dist/core-server/presets/common-override-preset.d.ts"
],
"core-events": [
"./dist/core-events/index.d.ts"
],
Expand Down
4 changes: 4 additions & 0 deletions code/core/scripts/helpers/generatePackageJsonFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ export async function generatePackageJsonFile(entries: ReturnType<typeof getEntr
'*': {
'*': ['./dist/index.d.ts'],
...entries.reduce<Record<string, string[]>>((acc, entry) => {
if (!entry.dts) {
return acc;
}

let main = slash(relative(cwd, entry.file).replace('src', 'dist'));
if (main === './dist/index.ts' || main === './dist/index.tsx') {
main = '.';
Expand Down
2 changes: 1 addition & 1 deletion code/core/scripts/prep.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ async function run() {
console.log(isWatch ? 'Watching...' : 'Bundling...');

const files = measure(generateSourceFiles);
const packageJson = measure(() => generatePackageJsonFile(entries));
const packageJson = measure(() => generatePackageJsonFile(entries.concat(bundles)));
kasperpeulen marked this conversation as resolved.
Show resolved Hide resolved
const dist = files.then(() => measure(generateDistFiles));
const types = files.then(() =>
measure(async () => {
Expand Down
45 changes: 25 additions & 20 deletions code/core/src/preview/runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,30 @@ import { globalPackages, globalsNameReferenceMap } from './globals/globals';
import { globalsNameValueMap } from './globals/runtime';
import { prepareForTelemetry } from './utils';

// Apply all the globals
globalPackages.forEach((key) => {
(global as any)[globalsNameReferenceMap[key]] = globalsNameValueMap[key];
});
export function setup() {
// Apply all the globals
globalPackages.forEach((key) => {
(global as any)[globalsNameReferenceMap[key]] = globalsNameValueMap[key];
});

global.sendTelemetryError = (error: any) => {
const channel = global.__STORYBOOK_ADDONS_CHANNEL__;
channel.emit(TELEMETRY_ERROR, prepareForTelemetry(error));
};
global.sendTelemetryError = (error: any) => {
const channel = global.__STORYBOOK_ADDONS_CHANNEL__;
channel.emit(TELEMETRY_ERROR, prepareForTelemetry(error));
};

// handle all uncaught StorybookError at the root of the application and log to telemetry if applicable
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);
}
});
// handle all uncaught StorybookError at the root of the application and log to telemetry if applicable
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);
}
});
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Especially those 2 seem problematic, when called twice.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made sure we only call the listeners once


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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

3 changes: 1 addition & 2 deletions code/frameworks/vue3-vite/src/plugins/vue-component-meta.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@ export async function vueComponentMeta(tsconfigPath = 'tsconfig.json'): Promise<
const { createFilter } = await import('vite');

// exclude stories, virtual modules and storybook internals
const exclude =
/\.stories\.(ts|tsx|js|jsx)$|^\0\/virtual:|^\/virtual:|^\/sb-preview\/|\.storybook\/.*\.(ts|js)$/;
const exclude = /\.stories\.(ts|tsx|js|jsx)$|^\0\/virtual:|^\/virtual:|\.storybook\/.*\.(ts|js)$/;
const include = /\.(vue|ts|js|tsx|jsx)$/;
const filter = createFilter(include, exclude);

Expand Down
1 change: 1 addition & 0 deletions code/lib/cli/core/manager/globals-runtime.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from '@storybook/core/manager/globals-runtime';
1 change: 1 addition & 0 deletions code/lib/cli/core/preview/runtime.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from '@storybook/core/preview/runtime';
15 changes: 6 additions & 9 deletions code/lib/cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,12 @@
"types": "./core/babel/index.d.ts",
"import": "./core/babel/index.js",
"require": "./core/babel/index.cjs"
},
"./internal/manager/globals-runtime": {
"import": "./core/manager/globals-runtime.js"
},
kasperpeulen marked this conversation as resolved.
Show resolved Hide resolved
"./internal/preview/runtime": {
"import": "./core/preview/runtime.js"
}
},
"main": "dist/index.cjs",
Expand Down Expand Up @@ -227,15 +233,6 @@
"internal/core-server": [
"./core/core-server/index.d.ts"
],
"internal/core-server/presets/common-manager": [
"./core/core-server/presets/common-manager.d.ts"
],
"internal/core-server/presets/common-override-preset": [
"./core/core-server/presets/common-override-preset.d.ts"
],
"internal/core-server/presets/common-preset": [
"./core/core-server/presets/common-preset.d.ts"
],
"internal/csf-tools": [
"./core/csf-tools/index.d.ts"
],
Expand Down
Loading