Merge pull request #23307 from storybookjs/norbert/page-addons-refactor

Addon API: Add experimental `page` addon type
storybookjs · Jul 13, 2023 · 82afbe6 · 82afbe6
2 parents 7bb7db7 + d5b712b
commit 82afbe6
Show file tree

Hide file tree

Showing 28 changed files with 585 additions and 216 deletions.
diff --git a/MIGRATION.md b/MIGRATION.md
@@ -1,5 +1,7 @@
 <h1>Migration</h1>
 
+- [From version 7.0.0 to 7.2.0](#from-version-700-to-720)
+    - [Addon API are more type-strict](#addon-api-are-more-type-strict)
 - [From version 6.5.x to 7.0.0](#from-version-65x-to-700)
   - [7.0 breaking changes](#70-breaking-changes)
     - [Dropped support for Node 15 and below](#dropped-support-for-node-15-and-below)
@@ -300,6 +302,31 @@
   - [Packages renaming](#packages-renaming)
   - [Deprecated embedded addons](#deprecated-embedded-addons)
 
+## From version 7.0.0 to 7.2.0
+
+#### Addon API are more type-strict
+
+When registering an addon using `@storybook/manager-api`, the addon API is now more type-strict. This means if you use typescript to compile your addon before publishing it might start giving you errors.
+
+The `type` property is now required field, and the `id` property should not be set anymore.
+
+Here's a correct example:
+```tsx
+import { addons, types } from '@storybook/manager-api';
+
+addons.register('my-addon', () => {
+  addons.add('my-addon/panel', {
+    type: types.PANEL,
+    title: 'My Addon',
+    render: ({ active }) => active ? <div>Hello World</div> : null,
+  });
+});
+```
+
+The API: `addons.addPanel()` is now deprecated, and will be removed in 8.0.0. Please use `addons.add()` instead.
+
+The `render` method can now be a `React.FunctionComponent` (without the `children` prop). Storybook will now render it, rather than calling it as a function.
+
 ## From version 6.5.x to 7.0.0
 
 A number of these changes can be made automatically by the Storybook CLI. To take advantage of these "automigrations", run `npx storybook@latest upgrade --prerelease` or `pnpx dlx storybook@latest upgrade --prerelease`.

diff --git a/code/addons/a11y/src/manager.tsx b/code/addons/a11y/src/manager.tsx
@@ -36,8 +36,8 @@ addons.register(ADDON_ID, (api) => {
   addons.add(PANEL_ID, {
     title: Title,
     type: types.PANEL,
-    render: ({ active = true, key }) => (
-      <A11yContextProvider key={key} active={active}>
+    render: ({ active = true }) => (
+      <A11yContextProvider active={active}>
         <A11YPanel />
       </A11yContextProvider>
     ),

diff --git a/code/addons/actions/src/manager.tsx b/code/addons/actions/src/manager.tsx
@@ -36,7 +36,7 @@ addons.register(ADDON_ID, (api) => {
   addons.add(PANEL_ID, {
     title: Title,
     type: types.PANEL,
-    render: ({ active, key }) => <ActionLogger key={key} api={api} active={!!active} />,
+    render: ({ active }) => <ActionLogger api={api} active={!!active} />,
     paramKey: PARAM_KEY,
   });
 });
diff --git a/code/addons/controls/src/manager.tsx b/code/addons/controls/src/manager.tsx
@@ -26,12 +26,12 @@ addons.register(ADDON_ID, (api) => {
     title: Title,
     type: types.PANEL,
     paramKey: PARAM_KEY,
-    render: ({ key, active }) => {
+    render: ({ active }) => {
       if (!active || !api.getCurrentStoryData()) {
         return null;
       }
       return (
-        <AddonPanel key={key} active={active}>
+        <AddonPanel active={active}>
           <ControlsPanel />
         </AddonPanel>
       );

diff --git a/code/addons/interactions/src/manager.tsx b/code/addons/interactions/src/manager.tsx
@@ -29,15 +29,15 @@ addons.register(ADDON_ID, (api) => {
     type: types.PANEL,
     title: Title,
     match: ({ viewMode }) => viewMode === 'story',
-    render: ({ key, active }) => {
+    render: ({ active }) => {
       const newLocal = useCallback(({ state }: Combo) => {
         return {
           storyId: state.storyId,
         };
       }, []);
 
       return (
-        <AddonPanel key={key} active={active}>
+        <AddonPanel active={active}>
           <Consumer filter={newLocal}>{({ storyId }) => <Panel storyId={storyId} />}</Consumer>
         </AddonPanel>
       );

diff --git a/code/addons/jest/src/manager.tsx b/code/addons/jest/src/manager.tsx
@@ -8,7 +8,7 @@ addons.register(ADDON_ID, (api) => {
   addons.add(PANEL_ID, {
     title: 'Tests',
     type: types.PANEL,
-    render: ({ active, key }) => <Panel key={key} api={api} active={active} />,
+    render: ({ active }) => <Panel api={api} active={active} />,
     paramKey: PARAM_KEY,
   });
 });
diff --git a/code/addons/storysource/src/manager.tsx b/code/addons/storysource/src/manager.tsx
@@ -8,7 +8,7 @@ addons.register(ADDON_ID, (api) => {
   addons.add(PANEL_ID, {
     type: types.PANEL,
     title: 'Code',
-    render: ({ active, key }) => (active ? <StoryPanel key={key} api={api} /> : null),
+    render: ({ active }) => (active ? <StoryPanel api={api} /> : null),
     paramKey: 'storysource',
   });
 });
diff --git a/code/lib/manager-api/src/index.tsx b/code/lib/manager-api/src/index.tsx
@@ -183,7 +183,7 @@ class ManagerProvider extends Component<ManagerProviderProps, State> {
       location,
       path,
       refId,
-      viewMode = props.docsOptions.docsMode ? 'docs' : 'story',
+      viewMode = props.docsOptions.docsMode ? 'docs' : props.viewMode,
       singleStory,
       storyId,
       docsOptions,
@@ -418,36 +418,41 @@ const addonStateCache: {
 // shared state
 export function useSharedState<S>(stateId: string, defaultState?: S) {
   const api = useStorybookApi();
-  const existingState = api.getAddonState<S>(stateId);
+  const existingState = api.getAddonState<S>(stateId) || addonStateCache[stateId];
   const state = orDefault<S>(
     existingState,
     addonStateCache[stateId] ? addonStateCache[stateId] : defaultState
   );
-  const setState = (s: S | API_StateMerger<S>, options?: Options) => {
-    // set only after the stories are loaded
-    if (addonStateCache[stateId]) {
+
+  if (api.getAddonState(stateId) && api.getAddonState(stateId) !== state) {
+    api.setAddonState<S>(stateId, state).then((s) => {
       addonStateCache[stateId] = s;
-    }
-    api.setAddonState<S>(stateId, s, options);
+    });
+  }
+
+  const setState = (s: S | API_StateMerger<S>, options?: Options) => {
+    const result = api.setAddonState<S>(stateId, s, options);
+    addonStateCache[stateId] = result;
+    return result;
   };
   const allListeners = useMemo(() => {
     const stateChangeHandlers = {
-      [`${SHARED_STATE_CHANGED}-client-${stateId}`]: (s: S) => setState(s),
-      [`${SHARED_STATE_SET}-client-${stateId}`]: (s: S) => setState(s),
+      [`${SHARED_STATE_CHANGED}-client-${stateId}`]: setState,
+      [`${SHARED_STATE_SET}-client-${stateId}`]: setState,
     };
     const stateInitializationHandlers = {
-      [SET_STORIES]: () => {
+      [SET_STORIES]: async () => {
         const currentState = api.getAddonState(stateId);
         if (currentState) {
           addonStateCache[stateId] = currentState;
           api.emit(`${SHARED_STATE_SET}-manager-${stateId}`, currentState);
         } else if (addonStateCache[stateId]) {
           // this happens when HMR
-          setState(addonStateCache[stateId]);
+          await setState(addonStateCache[stateId]);
           api.emit(`${SHARED_STATE_SET}-manager-${stateId}`, addonStateCache[stateId]);
         } else if (defaultState !== undefined) {
           // if not HMR, yet the defaults are from the manager
-          setState(defaultState);
+          await setState(defaultState);
           // initialize addonStateCache after first load, so its available for subsequent HMR
           addonStateCache[stateId] = defaultState;
           api.emit(`${SHARED_STATE_SET}-manager-${stateId}`, defaultState);
@@ -471,9 +476,9 @@ export function useSharedState<S>(stateId: string, defaultState?: S) {
   const emit = useChannel(allListeners);
   return [
     state,
-    (newStateOrMerger: S | API_StateMerger<S>, options?: Options) => {
-      setState(newStateOrMerger, options);
-      emit(`${SHARED_STATE_CHANGED}-manager-${stateId}`, newStateOrMerger);
+    async (newStateOrMerger: S | API_StateMerger<S>, options?: Options) => {
+      const result = await setState(newStateOrMerger, options);
+      emit(`${SHARED_STATE_CHANGED}-manager-${stateId}`, result);
     },
   ] as [S, (newStateOrMerger: S | API_StateMerger<S>, options?: Options) => void];
 }

diff --git a/code/lib/manager-api/src/lib/addons.ts b/code/lib/manager-api/src/lib/addons.ts
@@ -8,6 +8,8 @@ import type {
   Addon_Elements,
   Addon_Loaders,
   Addon_Type,
+  Addon_BaseType,
+  Addon_PageType,
   Addon_Types,
 } from '@storybook/types';
 import { Addon_TypesEnum } from '@storybook/types';
@@ -93,28 +95,56 @@ export class AddonStore {
     this.serverChannel = channel;
   };
 
-  getElements = (type: Addon_Types): Addon_Collection => {
+  getElements<T extends Addon_Types | Addon_TypesEnum.experimental_PAGE>(
+    type: T
+  ): T extends Addon_TypesEnum.experimental_PAGE
+    ? Addon_Collection<Addon_PageType>
+    : Addon_Collection<Addon_BaseType> {
     if (!this.elements[type]) {
       this.elements[type] = {};
     }
+    // @ts-expect-error (Kaspar told me to do this)
     return this.elements[type];
-  };
+  }
 
+  /**
+   * Adds a panel to the addon store.
+   * @param {string} id - The id of the panel.
+   * @param {Addon_Type} options - The options for the panel.
+   * @returns {void}
+   *
+   * @deprecated Use the 'add' method instead.
+   * @example
+   * addons.add('My Panel', {
+   *   title: 'My Title',
+   *   type: types.PANEL,
+   *   render: () => <div>My Content</div>,
+   * });
+   */
   addPanel = (
     id: string,
-    options: Omit<Addon_Type, 'type' | 'id'> & DeprecatedAddonWithId
+    options: Omit<Addon_BaseType, 'type' | 'id'> & DeprecatedAddonWithId
   ): void => {
     this.add(id, {
       type: Addon_TypesEnum.PANEL,
       ...options,
     });
   };
 
-  add = (id: string, addon: Omit<Addon_Type, 'id'> & DeprecatedAddonWithId) => {
+  /**
+   * Adds an addon to the addon store.
+   * @param {string} id - The id of the addon.
+   * @param {Addon_Type} addon - The addon to add.
+   * @returns {void}
+   */
+  add(
+    id: string,
+    addon: Addon_BaseType | (Omit<Addon_PageType, 'id'> & DeprecatedAddonWithId)
+  ): void {
     const { type } = addon;
     const collection = this.getElements(type);
     collection[id] = { id, ...addon };
-  };
+  }
 
   setConfig = (value: Addon_Config) => {
     Object.assign(this.config, value);
@@ -129,11 +159,18 @@ export class AddonStore {
 
   getConfig = () => this.config;
 
-  register = (name: string, registerCallback: (api: API) => void): void => {
-    if (this.loaders[name]) {
-      logger.warn(`${name} was loaded twice, this could have bad side-effects`);
+  /**
+   * Registers an addon loader function.
+   *
+   * @param {string} id - The id of the addon loader.
+   * @param {(api: API) => void} callback - The function that will be called to register the addon.
+   * @returns {void}
+   */
+  register = (id: string, callback: (api: API) => void): void => {
+    if (this.loaders[id]) {
+      logger.warn(`${id} was loaded twice, this could have bad side-effects`);
     }
-    this.loaders[name] = registerCallback;
+    this.loaders[id] = callback;
   };
 
   loadAddons = (api: any) => {

diff --git a/code/lib/manager-api/src/modules/addons.ts b/code/lib/manager-api/src/modules/addons.ts
@@ -1,7 +1,8 @@
 import type {
-  Addon_Type,
+  Addon_BaseType,
+  Addon_Collection,
+  Addon_PageType,
   Addon_Types,
-  API_Collection,
   API_Panels,
   API_StateMerger,
 } from '@storybook/types';
@@ -19,10 +20,14 @@ export interface SubAPI {
    * Returns a collection of elements of a specific type.
    * @protected This is used internally in storybook's manager.
    * @template T - The type of the elements in the collection.
-   * @param {Addon_Types} type - The type of the elements to retrieve.
+   * @param {Addon_Types | Addon_TypesEnum.experimental_PAGE} type - The type of the elements to retrieve.
    * @returns {API_Collection<T>} - A collection of elements of the specified type.
    */
-  getElements: <T = Addon_Type>(type: Addon_Types) => API_Collection<T>;
+  getElements: <T extends Addon_Types | Addon_TypesEnum.experimental_PAGE = Addon_Types>(
+    type: T
+  ) => T extends Addon_TypesEnum.experimental_PAGE
+    ? Addon_Collection<Addon_PageType>
+    : Addon_Collection<Addon_BaseType>;
   /**
    * Returns a collection of all panels.
    * This is the same as calling getElements('panel')
@@ -101,7 +106,7 @@ export const init: ModuleFn<SubAPI, SubState> = ({ provider, store, fullAPI }) =
 
       const { parameters } = story;
 
-      const filteredPanels: API_Collection = {};
+      const filteredPanels: Addon_Collection<Addon_BaseType> = {};
       Object.entries(allPanels).forEach(([id, panel]) => {
         const { paramKey } = panel;
         if (paramKey && parameters && parameters[paramKey] && parameters[paramKey].disable) {

diff --git a/code/lib/manager-api/src/modules/settings.ts b/code/lib/manager-api/src/modules/settings.ts
@@ -1,7 +1,9 @@
-import type { API_Settings } from '@storybook/types';
+import type { API_Settings, StoryId } from '@storybook/types';
 import type { ModuleFn } from '../index';
 
 export interface SubAPI {
+  storeSelection: () => void;
+  retrieveSelection: () => StoryId;
   /**
    * Changes the active settings tab.
    * @param path - The path of the settings page to navigate to. The path NOT should include the `/settings` prefix.
@@ -62,6 +64,18 @@ export const init: ModuleFn<SubAPI, SubState> = ({ store, navigate, fullAPI }) =
 
       navigate(path);
     },
+    retrieveSelection() {
+      const { settings } = store.getState();
+
+      return settings.lastTrackedStoryId;
+    },
+    storeSelection: async () => {
+      const { storyId, settings } = store.getState();
+
+      await store.setState({
+        settings: { ...settings, lastTrackedStoryId: storyId },
+      });
+    },
   };
 
   return { state: { settings: { lastTrackedStoryId: null } }, api };

diff --git a/code/lib/manager-api/src/modules/stories.ts b/code/lib/manager-api/src/modules/stories.ts
@@ -37,6 +37,7 @@ import {
   CURRENT_STORY_WAS_SET,
   STORY_MISSING,
   DOCS_PREPARED,
+  SET_CURRENT_STORY,
 } from '@storybook/core-events';
 import { logger } from '@storybook/client-logger';
 
@@ -593,13 +594,26 @@ export const init: ModuleFn<SubAPI, SubState, true> = ({
         const { sourceType } = getEventMetadata(this, fullAPI);
 
         if (sourceType === 'local') {
-          if (fullAPI.isSettingsScreenActive()) return;
-
-          // Special case -- if we are already at the story being specified (i.e. the user started at a given story),
-          // we don't need to change URL. See https://github.com/storybookjs/storybook/issues/11677
           const state = store.getState();
-          if (state.storyId !== storyId || state.viewMode !== viewMode) {
-            navigate(`/${viewMode}/${storyId}`);
+          const isCanvasRoute =
+            state.path === '/' || state.viewMode === 'story' || state.viewMode === 'docs';
+          const stateHasSelection = state.viewMode && state.storyId;
+          const stateSelectionDifferent = state.viewMode !== viewMode || state.storyId !== storyId;
+          /**
+           * When storybook starts, we want to navigate to the first story.
+           * But there are a few exceptions:
+           * - If the current storyId and viewMode are already set/correct.
+           * - If the user has navigated away already.
+           * - If the user started storybook with a specific page-URL like "/settings/about"
+           */
+          if (isCanvasRoute) {
+            if (stateHasSelection && stateSelectionDifferent) {
+              // The manager state is correct, the preview state is lagging behind
+              fullAPI.emit(SET_CURRENT_STORY, { storyId: state.storyId, viewMode: state.viewMode });
+            } else if (stateSelectionDifferent) {
+              // The preview state is correct, the manager state is lagging behind
+              navigate(`/${viewMode}/${storyId}`);
+            }
           }
         }
       }