-
-
Notifications
You must be signed in to change notification settings - Fork 988
Request for comments / direction: Add types into repo and type checking without converting to typescript #1031
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
Draft
raphendyr
wants to merge
2
commits into
expressjs:v2
Choose a base branch
from
raphendyr:add-types-and-type-checking
base: v2
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,106 @@ | ||
export interface CookieOptions { | ||
/** | ||
* Specifies the number (in milliseconds) to use when calculating the `Expires Set-Cookie` attribute. | ||
* This is done by taking the current server time and adding `maxAge` milliseconds to the value to calculate an `Expires` datetime. By default, no maximum age is set. | ||
* | ||
* If both `expires` and `maxAge` are set in the options, then the last one defined in the object is what is used. | ||
* `maxAge` should be preferred over `expires`. | ||
* | ||
* @see expires | ||
*/ | ||
maxAge?: number | undefined; | ||
|
||
/** | ||
* Specifies the `boolean` value for the [`Partitioned` `Set-Cookie`](https://tools.ietf.org/html/draft-cutler-httpbis-partitioned-cookies/) | ||
* attribute. When truthy, the `Partitioned` attribute is set, otherwise it is not. | ||
* By default, the `Partitioned` attribute is not set. | ||
* | ||
* **Note** This is an attribute that has not yet been fully standardized, and may | ||
* change in the future. This also means many clients may ignore this attribute until | ||
* they understand it. | ||
*/ | ||
partitioned?: boolean | undefined; | ||
|
||
/** | ||
* Specifies the `string` to be the value for the [`Priority` `Set-Cookie` attribute](https://tools.ietf.org/html/draft-west-cookie-priority-00#section-4.1). | ||
* | ||
* - `'low'` will set the `Priority` attribute to `Low`. | ||
* - `'medium'` will set the `Priority` attribute to `Medium`, the default priority when not set. | ||
* - `'high'` will set the `Priority` attribute to `High`. | ||
* | ||
* More information about the different priority levels can be found in | ||
* [the specification](https://tools.ietf.org/html/draft-west-cookie-priority-00#section-4.1). | ||
* | ||
* **Note** This is an attribute that has not yet been fully standardized, and may change in the future. | ||
* This also means many clients may ignore this attribute until they understand it. | ||
*/ | ||
priority?: "low" | "medium" | "high" | undefined; | ||
|
||
signed?: boolean | undefined; | ||
|
||
/** | ||
* Specifies the `Date` object to be the value for the `Expires Set-Cookie` attribute. | ||
* By default, no expiration is set, and most clients will consider this a "non-persistent cookie" and will delete it on a condition like exiting a web browser application. | ||
* | ||
* If both `expires` and `maxAge` are set in the options, then the last one defined in the object is what is used. | ||
* | ||
* @deprecated The `expires` option should not be set directly; instead only use the `maxAge` option | ||
* @see maxAge | ||
*/ | ||
expires?: Date | null | undefined; | ||
|
||
/** | ||
* Specifies the boolean value for the `HttpOnly Set-Cookie` attribute. When truthy, the `HttpOnly` attribute is set, otherwise it is not. | ||
* By default, the `HttpOnly` attribute is set. | ||
* | ||
* Be careful when setting this to `true`, as compliant clients will not allow client-side JavaScript to see the cookie in `document.cookie`. | ||
*/ | ||
httpOnly?: boolean | undefined; | ||
|
||
/** | ||
* Specifies the value for the `Path Set-Cookie` attribute. | ||
* By default, this is set to '/', which is the root path of the domain. | ||
*/ | ||
path?: string | undefined; | ||
|
||
/** | ||
* Specifies the value for the `Domain Set-Cookie` attribute. | ||
* By default, no domain is set, and most clients will consider the cookie to apply to only the current domain. | ||
*/ | ||
domain?: string | undefined; | ||
|
||
/** | ||
* Specifies the boolean value for the `Secure Set-Cookie` attribute. When truthy, the `Secure` attribute is set, otherwise it is not. By default, the `Secure` attribute is not set. | ||
* Be careful when setting this to true, as compliant clients will not send the cookie back to the server in the future if the browser does not have an HTTPS connection. | ||
* | ||
* Please note that `secure: true` is a **recommended option**. | ||
* However, it requires an https-enabled website, i.e., HTTPS is necessary for secure cookies. | ||
* If `secure` is set, and you access your site over HTTP, **the cookie will not be set**. | ||
* | ||
* The cookie.secure option can also be set to the special value `auto` to have this setting automatically match the determined security of the connection. | ||
* Be careful when using this setting if the site is available both as HTTP and HTTPS, as once the cookie is set on HTTPS, it will no longer be visible over HTTP. | ||
* This is useful when the Express "trust proxy" setting is properly setup to simplify development vs production configuration. | ||
* | ||
* If you have your node.js behind a proxy and are using `secure: true`, you need to set "trust proxy" in express. Please see the [README](https://github.com/expressjs/session) for details. | ||
* | ||
* Please see the [README](https://github.com/expressjs/session) for an example of using secure cookies in production, but allowing for testing in development based on NODE_ENV. | ||
*/ | ||
secure?: boolean | "auto" | undefined; | ||
|
||
encode?: ((val: string) => string) | undefined; | ||
|
||
/** | ||
* Specifies the boolean or string to be the value for the `SameSite Set-Cookie` attribute. | ||
* - `true` will set the `SameSite` attribute to `Strict` for strict same site enforcement. | ||
* - `false` will not set the `SameSite` attribute. | ||
* - `lax` will set the `SameSite` attribute to `Lax` for lax same site enforcement. | ||
* - `none` will set the `SameSite` attribute to `None` for an explicit cross-site cookie. | ||
* - `strict` will set the `SameSite` attribute to `Strict` for strict same site enforcement. | ||
* | ||
* More information about the different enforcement levels can be found in the specification. | ||
* | ||
* **Note:** This is an attribute that has not yet been fully standardized, and may change in the future. | ||
* This also means many clients may ignore this attribute until they understand it. | ||
*/ | ||
sameSite?: boolean | "lax" | "strict" | "none" | undefined; | ||
} |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if this is used by external Store implementations. In this repo, this was used only by the test code. The version in
index.js
callscookie.serialize
directly on purpose or by accident.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First commit removed this method, but the second is more about just adding types and converting to a class, hence this method is back. The above comment still applies, and this method probably could be removed.