feat(cookies): make `RequestCookies` and `ResponseCookies` more spec compliant by balazsorban44 · Pull Request #177 · vercel/edge-runtime

balazsorban44 · 2022-10-20T19:42:21Z

Changes based on the conversation in this Slack thread.

Spec: https://wicg.github.io/cookie-store

Notable breaking changes:

ResponseCookies#get has been renamed to ResponseCookies#getValue
ResponseCookies#getWithOptions has been renamed to ResponseCookies#get

…compliant

changeset-bot · 2022-10-20T19:42:23Z

🦋 Changeset detected

Latest commit: b471e97

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@edge-runtime/cookies	Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

vercel · 2022-10-20T19:46:29Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated
edge-runtime	✅ Ready (Inspect)	Visit Preview	Oct 21, 2022 at 3:08PM (UTC)

codecov-commenter · 2022-10-20T19:49:40Z

Codecov Report

Merging #177 (542f476) into main (94ad6e1) will decrease coverage by 2.94%.
The diff coverage is 84.31%.

❗ Current head 542f476 differs from pull request most recent head b16a7f7. Consider uploading reports for the commit b16a7f7 to get more accurate results

@@            Coverage Diff             @@
##             main     #177      +/-   ##
==========================================
- Coverage   87.91%   84.96%   -2.95%     
==========================================
  Files          14       21       +7     
  Lines        1704     1803      +99     
  Branches      213      181      -32     
==========================================
+ Hits         1498     1532      +34     
- Misses        192      251      +59     
- Partials       14       20       +6

Impacted Files	Coverage Δ
packages/runtime/src/edge-runtime.ts	`94.11% <66.66%> (-2.92%)`	⬇️
packages/cookies/src/response-cookies.ts	`71.42% <71.42%> (ø)`
packages/runtime/src/server/body-streams.ts	`38.13% <72.41%> (-24.51%)`	⬇️
packages/jest-environment/src/index.ts	`97.95% <75.00%> (-2.05%)`	⬇️
packages/cookies/src/request-cookies.ts	`77.35% <77.35%> (ø)`
packages/runtime/src/server/create-handler.ts	`86.02% <79.16%> (-8.28%)`	⬇️
packages/cookies/src/serialize.ts	`94.73% <94.73%> (ø)`
packages/user-agent/src/index.ts	`97.72% <97.72%> (ø)`
jest.setup.js	`100.00% <100.00%> (ø)`
packages/cookies/src/cached.ts	`100.00% <100.00%> (ø)`
... and 14 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

balazsorban44 · 2022-10-20T19:50:10Z

+   * Uses {@link ResponseCookies.get} to return only the cookie value.
+   */
+  getValue(...args: [key: string] | [options: Cookie]): string | undefined {
+    return this.get(...args)?.value


Arguably, this method should be unnecessary:
.getValue("cookie") is the same as
.get("cookie")?.value

Breaking spec to save 2 characters might not be worth it.

Kikobeats · 2022-10-21T11:12:39Z

@@ -1,3 +1,3 @@
-export type { Options } from './serialize'
+export type { Cookie, CookieListItem } from './serialize'


CookieOptions looks more natural to me, although I can see this is a thing defined by the spec interface

It's essentially the entire parsed cookie, with name and value. Only wording, so I am OK with changing it, but in my mind Cookie made more sense.

Kikobeats · 2022-10-21T11:26:55Z

-  }
+  getAll(...args: [name: string] | [Cookie] | [undefined]) {
+    const all = Array.from(this.#parsed())
+    if (!args.length) {


Maybe a strict comparison could be desirable

args.length === 0

Because of TypeScript, this would show:

This condition will always return 'false' since the types '1' and '0' have no overlap.ts(2367)

Since we technically don't allow more than one argument, but the user might/will try. !args.length should be as safe, since args is always going to be an array.

Kikobeats

super great PR! just added some suggestion to help maintain this code for the future.

Schniz · 2022-10-23T07:45:34Z

 */
 export class RequestCookies {
-  private readonly headers: Headers
+  readonly #headers: Headers


the future is here and now

…and `ResponseCookies` (#41526) Ref: [Slack thread](https://vercel.slack.com/archives/C035J346QQL/p1666056382299069?thread_ts=1666041444.633059&cid=C035J346QQL), [docs update](https://github.com/vercel/front/pull/17090) Spec: https://wicg.github.io/cookie-store/ BREAKING CHANGE: Ref: vercel/edge-runtime#177, vercel/edge-runtime#181 ## Bug - [ ] Related issues linked using `fixes #number` - [ ] Integration tests added - [ ] Errors have a helpful link attached, see `contributing.md` ## Feature - [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. - [ ] Related issues linked using `fixes #number` - [ ] Integration tests added - [ ] Documentation added - [ ] Telemetry added. In case of a feature if it's used or not. - [ ] Errors have a helpful link attached, see `contributing.md` ## Documentation / Examples - [ ] Make sure the linting passes by running `pnpm lint` - [ ] The "examples guidelines" are followed from [our contributing doc](https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md)

…compliant (vercel#177) * feat(cookies): make `RequestCookies` and `ResponseCookies` more spec compliant * add changeset * mention breaking changes * test `get()?.value` * address review

balazsorban44 added 2 commits October 20, 2022 21:38

feat(cookies): make RequestCookies and ResponseCookies more spec …

84d6695

…compliant

add changeset

542f476

mention breaking changes

739b83a

vercel Bot deployed to Preview October 20, 2022 19:47 View deployment

test get()?.value

b16a7f7

vercel Bot deployed to Preview October 20, 2022 19:49 View deployment

balazsorban44 commented Oct 20, 2022

View reviewed changes

balazsorban44 mentioned this pull request Oct 20, 2022

BREAKING CHANGE: feat(edge): split NextCookies to RequestCookies and ResponseCookies vercel/next.js#41526

Merged

11 tasks

Kikobeats reviewed Oct 21, 2022

View reviewed changes

Comment thread packages/cookies/src/serialize.ts Outdated

Kikobeats reviewed Oct 21, 2022

View reviewed changes

address review

b471e97

vercel Bot deployed to Preview October 21, 2022 15:08 View deployment

Kikobeats approved these changes Oct 21, 2022

View reviewed changes

Kikobeats merged commit 2c20094 into vercel:main Oct 21, 2022

Kikobeats mentioned this pull request Oct 21, 2022

Version Packages (beta) #179

Merged

balazsorban44 deleted the feat/response-cookies-to-cookiestore branch October 22, 2022 10:52

Schniz reviewed Oct 23, 2022

View reviewed changes

Kikobeats mentioned this pull request Oct 24, 2022

Version Packages #184

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(cookies): make `RequestCookies` and `ResponseCookies` more spec compliant#177

feat(cookies): make `RequestCookies` and `ResponseCookies` more spec compliant#177
Kikobeats merged 5 commits into
vercel:mainfrom
balazsorban44:feat/response-cookies-to-cookiestore

balazsorban44 commented Oct 20, 2022 •

edited

Loading

Uh oh!

changeset-bot Bot commented Oct 20, 2022 •

edited

Loading

Uh oh!

vercel Bot commented Oct 20, 2022 •

edited

Loading

Uh oh!

codecov-commenter commented Oct 20, 2022

Uh oh!

balazsorban44 Oct 20, 2022 •

edited

Loading

Uh oh!

Kikobeats Oct 21, 2022 •

edited

Loading

Uh oh!

balazsorban44 Oct 21, 2022

Uh oh!

Kikobeats Oct 21, 2022

Uh oh!

balazsorban44 Oct 21, 2022 •

edited

Loading

Uh oh!

Uh oh!

Kikobeats left a comment

Uh oh!

Schniz Oct 23, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		@@ -1,3 +1,3 @@
		export type { Options } from './serialize'
		export type { Cookie, CookieListItem } from './serialize'

Conversation

balazsorban44 commented Oct 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

changeset-bot Bot commented Oct 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

vercel Bot commented Oct 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Oct 20, 2022

Codecov Report

Uh oh!

balazsorban44 Oct 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Kikobeats Oct 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

balazsorban44 Oct 21, 2022

Choose a reason for hiding this comment

Uh oh!

Kikobeats Oct 21, 2022

Choose a reason for hiding this comment

Uh oh!

balazsorban44 Oct 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Kikobeats left a comment

Choose a reason for hiding this comment

Uh oh!

Schniz Oct 23, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

balazsorban44 commented Oct 20, 2022 •

edited

Loading

changeset-bot Bot commented Oct 20, 2022 •

edited

Loading

vercel Bot commented Oct 20, 2022 •

edited

Loading

balazsorban44 Oct 20, 2022 •

edited

Loading

Kikobeats Oct 21, 2022 •

edited

Loading

balazsorban44 Oct 21, 2022 •

edited

Loading