search

matthewmueller / next-cookies

Tiny little function for getting cookies on both client & server with next.js.
368 stars 17 forks source link

Add deprecation notice due to Next.js 10.0.4 having cookies in `getServerSideProps`? #38

Closed karlhorky closed 3 years ago

karlhorky commented 3 years ago

Hi there, thanks for this package! I've been happily using it and also teaching with it in the @upleveled bootcamp.

It seems like next@10.0.4 has support for req.cookies without any library: https://github.com/vercel/next.js/pull/19724

Would you consider deprecating this library and referencing the changes in the new version?

shunkakinoki commented 3 years ago

This seems like a good idea!

nfriedly commented 3 years ago

Hey, sorry this slipped through the cracks.

From a quick glance, it appears that next.js only provides the cookies during server-side-rendering, whereas this plugin provides them in both client and server code. So, unless I missed something, I don't think it makes sense to deprecate this module just yet.

karlhorky commented 3 years ago

Oh, how would it be used for getting cookies in client-side rendering?

nfriedly commented 3 years ago 
const allCookies = cookies(ctx);

It's the exact same API for client and server, that's kind of the point of this library.

karlhorky commented 3 years ago

I meant to ask about a use case for this... so, where and why would this code be used on the client?

With the getServerSideProps and getStaticProps APIs, the context is a server-side or build-time concept, is it not?

karlhorky commented 3 years ago

Or are you talking about the older getInitialProps, which is recommended to avoid in the docs?

nfriedly commented 3 years ago

You should really take a look at the readme, I think it would clarify things for you.

The ctx object is passed to your getInitialProps function by next.js.

This lib doesn't really make sense with getServerSideProps because it's for using the same code on client and server, whereas getServerSideProps is server side only.

Sometimes you just want to read the cookie value and render the page without caring about if it's on the client or on the server. That's when this library is useful.

karlhorky commented 3 years ago

I did look at the readme, it seems to do with the older getInitialProps.

What this issue is about is the currently recommended style of Next.js - getServerSideProps and getStaticProps.

It doesn't seem that this library adds anything there.

Now that ctx includes cookies in the req object, anywhere you have a ctx, you'll already have the cookies (via ctx.req.cookies).

So I don't see what your point is here.

nfriedly commented 3 years ago

I feel like we're going in circles here. Let's just say that this library isn't a good fit for your needs and leave it at that.

karlhorky commented 3 years ago

Yeah, I was just asking for a demo of how client-side context would make sense with the recommended getServerSideProps and getStaticProps APIs.

But no worries, if this library is intended for the older getInitialProps, that's fine too!

My main point is that this could be pointed out in the readme, so that users are directed towards recommended paths, instead of sticking with older APIs (which could eventually be deprecated) :)

But seems like I can't convince the maintainers here, so all good.