JesusTheHun / storybook-addon-remix-react-router

Use your app router in your stories. A decorator made for Remix React Router and Storybook
Apache License 2.0
47 stars 11 forks source link

Not working with nested `<Routes>` #8

Closed lifeiscontent closed 1 year ago

lifeiscontent commented 1 year ago

If I render a component with nested <Routes> the story doesn't render.

example: https://stackblitz.com/edit/react-ts-miwcgv?file=App.tsx

JesusTheHun commented 1 year ago

Can you try version 0.1.13 and tell me if it works?

lifeiscontent commented 1 year ago

@JesusTheHun nope, that doesn't work.

lifeiscontent commented 1 year ago

I think the issue is here: https://github.com/JesusTheHun/storybook-addon-react-router-v6/blob/main/src/components/StoryRouter.tsx#L20

the path is generated, but there's no way to modify the root route path. the entry needs to point to /:id/:subId where the root route needs to point /:id/* and there's no way to currently specify both

JesusTheHun commented 1 year ago

You can wrap things in your story template :

export default {
    title: 'Example/Repro',
    component: ListingDetailPage,
    decorators: [withRouter],
};

const Template = (args) => (
    <Routes>
        <Route index element={<Link to="/1">Goto Id</Link>} />
        <Route path=":id/*" element={<ListingDetailPage />} />
    </Routes>
);

export const Default = Template.bind({});
Default.parameters = {
    reactRouter: {
        routePath: '*',
    }
}
lifeiscontent commented 1 year ago

@JesusTheHun that wouldn't render the ListingDetailPage directly (including the parent layout)

JesusTheHun commented 1 year ago

Quick update : I'm currently working on this feature. ETA : this week

JesusTheHun commented 1 year ago

@lifeiscontent I've published version 0.2.1 which includes support for descendant routes.

Can you give it a try ? I've added some instructions in the readme ;)

It took some time because I wanted to still be able to provides route params for descendant routes, something that react router does not provide so I had to hackaround. Check the new routeMatches property in the log panel.

Let me know if you face any difficulties or have any question.

lifeiscontent commented 1 year ago

@JesusTheHun almost! using the /* renders the page, but :subId is not defined in params, instead it has the correct value but the param is * instead of :subId

lifeiscontent commented 1 year ago

also, not sure if its helpful, but its common to publish .rc releases for features that are in flux, not sure if you knew about rc releases, but just wanted to call it out in case you didn't / find it useful.

lifeiscontent commented 1 year ago

Oh, actually it's working as expected, I just had my route param setup incorrect. Oops! 😅

JesusTheHun commented 1 year ago

Thanks for your feedback. Do you think routeMatches is confusing ? Shall I make the routeParams display the params of all routes instead of using the routeMatches property ? Because right now the routeParams only show the params of the route declared at routePath. So this change would unify the behaviour ; maybe we don't make it an array if there is only one matching route.

// this version would display all params available inside the route's element
routeParams = [
  '/listing/*': { *: '1/1' },
  '/:id': { id: 1 },
  '/:subId': { id: 1, subId: 1 },
]

// display only additional params
routeParams = [
  '/listing/*': { *: '1/1' },
  '/:id': { id: 1 },
  '/:subId': { subId: 1 },
]

// we try to be smart (dangerous)
routeParams = [
  // '/listing/*': { *: '1/1' }, # we detect no dynamic param and a wildcard so we don't show this entry
  '/:id': { id: 1 },
  '/:subId': { subId: 1 },
]