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 problematic type comments #284

Merged
merged 2 commits into from
May 15, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions types/plugin.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,25 +115,26 @@ declare namespace fastifyCookie {
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. */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's not what the implementation does.
It simply sets both attributes with the corresponding values.

/** The expiration `date` used for the `Expires` attribute. */
expires?: Date;
/** The `boolean` value of the `HttpOnly` attribute. Defaults to true. */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

HttpOnly is an attribute without value, so I changed the wording.

/** 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. */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's not what the implementation does.
It simply sets the Max-Age attribute with the provided value.

/** A `number` in seconds that specifies the `Max-Age` attribute. */
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). */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's no default value. It simply doesn't set the Path attribute if this option is not provided.

/** The `Path` attribute. */
path?: string;
/** A `boolean` or one of the `SameSite` string attributes. E.g.: `lax`, `none` or `strict`. */
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. */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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 the Secure attribute 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)

/** Add the `Secure` attribute. Defaults to `false`. */
Copy link
Contributor

Choose a reason for hiding this comment

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

How does this improve it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"auto" is only available in CookieSerializeOptions, so it's moved there. 👀

secure?: boolean;
}

export interface CookieSerializeOptions extends Omit<SerializeOptions, 'secure'> {
/** Add the `Secure` attribute. Value can be set to `"auto"`; in this case the `Secure` attribute will only be added for HTTPS requests. Defaults to `false`. */
secure?: boolean | 'auto';
signed?: boolean;
}
Expand Down