Remove Next from peer dependencies

[jfranciscosousa]

No description provided.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
react-url-modal	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Sep 23, 2023 4:15pm

[github-actions]

size-limit report 📦

Path	Size
dist/react-url-modal.cjs.js	2.56 KB (-9.39% 🔽)
dist/react-url-modal.esm.js	2.63 KB (-5.75% 🔽)
dist/Modal/index.js	10.5 KB (+0.32% 🔺)

[jfranciscosousa]

@donaminos this is a work in progress, but what do you think? This gets rid of any peer dependencies, which are a pain. It's not really possible to have optional deps nowadays as most bundlers statically analyse all imports, which cause all sorts of problems to our consumers that are not using next. I was only able to use this without next on a webpack project. vite, esbuild all blown up without the peer dependency of Next.

[donaminos]

@donaminos this is a work in progress, but what do you think? This gets rid of any peer dependencies, which are a pain. It's not really possible to have optional deps nowadays as most bundlers statically analyse all imports, which cause all sorts of problems to our consumers that are not using next. I was only able to use this without next on a webpack project. vite, esbuild all blown up without the peer dependency of Next.

Thanks a lot @jfranciscosousa for looking into this. I am a bit short on time this week but I will try my best to review it before the end of the week

[donaminos]

@jfranciscosousa thanks a lot for looking into this. I like this approach but something is not clear in my mind. Why do you want to add an optional dependency for Next?

The idea is to use the browser API by default and the consumer should pass a customer implementation if he/she wants to use a router from a framework. isn't it? did I misunderstand this PR?

[jfranciscosousa]

@donaminos I think you are misunderstanding the PR. I'm removing Next.js from the optional dependencies. Currently we have to add next.js as a peerDep to this package. Our platform uses the next adapter from this package, not the browser API, because it's unreliable when using other client-side routers.

We are using an adapter pattern for this package (default or next.js). I'm instead replacing it with full control. Instead of providing an adapter, we can let consumers of this package customize customRouterAction.

This way we have no need of maintaining peer dependencies, and our consumers can use the package as they wish.

For our specific use case on our platform, we would have to use a customRouterAction that uses next/router replace and push. There's examples on the documentation of this PR.

Btw, for some reason Github completely ignored the description I wrote for this PR. I'll write it later when I get back from PTO.

[jfranciscosousa]

This was what we had before @donaminos

[jfranciscosousa]

And it's what our platform uses.

[jfranciscosousa]

Any user trying to use this package without Next.js was going to get a build error as optionalDeps are not a thing on modern JS bundlers.

[jfranciscosousa]

Just check the existing issues for this project.

[donaminos]

@donaminos I think you are misunderstanding the PR. I'm removing Next.js from the optional dependencies. Currently we have to add next.js as a peerDep to this package. Our platform uses the next adapter from this package, not the browser API, because it's unreliable when using other client-side routers.

We are using an adapter pattern for this package (default or next.js). I'm instead replacing it with full control. Instead of providing an adapter, we can let consumers of this package customize customRouterAction.

This way we have no need of maintaining peer dependencies, and our consumers can use the package as they wish.

For our specific use case on our platform, we would have to use a customRouterAction that uses next/router replace and push. There's examples on the documentation of this PR.

Btw, for some reason Github completely ignored the description I wrote for this PR. I'll write it later when I get back from PTO.

Thanks a lot for this clear explanation. It makes total sense 👍