Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #107 : Reactivestack typings (cookies) #124

Merged
merged 4 commits into from
Feb 17, 2018
Merged

Conversation

Guymestef
Copy link

Hi,
I've created a first version of the typings.
Any modification needed? @eXon @PeterKottas

@PeterKottas
Copy link

Looking good as far as I can tell. Glad to be of help ;) I am using these atm for a real app and so far no problems.

httpOnly?: boolean;
}

export type ReactCookieProps = {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my library, react-cookie-banner, I had a problem with this part as we declared the prop cookies as optional with a default to new Cookies().

export type Cookie = string;

export class Cookies {
get: (key: string, options?: ReactCookieGetOptions) => Cookie;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, I think this should return Cookie | undefined as:

new Cookies().get('foo') // => undefined

@eXon
Copy link
Collaborator

eXon commented Dec 13, 2017

This is awesome! We might as well publish TS typing for universal-cookie Cookies class. I think someone posted it on the issue.

@eXon
Copy link
Collaborator

eXon commented Dec 13, 2017

Oh I just saw it is included but within react-cookie only. Would it be possible to add it as well to the universal-cookie library?

@FrancescoCioria
Copy link

We might as well publish TS typing for universal-cookie Cookies class. I think someone posted it on the issue.

@eXon I think it's already been implemented: aren't these ones the typings for the Cookies class?

@inunotaisho
Copy link

inunotaisho commented Dec 17, 2017

@Guymestef , @eXon Would be willing to help to. Could use the experience.

@eXon eXon merged commit 2575d01 into bendotcodes:master Feb 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants