feat: query cookie by name #128
Conversation
Updated CHANGELOG.md with pull request reference.
There was a problem hiding this comment.
Hi @criticalbh Thank you for this PR! Small request: could you please also update the docs/api.md and API TOC in /README.md?
@joelgriffith @timsuchanek @schickling in this case, should we rename cookiesGet() to cookies()?
|
Yes, I will wait for decision if method should be renamed. |
|
Yes, I agree on |
| @@ -11,13 +11,14 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. | |||
| - CODE_OF_CONDUCT.md, CONTRIBUTING.md | |||
| - `getHtml()` and `setHtml()` APIs. [#112](https://github.com/graphcool/chromeless/pull/112), [#74](https://github.com/graphcool/chromeless/issues/74) | |||
| - `mousedown(selector: string): Chromeless<T>` and `mouseup(selector: string): Chromeless<T>` APIs [#118](https://github.com/graphcool/chromeless/pull/118) @criticalbh | |||
| - `cookiesGet(name: string)` [#128](https://github.com/graphcool/chromeless/pull/128) API @criticalbh | |||
There was a problem hiding this comment.
Small change: method name is cookie it looks (and doesn't take any args)
There was a problem hiding this comment.
as I could understand, if we want to take all cookies, we will use cookies getter, but if we are going to query via name or query object than we would use method cookiesGet(name | query)?
| @@ -189,25 +189,28 @@ export default class Chromeless<T extends any> implements Promise<T> { | |||
| /** | |||
| * Get the cookies for the current url | |||
| */ | |||
There was a problem hiding this comment.
I'm having a hard time understanding how this all works here (and even before). @timsuchanek or @adieuadieu I'd love to hear your feedback on this
There was a problem hiding this comment.
any update on this?
|
So, thinking about this some more, I think this would makes sense:
@adieuadieu does that sound reasonable to you? |
| @@ -416,14 +416,14 @@ console.log(html) // <html><head></head><body><h1>Hello world!</h1></body></html | |||
|
|
|||
| <a name="api-cookiesget" /> | |||
|
|
|||
| ### cookiesGet(): Chromeless<Cookie[] | null> | |||
| ### get cookies(): Chromeless<Cookie[] | null> | |||
There was a problem hiding this comment.
I guess we don't need the word "get" here
|
@joelgriffith yea that sounds good. So to reiterate, the various cookie methods become: Cookies for the current URL:
Also need to update this one: All the cookies the browser has stored. |
Hello,
Here is the implementation of get cookie by name.
I know that in the API spec
cookiesGet(name: string)should returnCookie | nullbut in different paths it is possible to have same name so I implemented it to returnCookie[] | null. However I would like to hear opinions from you and make changes on this PR.Thanks,
Admir