connected-next-router icon indicating copy to clipboard operation
connected-next-router copied to clipboard

Feature request: createMatchSelector to replace connected-react-router

Open Guneetgstar opened this issue 4 years ago • 8 comments

I am migrating from client side rendering to next.js for SSR. Although connected-next-router is supposed to replace connected-react-router (with the basic analogical name) it does not include a very basic functionality to match to opponent. I haven't covered everything thats missing but found that this can be included in the library itself very easily to fast line the migration process.

Guneetgstar avatar Oct 31 '20 21:10 Guneetgstar

@Guneetgstar I think it's a good idea to add the same selectors connected-react-router has, would you be interested in opening a PR?

danielr18 avatar Oct 31 '20 22:10 danielr18

@danielr18 Yeah I think I can do it. But currently I am stuck at how to change redux state using next/link API. Do you have a workaround?

Guneetgstar avatar Oct 31 '20 23:10 Guneetgstar

Oh, I fixed it, that was a tiny configuration mistake. There was a wrong router reducer set.

Guneetgstar avatar Nov 01 '20 18:11 Guneetgstar

Hi @danielr18 I looked further with the API and found that this repo does not give a porting option for connected-react-router users to next project as it not only lacks features that I just mentioned above (selector functions) but also doesn't have the same API for the very basic CallHistoryMethodAction like push and replace. Ex: This would break in connected-next-router:-

 replace({pathname: `${paths.PRODUCTS}/${product.category}`, search: `?id=${id}`})

Here you mentioned the react-router but it supports this sort of URL de-structuring as well.

In fact the whole implementation is limited to similar behaviour only and a developer just cant include connected-next-router and remove connected-react-router where ever used as a migration script as it would involve a lot of breaking changes after that. To make it more useful we should provide an easy migration guide or at least include the caveats in the README.md

Guneetgstar avatar Nov 04 '20 00:11 Guneetgstar

You are right, it would be good to document those differences in a migration guide. The good this is Next.js 10 has automatic href resolving, so the as param wouldn't need to be added while migrating dynamic routes to pages.

danielr18 avatar Nov 04 '20 00:11 danielr18

Thanks for the mention about nextjsv10 feature I can now the above example works like:

replace(
               `${paths.PRODUCTS}/[[...slug]]/`,
               `${paths.PRODUCTS}/${product.category}?${new URLSearchParams({id:id})}`
 )

But I still see that replace function only works with string parameters not Url or UrlObject and I think I should open a new issue for it.

Guneetgstar avatar Nov 04 '20 01:11 Guneetgstar

Hi @danielr18 again, although I really wanted to contribute, this project is maintained in TypeScript and I know very less about it and I guess it would be a easy task for you to make a PR for what we have discussed.

Guneetgstar avatar Nov 04 '20 12:11 Guneetgstar

Please check the PR.

Guneetgstar avatar Nov 04 '20 12:11 Guneetgstar