From b309c94dede25f32d61f97286091f7bfeb902ea2 Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Sat, 4 May 2024 20:52:16 +0200 Subject: [PATCH] Revert "Nextjs: Implement next redirect and the RedirectBoundary" --- .../src/export-mocks/navigation/index.ts | 39 +++---- .../nextjs/src/fastRefresh/webpack.ts | 9 +- code/frameworks/nextjs/src/preview.tsx | 34 ------ .../nextjs/src/routing/decorator.tsx | 10 +- .../Redirect.stories.tsx | 54 --------- .../ServerActions.stories.tsx | 107 ++++++------------ code/lib/react-dom-shim/src/react-18.tsx | 10 +- code/renderers/react/src/renderToCanvas.tsx | 2 +- 8 files changed, 63 insertions(+), 202 deletions(-) delete mode 100644 code/frameworks/nextjs/template/stories_nextjs-default-ts/Redirect.stories.tsx diff --git a/code/frameworks/nextjs/src/export-mocks/navigation/index.ts b/code/frameworks/nextjs/src/export-mocks/navigation/index.ts index f55ee86c36ca..dd9e9a692e6f 100644 --- 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 index f9bb9d6c51e7..83e91518383d 100644 --- 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 index 9a46a56c6918..8f141e7b1ef0 100644 --- 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[] = [ 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 index 6c3f66ba6778..4979b6e3b08a 100644 --- 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. - */} - - - + ); } 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 deleted file mode 100644 index f76d8a7f5a90..000000000000 --- a/code/frameworks/nextjs/template/stories_nextjs-default-ts/Redirect.stories.tsx +++ /dev/null @@ -1,54 +0,0 @@ -import React from 'react'; -import type { Meta, StoryObj } from '@storybook/react'; -import { userEvent, within } from '@storybook/test'; -import { redirect } from 'next/navigation'; - -let state = 'Bug! Not invalidated'; - -export default { - render() { - return ( -
-
{state}
-
{ - state = 'State is invalidated successfully.'; - redirect('/'); - }} - > - -
-
- ); - }, - parameters: { - 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, - }, - nextjs: { - appDirectory: true, - navigation: { - pathname: '/', - }, - }, - }, -} as Meta; - -export const SingletonStateGetsInvalidatedAfterRedirecting: StoryObj = { - play: async ({ canvasElement, step }) => { - const canvas = within(canvasElement); - await userEvent.click(canvas.getByRole('button')); - }, -}; 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 index cb3f5bbdb2dd..17d364429726 100644 --- 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; -export const ProtectedWhileLoggedOut: StoryObj = { - 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 = { - 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 = { - beforeEach() { - cookies().set('user', 'storybookjs'); - }, - play: async ({ canvasElement }) => { +export const Default: StoryObj = { + 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 = { - 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/react-dom-shim/src/react-18.tsx b/code/lib/react-dom-shim/src/react-18.tsx index 5b9e88f98a84..ddfa738d4dd9 100644 --- a/code/lib/react-dom-shim/src/react-18.tsx +++ b/code/lib/react-dom-shim/src/react-18.tsx @@ -1,6 +1,6 @@ import type { FC, ReactElement } from 'react'; +import type { Root as ReactRoot } from 'react-dom/client'; import React, { useLayoutEffect, useRef } from 'react'; -import type { Root as ReactRoot, RootOptions } from 'react-dom/client'; import ReactDOM from 'react-dom/client'; // A map of all rendered React 18 nodes @@ -21,9 +21,9 @@ const WithCallback: FC<{ callback: () => void; children: ReactElement }> = ({ return children; }; -export const renderElement = async (node: ReactElement, el: Element, rootOptions?: RootOptions) => { +export const renderElement = async (node: ReactElement, el: Element) => { // Create Root Element conditionally for new React 18 Root Api - const root = await getReactRoot(el, rootOptions); + const root = await getReactRoot(el); return new Promise((resolve) => { root.render( resolve(null)}>{node}); @@ -39,11 +39,11 @@ export const unmountElement = (el: Element, shouldUseNewRootApi?: boolean) => { } }; -const getReactRoot = async (el: Element, rootOptions?: RootOptions): Promise => { +const getReactRoot = async (el: Element): Promise => { let root = nodes.get(el); if (!root) { - root = ReactDOM.createRoot(el, rootOptions); + root = ReactDOM.createRoot(el); nodes.set(el, root); } diff --git a/code/renderers/react/src/renderToCanvas.tsx b/code/renderers/react/src/renderToCanvas.tsx index 8cb2e76f9b2d..d8821a3458e4 100644 --- a/code/renderers/react/src/renderToCanvas.tsx +++ b/code/renderers/react/src/renderToCanvas.tsx @@ -74,7 +74,7 @@ export async function renderToCanvas( unmountElement(canvasElement); } - await renderElement(element, canvasElement, storyContext?.parameters?.react?.rootOptions); + await renderElement(element, canvasElement); return () => unmountElement(canvasElement); }