Merge branch 'jeppe/react-prerelease-sandboxes' of github.com:storybo…

…okjs/storybook into jeppe/temp-gen-react-prerelease-sandboxes
storybookjs · May 4, 2024 · 1ad7579 · 1ad7579
2 parents 78e3758 + da74a1f
commit 1ad7579
Show file tree

Hide file tree

Showing 12 changed files with 180 additions and 236 deletions.
diff --git a/code/frameworks/nextjs/src/export-mocks/navigation/index.ts b/code/frameworks/nextjs/src/export-mocks/navigation/index.ts
@@ -1,9 +1,7 @@
 import type { Mock } from '@storybook/test';
 import { fn } from '@storybook/test';
-import * as actual from 'next/dist/client/components/navigation';
 import { NextjsRouterMocksNotAvailable } from '@storybook/core-events/preview-errors';
-import { RedirectStatusCode } from 'next/dist/client/components/redirect-status-code';
-import { getRedirectError } from 'next/dist/client/components/redirect';
+import * as originalNavigation from 'next/dist/client/components/navigation';
 
 let navigationAPI: {
   push: Mock;
@@ -58,37 +56,34 @@ export const getRouter = () => {
 export * from 'next/dist/client/components/navigation';
 
 // mock utilities/overrides (as of Next v14.2.0)
-export const redirect = fn(
-  (url: string, type: actual.RedirectType = actual.RedirectType.push): never => {
-    throw getRedirectError(url, type, RedirectStatusCode.SeeOther);
-  }
-).mockName('next/navigation::redirect');
-
-export const permanentRedirect = fn(
-  (url: string, type: actual.RedirectType = actual.RedirectType.push): never => {
-    throw getRedirectError(url, type, RedirectStatusCode.SeeOther);
-  }
-).mockName('next/navigation::permanentRedirect');
+export const redirect = fn().mockName('next/navigation::redirect');
 
 // passthrough mocks - keep original implementation but allow for spying
-export const useSearchParams = fn(actual.useSearchParams).mockName(
+export const useSearchParams = fn(originalNavigation.useSearchParams).mockName(
   'next/navigation::useSearchParams'
 );
-export const usePathname = fn(actual.usePathname).mockName('next/navigation::usePathname');
-export const useSelectedLayoutSegment = fn(actual.useSelectedLayoutSegment).mockName(
+export const usePathname = fn(originalNavigation.usePathname).mockName(
+  'next/navigation::usePathname'
+);
+export const useSelectedLayoutSegment = fn(originalNavigation.useSelectedLayoutSegment).mockName(
   'next/navigation::useSelectedLayoutSegment'
 );
-export const useSelectedLayoutSegments = fn(actual.useSelectedLayoutSegments).mockName(
+export const useSelectedLayoutSegments = fn(originalNavigation.useSelectedLayoutSegments).mockName(
   'next/navigation::useSelectedLayoutSegments'
 );
-export const useRouter = fn(actual.useRouter).mockName('next/navigation::useRouter');
-export const useServerInsertedHTML = fn(actual.useServerInsertedHTML).mockName(
+export const useRouter = fn(originalNavigation.useRouter).mockName('next/navigation::useRouter');
+export const useServerInsertedHTML = fn(originalNavigation.useServerInsertedHTML).mockName(
   'next/navigation::useServerInsertedHTML'
 );
-export const notFound = fn(actual.notFound).mockName('next/navigation::notFound');
+export const notFound = fn(originalNavigation.notFound).mockName('next/navigation::notFound');
+export const permanentRedirect = fn(originalNavigation.permanentRedirect).mockName(
+  'next/navigation::permanentRedirect'
+);
 
 // Params, not exported by Next.js, is manually declared to avoid inference issues.
 interface Params {
   [key: string]: string | string[];
 }
-export const useParams = fn<[], Params>(actual.useParams).mockName('next/navigation::useParams');
+export const useParams = fn<[], Params>(originalNavigation.useParams).mockName(
+  'next/navigation::useParams'
+);
diff --git a/code/frameworks/nextjs/src/fastRefresh/webpack.ts b/code/frameworks/nextjs/src/fastRefresh/webpack.ts
@@ -4,9 +4,10 @@ import ReactRefreshWebpackPlugin from '@pmmmwh/react-refresh-webpack-plugin';
 export const configureFastRefresh = (baseConfig: WebpackConfig): void => {
   baseConfig.plugins = [
     ...(baseConfig.plugins ?? []),
-    // overlay is disabled as it is shown with caught errors in error boundaries
-    // and the next app router is using error boundaries to redirect
-    // TODO use the Next error overlay
-    new ReactRefreshWebpackPlugin({ overlay: false }),
+    new ReactRefreshWebpackPlugin({
+      overlay: {
+        sockIntegration: 'whm',
+      },
+    }),
   ];
 };
diff --git a/code/frameworks/nextjs/src/preview.tsx b/code/frameworks/nextjs/src/preview.tsx
@@ -17,7 +17,6 @@ import { createRouter } from '@storybook/nextjs/router.mock';
 // eslint-disable-next-line @typescript-eslint/ban-ts-comment
 // @ts-ignore we must ignore types here as during compilation they are not generated yet
 import { createNavigation } from '@storybook/nextjs/navigation.mock';
-import { isNextRouterError } from 'next/dist/client/components/is-next-router-error';
 
 function addNextHeadCount() {
   const meta = document.createElement('meta');
@@ -26,33 +25,8 @@ function addNextHeadCount() {
   document.head.appendChild(meta);
 }
 
-function isAsyncClientComponentError(error: unknown) {
-  return (
-    typeof error === 'string' &&
-    (error.includes('A component was suspended by an uncached promise.') ||
-      error.includes('async/await is not yet supported in Client Components'))
-  );
-}
 addNextHeadCount();
 
-// Copying Next patch of console.error:
-// https://github.com/vercel/next.js/blob/a74deb63e310df473583ab6f7c1783bc609ca236/packages/next/src/client/app-index.tsx#L15
-const origConsoleError = globalThis.console.error;
-globalThis.console.error = (...args: unknown[]) => {
-  const error = args[0];
-  if (isNextRouterError(error) || isAsyncClientComponentError(error)) {
-    return;
-  }
-  origConsoleError.apply(globalThis.console, args);
-};
-
-globalThis.addEventListener('error', (ev: WindowEventMap['error']): void => {
-  if (isNextRouterError(ev.error) || isAsyncClientComponentError(ev.error)) {
-    ev.preventDefault();
-    return;
-  }
-});
-
 export const decorators: Addon_DecoratorFunction<any>[] = [
   StyledJsxDecorator,
   ImageDecorator,
@@ -78,12 +52,4 @@ export const parameters = {
       excludeDecorators: true,
     },
   },
-  react: {
-    rootOptions: {
-      onCaughtError(error: unknown) {
-        if (isNextRouterError(error)) return;
-        console.error(error);
-      },
-    },
-  },
 };
diff --git a/code/frameworks/nextjs/src/routing/decorator.tsx b/code/frameworks/nextjs/src/routing/decorator.tsx
@@ -3,7 +3,6 @@ import type { Addon_StoryContext } from '@storybook/types';
 import { AppRouterProvider } from './app-router-provider';
 import { PageRouterProvider } from './page-router-provider';
 import type { RouteParams, NextAppDirectory } from './types';
-import { RedirectBoundary } from 'next/dist/client/components/redirect-boundary';
 
 const defaultRouterParams: RouteParams = {
   pathname: '/',
@@ -28,14 +27,7 @@ export const RouterDecorator = (
           ...parameters.nextjs?.navigation,
         }}
       >
-        {/*
-        The next.js RedirectBoundary causes flashing UI when used client side.
-        Possible use the implementation of the PR: https://github.com/vercel/next.js/pull/49439
-        Or wait for next to solve this on their side.
-        */}
-        <RedirectBoundary>
-          <Story />
-        </RedirectBoundary>
+        <Story />
       </AppRouterProvider>
     );
   }

diff --git a/code/frameworks/nextjs/template/stories_nextjs-default-ts/Redirect.stories.tsx b/code/frameworks/nextjs/template/stories_nextjs-default-ts/Redirect.stories.tsx
diff --git a/code/frameworks/nextjs/template/stories_nextjs-default-ts/ServerActions.stories.tsx b/code/frameworks/nextjs/template/stories_nextjs-default-ts/ServerActions.stories.tsx
@@ -1,9 +1,9 @@
 import React from 'react';
 import type { Meta, StoryObj } from '@storybook/react';
-import { expect, within, userEvent, waitFor } from '@storybook/test';
+import { expect, within, userEvent } from '@storybook/test';
 import { cookies } from '@storybook/nextjs/headers.mock';
 import { revalidatePath } from '@storybook/nextjs/cache.mock';
-import { redirect, getRouter } from '@storybook/nextjs/navigation.mock';
+import { redirect } from '@storybook/nextjs/navigation.mock';
 
 import { accessRoute, login, logout } from './server-actions';
 
@@ -31,84 +31,45 @@ function Component() {
 
 export default {
   component: Component,
-  parameters: {
-    nextjs: {
-      appDirectory: true,
-      navigation: {
-        pathname: '/',
-      },
-    },
-    test: {
-      // This is needed until Next will update to the React 19 beta: https://github.com/vercel/next.js/pull/65058
-      // In the React 19 beta ErrorBoundary errors (such as redirect) are only logged, and not thrown.
-      // We will also suspress console.error logs for re the console.error logs for redirect in the next framework.
-      // Using the onCaughtError react root option:
-      //   react: {
-      //     rootOptions: {
-      //       onCaughtError(error: unknown) {
-      //         if (isNextRouterError(error)) return;
-      //         console.error(error);
-      //       },
-      //     },
-      // See: code/frameworks/nextjs/src/preview.tsx
-      dangerouslyIgnoreUnhandledErrors: true,
-    },
-  },
 } as Meta<typeof Component>;
 
-export const ProtectedWhileLoggedOut: StoryObj<typeof Component> = {
-  play: async ({ canvasElement }) => {
-    const canvas = within(canvasElement);
-    await userEvent.click(canvas.getByText('Access protected route'));
-
-    await expect(cookies().get).toHaveBeenCalledWith('user');
-    await expect(redirect).toHaveBeenCalledWith('/');
-
-    await waitFor(() => expect(getRouter().push).toHaveBeenCalled());
-  },
-};
-
-export const ProtectedWhileLoggedIn: StoryObj<typeof Component> = {
-  beforeEach() {
-    cookies().set('user', 'storybookjs');
-  },
-  play: async ({ canvasElement }) => {
-    const canvas = within(canvasElement);
-    await userEvent.click(canvas.getByText('Access protected route'));
-
-    await expect(cookies().get).toHaveBeenLastCalledWith('user');
-    await expect(revalidatePath).toHaveBeenLastCalledWith('/');
-    await expect(redirect).toHaveBeenLastCalledWith('/protected');
-
-    await waitFor(() => expect(getRouter().push).toHaveBeenCalled());
-  },
-};
-
-export const Logout: StoryObj<typeof Component> = {
-  beforeEach() {
-    cookies().set('user', 'storybookjs');
-  },
-  play: async ({ canvasElement }) => {
+export const Default: StoryObj<typeof Component> = {
+  play: async ({ canvasElement, step }) => {
     const canvas = within(canvasElement);
 
-    await userEvent.click(canvas.getByText('Logout'));
-    await expect(cookies().delete).toHaveBeenCalled();
-    await expect(revalidatePath).toHaveBeenCalledWith('/');
-    await expect(redirect).toHaveBeenCalledWith('/');
+    const loginBtn = canvas.getByText('Login');
+    const logoutBtn = canvas.getByText('Logout');
+    const accessRouteBtn = canvas.getByText('Access protected route');
 
-    await waitFor(() => expect(getRouter().push).toHaveBeenCalled());
-  },
-};
+    await step('accessRoute flow - logged out', async () => {
+      await userEvent.click(accessRouteBtn);
+      await expect(cookies().get).toHaveBeenCalledWith('user');
+      await expect(redirect).toHaveBeenCalledWith('/');
+    });
 
-export const Login: StoryObj<typeof Component> = {
-  play: async ({ canvasElement }) => {
-    const canvas = within(canvasElement);
-    await userEvent.click(canvas.getByText('Login'));
+    await step('accessRoute flow - logged', async () => {
+      cookies.mockRestore();
+      cookies().set('user', 'storybookjs');
+      await userEvent.click(accessRouteBtn);
+      await expect(cookies().get).toHaveBeenCalledWith('user');
+      await expect(revalidatePath).toHaveBeenCalledWith('/');
+      await expect(redirect).toHaveBeenCalledWith('/protected');
+    });
 
-    await expect(cookies().set).toHaveBeenCalledWith('user', 'storybookjs');
-    await expect(revalidatePath).toHaveBeenCalledWith('/');
-    await expect(redirect).toHaveBeenCalledWith('/');
+    await step('logout flow', async () => {
+      cookies.mockRestore();
+      await userEvent.click(logoutBtn);
+      await expect(cookies().delete).toHaveBeenCalled();
+      await expect(revalidatePath).toHaveBeenCalledWith('/');
+      await expect(redirect).toHaveBeenCalledWith('/');
+    });
 
-    await waitFor(() => expect(getRouter().push).toHaveBeenCalled());
+    await step('login flow', async () => {
+      cookies.mockRestore();
+      await userEvent.click(loginBtn);
+      await expect(cookies().set).toHaveBeenCalledWith('user', 'storybookjs');
+      await expect(revalidatePath).toHaveBeenCalledWith('/');
+      await expect(redirect).toHaveBeenCalledWith('/');
+    });
   },
 };
diff --git a/code/lib/preview-api/src/modules/store/csf/portable-stories.test.ts b/code/lib/preview-api/src/modules/store/csf/portable-stories.test.ts
@@ -255,6 +255,49 @@ describe('composeStory', () => {
     expect(spyFn).toHaveBeenNthCalledWith(2, 'from beforeEach');
   });
 
+  it('should warn when previous cleanups are still around when rendering a story', async () => {
+    const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
+    const cleanupSpy = vi.fn();
+    const beforeEachSpy = vi.fn(() => {
+      return () => {
+        cleanupSpy();
+      };
+    });
+
+    const PreviousStory: Story = {
+      render: () => 'first',
+      beforeEach: beforeEachSpy,
+    };
+    const CurrentStory: Story = {
+      render: () => 'second',
+      args: {
+        firstArg: false,
+        secondArg: true,
+      },
+    };
+    const firstComposedStory = composeStory(PreviousStory, {});
+    await firstComposedStory.load();
+    firstComposedStory();
+
+    expect(beforeEachSpy).toHaveBeenCalled();
+    expect(cleanupSpy).not.toHaveBeenCalled();
+    expect(consoleWarnSpy).not.toHaveBeenCalled();
+
+    const secondComposedStory = composeStory(CurrentStory, {});
+    secondComposedStory();
+
+    expect(cleanupSpy).not.toHaveBeenCalled();
+    expect(consoleWarnSpy).toHaveBeenCalledOnce();
+    expect(consoleWarnSpy.mock.calls[0][0]).toMatchInlineSnapshot(
+      `
+      "Some stories were not cleaned up before rendering 'Unnamed Story (firstArg, secondArg)'.
+
+      You should load the story with \`await Story.load()\` before rendering it.
+      See https://storybook.js.org/docs/api/portable-stories-vitest#3-load for more information."
+    `
+    );
+  });
+
   it('should throw an error if Story is undefined', () => {
     expect(() => {
       // @ts-expect-error (invalid input)