storybookjs / storybook

Storybook is the industry standard workshop for building, documenting, and testing UI components in isolation
https://storybook.js.org
MIT License
84.61k stars 9.31k forks source link

[Bug]: invariant expected app router to be mounted #24722

Closed dfd1123 closed 9 months ago

dfd1123 commented 1 year ago

Describe the bug

The component that imports next/navigation and uses useRouter generates an error saying 'Error: invariant expected app router to be mounted'. I am using storybook version 7.5.2 and next14 version. And I also set nextjs: { appDirectory: true } in the parameters.

However, the error does not go away and continues to occur. Did I possibly do something wrong??

스크린샷 2023-11-06 오전 10 38 18 스크린샷 2023-11-06 오전 10 39 12 스크린샷 2023-11-06 오전 10 34 26

To Reproduce

No response

System

No response

Additional context

No response

felixmagnus commented 1 year ago

We're also facing this issue using storybook 7.5.2 and next 13.5.6.

We notice this behavior since the peer dependency @babel/core was upgraded from 7.23.0 (working) to 7.23.2 (not working).

Edit: after investigating further, the issue does not seem to be the upgrade of the babel-core version itself, but rather a mismatch between babel-core-versions between different packages, i.e. having one use 7.23.0 and the other using 7.23.2).

After installing 7.23.2 as our own dependency, this issue was resolved (we're using pnpm). --> We can use this as a temporary workaround, but would prefer this being investigated still

whateverneveranywhere commented 12 months ago

having this issue with next ^14.0.1. but unlike @felixmagnus, switching to ^7.23.2 for babel/core didn't help me solve the issue. image

valentinpalkovic commented 12 months ago

Can someone provide a reproduction?

whateverneveranywhere commented 12 months ago

Can someone provide a reproduction?

Found the issue, it was on my side, so in my case, while on "next": "^14.0.1",: after adding appDirectory: true to the parameters, it worked as expected.

const meta = {
    title: 'Auth/Register',
    component: Register,
    decorators: [
        (Story) => (
            <StorybookLayout>
                <Story />
            </StorybookLayout>
        ),
    ],
    parameters: {
        layout: 'centered',
+              nextjs: {
+                  appDirectory: true,
+              },
    },
    tags: ['autodocs'],
} satisfies Meta<typeof Register>;`
valentinpalkovic commented 12 months ago

@dfd1123 Can you remove the render functions and the template? For these simple argument mapping it is not necessary. Storybook does this under the hood. Second, do you have some custom add-ons or decorators in place?

mecaota commented 11 months ago

@valentinpalkovic

I'll put a reproduction below.I'm also running into the same issue and it's really bothering me.

https://github.com/mecaota/bugged-nextjs/tree/7122c366fa9bfb465a6b8114875d258b48cb66a0

isBatak commented 9 months ago

Is there any workarounds for this issue?

EDIT: Here is the CodeSandbox reproduction link https://codesandbox.io/p/devbox/storybook-next-usequerystate-issue-wnytw2?file=%2Fsrc%2FButton%2FButton.tsx%3A14%2C42

MiroslavPetrik commented 9 months ago

Can someone provide a reproduction?

Found the issue, it was on my side, so in my case, while on "next": "^14.0.1",: after adding appDirectory: true to the parameters, it worked as expected.

const meta = { title: 'Auth/Register', component: Register, decorators: [ (Story) => ( <StorybookLayout> <Story /> </StorybookLayout> ), ], parameters: { layout: 'centered', nextjs: { appDirectory: true, }, }, tags: ['autodocs'], } satisfies Meta<typeof Register>;

I approve this message.

MichaelCasaDev commented 9 months ago

Any updates on that? I've also tried using the nextjs: { appDirectory: true, } parameter without any success... I'm using the latest nextjs version (14.1.0) and latest storybook (7.6.10)

MichaelCasaDev commented 9 months ago

For everyone having this issue, I've found a workaround using the next-router-mock package!

import { AppRouterContext } from "next/dist/shared/lib/app-router-context.shared-runtime";
import mockRouter from "next-router-mock";

const withRouter: Decorator = (Story, context) => (
  <AppRouterContext.Provider value={mockRouter as any}>
    <Story />
  </AppRouterContext.Provider>
);
isBatak commented 9 months ago

@MichaelCasaDev thx for the workaround. Great idea! I didn't even need the next-router-mock package. Also, next-router-mock only supports pages router, which means that AppRouterInstance is different form mockRouter instance and it feels wrong to pass it to the AppRouterContext,

import {
    AppRouterContext,
    type AppRouterInstance,
} from 'next/dist/shared/lib/app-router-context.shared-runtime';

const meta = {
    decorators: [
        (Story) => (
            <AppRouterContext.Provider value={{} as AppRouterInstance}>
                <Story />
            </AppRouterContext.Provider>
        ),
    ],
};
valentinpalkovic commented 9 months ago

@mecaota Your reproduction does not set up the parameters.nextjs.appDirectory: true. After setting up the parameter correctly, it works. Please follow the documentation here: https://github.com/storybookjs/storybook/blob/main/code/frameworks/nextjs/README.md#nextjs-navigation

This documentation will also be available soon on the official Storybook docs.

valentinpalkovic commented 9 months ago

@isBatak Your reproduction doesn't bootstrap. But I can see that you are not setting parameters.nextjs.appDirectory: true. That's the reason why it is not working.

@isBatak, @MichaelCasaDev The AppRouterContext.Provider doesn't have to be added manually. Storybook does that for you in the case where parameters.nextjs.appDirectory: true is set as documented here: https://github.com/storybookjs/storybook/blob/main/code/frameworks/nextjs/README.md#nextjs-navigation

@MichaelCasaDev You are mentioning that parameters.nextjs.appDirectory: true doesn't help. Please provide a reproduction in that case.

I am closing this issue for now because it seems that the required parameter is not set accordingly. If parameters.nextjs.appDirectory: true is set as documented, and it is still not working; please let me know. In the meantime, I will close this issue because it was about misconfiguration.

isBatak commented 9 months ago

@valentinpalkovic please reopen this issue because your suggested solution does not work.

Please read my original issue https://github.com/storybookjs/storybook/issues/25612. What we need is to be able to use pages folder useRouter together with next/navigation useRouter in the same component. I can't just enable parameters.nextjs.appDirectory: true because then pages useRouter will not work.

valentinpalkovic commented 9 months ago

Can you point me to the Next.js documentation where it explicitly says that next/navigation can be used in the pages directory? When I go to the https://nextjs.org/docs/pages/building-your-application/routing/linking-and-navigating, it is written to use next/router instead. I don't understand which kind of wild tricks next-usequerystate/nuqs does (importing next/navigation.js instead of next/navigation), but it seems to be a hack rather than something officially supported by Next.js

isBatak commented 9 months ago

@valentinpalkovic I can't find any documentation but I can prove that pages folder actually have access to AppRouterContext, check it here https://stackblitz.com/edit/nextjs-3vyr8e?file=app%2Fpage.tsx,pages%2Ftest.tsx

I would say it's there for easier migration form pages to app folder. If you can mimic that in storybook it would fix the issue.

To clarify:

I don't see why would this be a problem.

sinashahoveisi commented 8 months ago

@MichaelCasaDev thx for the workaround. Great idea! I didn't even need the next-router-mock package. Also, next-router-mock only supports pages router, which means that AppRouterInstance is different form mockRouter instance and it feels wrong to pass it to the AppRouterContext,

import {
  AppRouterContext,
  type AppRouterInstance,
} from 'next/dist/shared/lib/app-router-context.shared-runtime';

const meta = {
  decorators: [
      (Story) => (
          <AppRouterContext.Provider value={{} as AppRouterInstance}>
              <Story />
          </AppRouterContext.Provider>
      ),
  ],
};

@isBatak This solution worked for me, thanks for your solution

weiying-chen commented 6 months ago

parameters.nextjs.appDirectory set to true didn't fix Error: invariant expected app router to be mounted.

The code of @MichaelCasaDev and @isBatak did:

import {
    AppRouterContext,
    type AppRouterInstance,
} from 'next/dist/shared/lib/app-router-context.shared-runtime';

const meta = {
    decorators: [
        (Story) => (
            <AppRouterContext.Provider value={{} as AppRouterInstance}>
                <Story />
            </AppRouterContext.Provider>
        ),
    ],
};
alvis commented 6 months ago

parameters.nextjs.appDirectory set to true didn't fix Error: invariant expected app router to be mounted.

The code of @MichaelCasaDev and @isBatak did:

import {
  AppRouterContext,
  type AppRouterInstance,
} from 'next/dist/shared/lib/app-router-context.shared-runtime';

const meta = {
  decorators: [
      (Story) => (
          <AppRouterContext.Provider value={{} as AppRouterInstance}>
              <Story />
          </AppRouterContext.Provider>
      ),
  ],
};

same here, but it worked before. Seems like some version change break the thing under some circumstances

CarlosLeonCode commented 1 month ago

@MichaelCasaDev thx for the workaround. Great idea! I didn't even need the next-router-mock package. Also, next-router-mock only supports pages router, which means that AppRouterInstance is different form mockRouter instance and it feels wrong to pass it to the AppRouterContext,

import {
    AppRouterContext,
    type AppRouterInstance,
} from 'next/dist/shared/lib/app-router-context.shared-runtime';

const meta = {
    decorators: [
        (Story) => (
            <AppRouterContext.Provider value={{} as AppRouterInstance}>
                <Story />
            </AppRouterContext.Provider>
        ),
    ],
};

@isBatak This solution worked for me, thanks for your solution

thnks, that works for me.