Fix problematic type comments #284
Conversation
Signed-off-by: Raku Zeta <[email protected]>
|
If the comments are incorrect, could you fix them instead of removing? |
Signed-off-by: Raku Zeta <[email protected]>
| /** One of the `Priority` string attributes (`low`, `medium` or `high`) specifying a retention priority for HTTP cookies that will be respected by user agents during cookie eviction. */ | ||
| priority?: 'low' | 'medium' | 'high'; | ||
| /** The `boolean` value of the `Secure` attribute. Set this option to false when communicating over an unencrypted (HTTP) connection. Value can be set to `auto`; in this case the `Secure` attribute will be set to false for HTTP request, in case of HTTPS it will be set to true. */ | ||
| /** Add the `Secure` attribute. Defaults to `false`. */ |
There was a problem hiding this comment.
"auto" is only available in CookieSerializeOptions, so it's moved there. 👀
| /** The `boolean` value of the `HttpOnly` attribute. Defaults to true. */ | ||
| /** Add the `HttpOnly` attribute. Defaults to `false`. */ | ||
| httpOnly?: boolean; | ||
| /** A `number` in seconds that specifies the `Expires` attribute by adding the specified seconds to the current date. If both `expires` and `maxAge` are set, then `expires` is used. */ |
There was a problem hiding this comment.
That's not what the implementation does.
It simply sets the Max-Age attribute with the provided value.
| maxAge?: number; | ||
| /** A `boolean` indicating whether the cookie is tied to the top-level site where it's initially set and cannot be accessed from elsewhere. */ | ||
| partitioned?: boolean; | ||
| /** The `Path` attribute. Defaults to `/` (the root path). */ |
There was a problem hiding this comment.
There's no default value. It simply doesn't set the Path attribute if this option is not provided.
| sameSite?: 'lax' | 'none' | 'strict' | boolean; | ||
| /** One of the `Priority` string attributes (`low`, `medium` or `high`) specifying a retention priority for HTTP cookies that will be respected by user agents during cookie eviction. */ | ||
| priority?: 'low' | 'medium' | 'high'; | ||
| /** The `boolean` value of the `Secure` attribute. Set this option to false when communicating over an unencrypted (HTTP) connection. Value can be set to `auto`; in this case the `Secure` attribute will be set to false for HTTP request, in case of HTTPS it will be set to true. Defaults to true. */ |
There was a problem hiding this comment.
Set this option to false when communicating over an unencrypted (HTTP) connection.
This is unnecessary and confusing so therefore removed.
Value can be set to
auto; in this case theSecureattribute will be set to false for HTTP request, in case of HTTPS it will be set to true.
"auto" is only available in CookieSerializeOptions, not SerializeOptions. (moved to CookieSerializeOptions)
| /** The expiration `date` used for the `Expires` attribute. If both `expires` and `maxAge` are set, then `expires` is used. */ | ||
| /** The expiration `date` used for the `Expires` attribute. */ | ||
| expires?: Date; | ||
| /** The `boolean` value of the `HttpOnly` attribute. Defaults to true. */ |
There was a problem hiding this comment.
HttpOnly is an attribute without value, so I changed the wording.
| domain?: string; | ||
| /** Specifies a function that will be used to encode a cookie's value. Since value of a cookie has a limited character set (and must be a simple string), this function can be used to encode a value into a string suited for a cookie's value. */ | ||
| encode?(val: string): string; | ||
| /** The expiration `date` used for the `Expires` attribute. If both `expires` and `maxAge` are set, then `expires` is used. */ |
There was a problem hiding this comment.
That's not what the implementation does.
It simply sets both attributes with the corresponding values.
4 participants
A previous commit introduced some comments that didn't reflect the real implementation which may lead to unsafe assumptions of default behaviors:
04ea01f#diff-0647ab7efe94459b8666eed6d1660236e363af58bb3fa7eda85041a316c97bb2R108-R126
This PR fixes that.
Checklist
npm run testandnpm run benchmarkand the Code of conduct