From 19251823fc670acc4b7025807cd27ab565a2a960 Mon Sep 17 00:00:00 2001 From: Norbert de Langen Date: Thu, 1 Dec 2022 18:33:25 +0100 Subject: [PATCH 1/4] some fixes, need more work --- .../builder-vite/src/codegen-iframe-script.ts | 19 ++++++++++--------- code/lib/instrumenter/src/instrumenter.ts | 2 +- .../src/modules/core-client/start.ts | 2 +- .../src/components/preview/preview.tsx | 7 +++++-- 4 files changed, 17 insertions(+), 13 deletions(-) diff --git a/code/lib/builder-vite/src/codegen-iframe-script.ts b/code/lib/builder-vite/src/codegen-iframe-script.ts index 8f986e9ca26..9cae4e246e6 100644 --- a/code/lib/builder-vite/src/codegen-iframe-script.ts +++ b/code/lib/builder-vite/src/codegen-iframe-script.ts @@ -29,14 +29,9 @@ export async function generateIframeScriptCode(options: ExtendedOptions) { // is loaded. That way our client-apis can assume the existence of the API+store import { configure } from '${rendererName}'; + import { logger } from '@storybook/client-logger'; - import * as clientApi from "@storybook/preview-api"; - ${filesToImport(configEntries, 'config')} - - import * as preview from '${virtualPreviewFile}'; - import { configStories } from '${virtualStoriesFile}'; - - const { + import { addDecorator, addParameters, addLoader, @@ -46,8 +41,14 @@ export async function generateIframeScriptCode(options: ExtendedOptions) { addArgTypesEnhancer, addArgsEnhancer, setGlobalRender, - } = previewApi; - + } from "@storybook/preview-api"; + ${filesToImport(configEntries, 'config')} + + import * as preview from '${virtualPreviewFile}'; + import { configStories } from '${virtualStoriesFile}'; + + console.log('${rendererName}'); + const configs = [${importArray('config', configEntries.length) .concat('preview.default') .join(',')}].filter(Boolean) diff --git a/code/lib/instrumenter/src/instrumenter.ts b/code/lib/instrumenter/src/instrumenter.ts index c29b1a6fbd9..69d4b4eb2a9 100644 --- a/code/lib/instrumenter/src/instrumenter.ts +++ b/code/lib/instrumenter/src/instrumenter.ts @@ -334,7 +334,7 @@ export class Instrumenter { track(method: string, fn: Function, args: any[], options: Options) { const storyId: StoryId = args?.[0]?.__storyId__ || - global.window.__STORYBOOK_PREVIEW__.selectionStore.selection.storyId; + global.window.__STORYBOOK_PREVIEW__.selectionStore?.selection?.storyId; const { cursor, ancestors } = this.getState(storyId); this.setState(storyId, { cursor: cursor + 1 }); const id = `${ancestors.slice(-1)[0] || storyId} [${cursor}] ${method}`; diff --git a/code/lib/preview-api/src/modules/core-client/start.ts b/code/lib/preview-api/src/modules/core-client/start.ts index bdfb5698ae3..bd5c1c22df1 100644 --- a/code/lib/preview-api/src/modules/core-client/start.ts +++ b/code/lib/preview-api/src/modules/core-client/start.ts @@ -3,7 +3,7 @@ import global from 'global'; import type { Renderer, ArgsStoryFn, Path, ProjectAnnotations } from '@storybook/types'; import { createChannel } from '@storybook/channel-postmessage'; import { FORCE_RE_RENDER } from '@storybook/core-events'; -import { addons } from '../addons'; +import { addons } from '../../addons'; import { PreviewWeb } from '../../preview-web'; import { ClientApi } from '../../client-api'; diff --git a/code/ui/manager/src/components/preview/preview.tsx b/code/ui/manager/src/components/preview/preview.tsx index 41f3f64dcc7..5ee0cc07f5c 100644 --- a/code/ui/manager/src/components/preview/preview.tsx +++ b/code/ui/manager/src/components/preview/preview.tsx @@ -1,6 +1,7 @@ +import global from 'global'; + import React, { Fragment, useMemo, useEffect, useRef, useState } from 'react'; import { Helmet } from 'react-helmet-async'; -import global from 'global'; import { type API, @@ -24,6 +25,8 @@ import { FramesRenderer } from './FramesRenderer'; import type { PreviewProps } from './utils/types'; +const { FEATURES } = global; + const getWrappers = (getFn: API['getElements']) => Object.values(getFn(types.PREVIEW)); const getTabs = (getFn: API['getElements']) => Object.values(getFn(types.TAB)); @@ -70,7 +73,7 @@ const createCanvas = (id: string, baseUrl = 'iframe.html', withLoader = true): A const [progress, setProgress] = useState(undefined); useEffect(() => { - if (global.CONFIG_TYPE === 'DEVELOPMENT') { + if (FEATURES?.storyStoreV7 && global.CONFIG_TYPE === 'DEVELOPMENT') { const channel = addons.getServerChannel(); channel.on(PREVIEW_BUILDER_PROGRESS, (options) => { From f8d5551d79187539d9521ea30f8365aecf88d541 Mon Sep 17 00:00:00 2001 From: Norbert de Langen Date: Fri, 2 Dec 2022 17:46:56 +0100 Subject: [PATCH 2/4] try to make builder-manager prefer mjs, for tree-shaking --- code/lib/builder-manager/src/index.ts | 2 ++ code/lib/core-common/src/presets.ts | 24 +++++++++++++++++++----- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/code/lib/builder-manager/src/index.ts b/code/lib/builder-manager/src/index.ts index cda53d1161d..edce7b73960 100644 --- a/code/lib/builder-manager/src/index.ts +++ b/code/lib/builder-manager/src/index.ts @@ -41,6 +41,7 @@ export const getConfig: ManagerBuilder['getConfig'] = async (options) => { outdir: join(options.outputDir || './', 'sb-addons'), format: 'esm', write: false, + resolveExtensions: ['.ts', '.tsx', '.mjs', '.js', '.jsx'], outExtension: { '.js': '.mjs' }, loader: { '.js': 'jsx', @@ -57,6 +58,7 @@ export const getConfig: ManagerBuilder['getConfig'] = async (options) => { bundle: true, minify: false, sourcemap: true, + conditions: ['browser', 'module', 'default'], jsxFactory: 'React.createElement', jsxFragment: 'React.Fragment', diff --git a/code/lib/core-common/src/presets.ts b/code/lib/core-common/src/presets.ts index c17f8c17d41..12a7b2d23d6 100644 --- a/code/lib/core-common/src/presets.ts +++ b/code/lib/core-common/src/presets.ts @@ -10,6 +10,7 @@ import type { PresetConfig, Presets, } from '@storybook/types'; +import { join, parse } from 'path'; import { loadCustomPresets } from './utils/load-custom-presets'; import { safeResolve, safeResolveFrom } from './utils/safeResolve'; import { interopRequireDefault } from './utils/interpret-require'; @@ -66,14 +67,19 @@ export const resolveAddonName = ( const resolved = resolve(name); if (resolved) { - if (name.match(/\/(manager|register(-panel)?)(\.(js|ts|tsx|jsx))?$/)) { + const { dir: fdir, name: fname } = parse(resolved); + + if (name.match(/\/(manager|register(-panel)?)(\.(js|mjs|ts|tsx|jsx))?$/)) { return { type: 'virtual', name, - managerEntries: [resolved], + // we remove the extension + // this is a bit of a hack to try en be able to find .mjs files + // node can only ever resolve .js files; it does not look at the exports field in package.json + managerEntries: [join(fdir, fname)], }; } - if (name.match(/\/(preset)(\.(js|ts|tsx|jsx))?$/)) { + if (name.match(/\/(preset)(\.(js|mjs|ts|tsx|jsx))?$/)) { return { type: 'presets', name: resolved, @@ -113,11 +119,19 @@ export const resolveAddonName = ( const managerEntries = []; if (managerFile) { - managerEntries.push(managerFile); + // we remove the extension + // this is a bit of a hack to try en be able to find .mjs files + // node can only ever resolve .js files; it does not look at the exports field in package.json + const { dir: fdir, name: fname } = parse(managerFile); + managerEntries.push(join(fdir, fname)); } // register file is the old way of registering addons if (!managerFile && registerFile && !presetFile) { - managerEntries.push(registerFile); + // we remove the extension + // this is a bit of a hack to try en be able to find .mjs files + // node can only ever resolve .js files; it does not look at the exports field in package.json + const { dir: fdir, name: fname } = parse(registerFile); + managerEntries.push(join(fdir, fname)); } return { From b6f3d29d6af5a6c7dbf455834fefe5813f5da17f Mon Sep 17 00:00:00 2001 From: Norbert de Langen Date: Fri, 2 Dec 2022 17:48:54 +0100 Subject: [PATCH 3/4] trying to debug the vite-builder problem The issue is vite doesn't pass certain files throw the transform plugin, this causes the globalization to not be complete, causing multiple version of the same module, which is designed to be a singleton. me and Ian looked at it, we need to investigate further. Co-authored-by: Ian VanSchooten --- .../builder-vite/src/codegen-iframe-script.ts | 46 ++++++++----------- code/lib/builder-vite/src/vite-config.ts | 14 +++++- .../src/modules/client-api/ClientApi.ts | 4 ++ code/renderers/react/src/public-api.tsx | 4 ++ 4 files changed, 41 insertions(+), 27 deletions(-) diff --git a/code/lib/builder-vite/src/codegen-iframe-script.ts b/code/lib/builder-vite/src/codegen-iframe-script.ts index 9cae4e246e6..ea39e056c7e 100644 --- a/code/lib/builder-vite/src/codegen-iframe-script.ts +++ b/code/lib/builder-vite/src/codegen-iframe-script.ts @@ -21,34 +21,28 @@ export async function generateIframeScriptCode(options: ExtendedOptions) { const importArray = (name: string, length: number) => new Array(length).fill(0).map((_, i) => `${name}_${i}`); + // the "__DO_NOT_USE_OR_YOU_WILL_BE_FIRED_STORYSTOREV6_API__" is a experiment I tried to fix the vite-builder in dev mode. + // it should be removed, if this is the only way to make things work (oof), then it likely mean we have to add more, and add it to all renderers + // but I'm really hoping we do not have to do that. + // noinspection UnnecessaryLocalVariableJS /** @todo Inline variable and remove `noinspection` */ // language=JavaScript const code = ` // Ensure that the client API is initialized by the framework before any other iframe code // is loaded. That way our client-apis can assume the existence of the API+store - import { configure } from '${rendererName}'; - + import { configure, __DO_NOT_USE_OR_YOU_WILL_BE_FIRED_STORYSTOREV6_API__ } from '${rendererName}'; import { logger } from '@storybook/client-logger'; - import { - addDecorator, - addParameters, - addLoader, - addArgs, - addArgTypes, - addStepRunner, - addArgTypesEnhancer, - addArgsEnhancer, - setGlobalRender, - } from "@storybook/preview-api"; + + const { clientApi } = __DO_NOT_USE_OR_YOU_WILL_BE_FIRED_STORYSTOREV6_API__; + ${filesToImport(configEntries, 'config')} import * as preview from '${virtualPreviewFile}'; + import * as previewApi from '@storybook/preview-api'; import { configStories } from '${virtualStoriesFile}'; - - console.log('${rendererName}'); - + const configs = [${importArray('config', configEntries.length) .concat('preview.default') .join(',')}].filter(Boolean) @@ -58,34 +52,34 @@ export async function generateIframeScriptCode(options: ExtendedOptions) { const value = config[key]; switch (key) { case 'args': { - return addArgs(value); + return clientApi.addArgs(value); } case 'argTypes': { - return addArgTypes(value); + return clientApi.addArgTypes(value); } case 'decorators': { - return value.forEach((decorator) => addDecorator(decorator, false)); + return value.forEach((decorator) => clientApi.addDecorator(decorator, false)); } case 'loaders': { - return value.forEach((loader) => addLoader(loader, false)); + return value.forEach((loader) => clientApi.addLoader(loader, false)); } case 'parameters': { - return addParameters({ ...value }, false); + return clientApi.addParameters({ ...value }, false); } case 'argTypesEnhancers': { - return value.forEach((enhancer) => addArgTypesEnhancer(enhancer)); + return value.forEach((enhancer) => clientApi.addArgTypesEnhancer(enhancer)); } case 'argsEnhancers': { - return value.forEach((enhancer) => addArgsEnhancer(enhancer)) + return value.forEach((enhancer) => clientApi.addArgsEnhancer(enhancer)) } case 'render': { - return setGlobalRender(value) + return previewApi.setGlobalRender(value) } case 'globals': case 'globalTypes': { const v = {}; v[key] = value; - return addParameters(v, false); + return clientApi.addParameters(v, false); } case 'decorateStory': case 'applyDecorators': @@ -94,7 +88,7 @@ export async function generateIframeScriptCode(options: ExtendedOptions) { return null; // This key is not handled directly in v6 mode. } case 'runStep': { - return addStepRunner(value); + return previewApi.addStepRunner(value); } default: { // eslint-disable-next-line prefer-template diff --git a/code/lib/builder-vite/src/vite-config.ts b/code/lib/builder-vite/src/vite-config.ts index 464373b6f7c..1451d08041d 100644 --- a/code/lib/builder-vite/src/vite-config.ts +++ b/code/lib/builder-vite/src/vite-config.ts @@ -19,6 +19,10 @@ import { } from './plugins'; import type { ExtendedOptions } from './types'; +const globalPluginInstance = externalGlobals(globals); + +console.log({ globalPluginInstance, fn: globalPluginInstance.transform.toString() }); + export type PluginConfigType = 'build' | 'development'; const configEnvServe: ConfigEnv = { @@ -74,6 +78,14 @@ export async function pluginConfig(options: ExtendedOptions) { const plugins = [ codeGeneratorPlugin(options), + { + name: 'ians-cool-plugin', + enforce: 'post', + transform: (code, id) => { + console.log({ id }); + return code; + }, + }, // sourceLoaderPlugin(options), mdxPlugin(), injectExportOrderPlugin, @@ -91,7 +103,7 @@ export async function pluginConfig(options: ExtendedOptions) { } }, }, - externalGlobals(globals), + globalPluginInstance, ] as PluginOption[]; // We need the react plugin here to support MDX in non-react projects. diff --git a/code/lib/preview-api/src/modules/client-api/ClientApi.ts b/code/lib/preview-api/src/modules/client-api/ClientApi.ts index f2d8eb35874..22af05618eb 100644 --- a/code/lib/preview-api/src/modules/client-api/ClientApi.ts +++ b/code/lib/preview-api/src/modules/client-api/ClientApi.ts @@ -29,6 +29,10 @@ import { combineParameters, composeStepRunners, normalizeInputTypes } from '../. import { StoryStoreFacade } from './StoryStoreFacade'; +console.log( + 'CLIENT_API_MODULE, you should only see this log, exactly ONCE, in the browser console' +); + // ClientApi (and StoreStore) are really singletons. However they are not created until the // relevant framework instanciates them via `start.js`. The good news is this happens right away. let singleton: ClientApi; diff --git a/code/renderers/react/src/public-api.tsx b/code/renderers/react/src/public-api.tsx index 05540a0fb82..e0ccb470a75 100644 --- a/code/renderers/react/src/public-api.tsx +++ b/code/renderers/react/src/public-api.tsx @@ -23,3 +23,7 @@ export const storiesOf: ClientApi['storiesOf'] = (kind, m) => { export const configure: ClientApi['configure'] = (...args) => api.configure(FRAMEWORK, ...args); export const forceReRender: ClientApi['forceReRender'] = api.forceReRender; export const raw: ClientApi['raw'] = api.clientApi.raw; + +// I added this temporarily, it should be removed, I only added it to debug the vite singleton module problem when running storybook in dev-mode +// eslint-disable-next-line no-underscore-dangle, @typescript-eslint/naming-convention +export const __DO_NOT_USE_OR_YOU_WILL_BE_FIRED_STORYSTOREV6_API__ = api; From db36af9306393d23e01500eb7c94b6473f16f18a Mon Sep 17 00:00:00 2001 From: Norbert de Langen Date: Fri, 2 Dec 2022 17:57:07 +0100 Subject: [PATCH 4/4] unsupported temp api that's not supposed to be there, doesn't need types --- code/renderers/react/src/public-api.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/renderers/react/src/public-api.tsx b/code/renderers/react/src/public-api.tsx index e0ccb470a75..ce0d90c8af4 100644 --- a/code/renderers/react/src/public-api.tsx +++ b/code/renderers/react/src/public-api.tsx @@ -26,4 +26,4 @@ export const raw: ClientApi['raw'] = api.clientApi.raw; // I added this temporarily, it should be removed, I only added it to debug the vite singleton module problem when running storybook in dev-mode // eslint-disable-next-line no-underscore-dangle, @typescript-eslint/naming-convention -export const __DO_NOT_USE_OR_YOU_WILL_BE_FIRED_STORYSTOREV6_API__ = api; +export const __DO_NOT_USE_OR_YOU_WILL_BE_FIRED_STORYSTOREV6_API__ = api as any;