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

Edge Cookies: Add .getWithOptions method #36943

Merged
merged 5 commits into from May 19, 2022
Merged

Conversation

Kikobeats
Copy link
Member

@Kikobeats Kikobeats commented May 16, 2022

Hello,

This is an iteration after first work at #36478.

What that PR missed is a way to just get a cookie value. Well, this PR adds two new things:

cookies.get returns the cookie value that could be string | undefined:

const response = new NextResponse()
response.cookies.set('foo', 'bar', { path: '/test' })

const value = response.cookies.get('foo')
console.log(value) // => 'bar'

Additionally, if you want to know all the cookie details, you can use cookies.getWithOptions:

const response = new NextResponse()

response.cookies.set('foo', 'bar', { path: '/test' })
const { value, options } response.cookies.getWithOptions('foo')

console.log(value) // => 'bar'
console.log(options) // => { Path: '/test' }

@Kikobeats Kikobeats changed the title Edge Cookies: Add getRaw method Edge Cookies: Add .getRaw May 16, 2022
@ijjk
Copy link
Member

ijjk commented May 16, 2022

Stats from current PR

Default Build (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary Kikobeats/next.js edge/cookies Change
buildDuration 15.9s 16s ⚠️ +6ms
buildDurationCached 6.3s 6.5s ⚠️ +151ms
nodeModulesSize 478 MB 478 MB ⚠️ +1.24 kB
Page Load Tests Overall increase ✓
vercel/next.js canary Kikobeats/next.js edge/cookies Change
/ failed reqs 0 0
/ total time (seconds) 3.886 3.857 -0.03
/ avg req/sec 643.33 648.12 +4.79
/error-in-render failed reqs 0 0
/error-in-render total time (seconds) 1.335 1.308 -0.03
/error-in-render avg req/sec 1872.99 1911.94 +38.95
Client Bundles (main, webpack)
vercel/next.js canary Kikobeats/next.js edge/cookies Change
925.HASH.js gzip 179 B 179 B
framework-HASH.js gzip 42 kB 42 kB
main-HASH.js gzip 28.8 kB 28.8 kB
webpack-HASH.js gzip 1.44 kB 1.44 kB
Overall change 72.4 kB 72.4 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary Kikobeats/next.js edge/cookies Change
polyfills-HASH.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary Kikobeats/next.js edge/cookies Change
_app-HASH.js gzip 1.36 kB 1.36 kB
_error-HASH.js gzip 193 B 193 B
amp-HASH.js gzip 308 B 308 B
css-HASH.js gzip 327 B 327 B
dynamic-HASH.js gzip 2.71 kB 2.71 kB
head-HASH.js gzip 359 B 359 B
hooks-HASH.js gzip 920 B 920 B
image-HASH.js gzip 5.73 kB 5.73 kB
index-HASH.js gzip 263 B 263 B
link-HASH.js gzip 2.65 kB 2.65 kB
routerDirect..HASH.js gzip 320 B 320 B
script-HASH.js gzip 391 B 391 B
withRouter-HASH.js gzip 318 B 318 B
85e02e95b279..7e3.css gzip 107 B 107 B
Overall change 16 kB 16 kB
Client Build Manifests
vercel/next.js canary Kikobeats/next.js edge/cookies Change
_buildManifest.js gzip 458 B 458 B
Overall change 458 B 458 B
Rendered Page Sizes
vercel/next.js canary Kikobeats/next.js edge/cookies Change
index.html gzip 532 B 532 B
link.html gzip 544 B 544 B
withRouter.html gzip 527 B 527 B
Overall change 1.6 kB 1.6 kB

Default Build with SWC (Decrease detected ✓)
General Overall increase ⚠️
vercel/next.js canary Kikobeats/next.js edge/cookies Change
buildDuration 18.2s 18.6s ⚠️ +386ms
buildDurationCached 6.5s 6.3s -212ms
nodeModulesSize 478 MB 478 MB ⚠️ +1.24 kB
Page Load Tests Overall decrease ⚠️
vercel/next.js canary Kikobeats/next.js edge/cookies Change
/ failed reqs 0 0
/ total time (seconds) 3.937 3.982 ⚠️ +0.05
/ avg req/sec 635 627.86 ⚠️ -7.14
/error-in-render failed reqs 0 0
/error-in-render total time (seconds) 1.357 1.443 ⚠️ +0.09
/error-in-render avg req/sec 1841.8 1733 ⚠️ -108.8
Client Bundles (main, webpack)
vercel/next.js canary Kikobeats/next.js edge/cookies Change
925.HASH.js gzip 178 B 178 B
framework-HASH.js gzip 42.7 kB 42.7 kB
main-HASH.js gzip 29.2 kB 29.2 kB
webpack-HASH.js gzip 1.45 kB 1.45 kB
Overall change 73.5 kB 73.5 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary Kikobeats/next.js edge/cookies Change
polyfills-HASH.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary Kikobeats/next.js edge/cookies Change
_app-HASH.js gzip 1.35 kB 1.35 kB
_error-HASH.js gzip 179 B 179 B
amp-HASH.js gzip 311 B 311 B
css-HASH.js gzip 324 B 324 B
dynamic-HASH.js gzip 2.89 kB 2.89 kB
head-HASH.js gzip 357 B 357 B
hooks-HASH.js gzip 920 B 920 B
image-HASH.js gzip 5.82 kB 5.82 kB
index-HASH.js gzip 261 B 261 B
link-HASH.js gzip 2.78 kB 2.78 kB
routerDirect..HASH.js gzip 322 B 322 B
script-HASH.js gzip 392 B 392 B
withRouter-HASH.js gzip 317 B 317 B
85e02e95b279..7e3.css gzip 107 B 107 B
Overall change 16.3 kB 16.3 kB
Client Build Manifests
vercel/next.js canary Kikobeats/next.js edge/cookies Change
_buildManifest.js gzip 457 B 457 B
Overall change 457 B 457 B
Rendered Page Sizes
vercel/next.js canary Kikobeats/next.js edge/cookies Change
index.html gzip 531 B 531 B
link.html gzip 546 B 546 B
withRouter.html gzip 528 B 528 B
Overall change 1.6 kB 1.6 kB
Commit: a644acb

@Kikobeats Kikobeats force-pushed the edge/cookies branch 2 times, most recently from 737ad05 to 39af2a0 Compare May 16, 2022 14:29
@@ -59,10 +59,17 @@ export class NextCookies extends Cookies {
super(response.headers.get('cookie'))
this.response = response
}
get = (...args: Parameters<Cookies['get']>) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

can we be explicit on types here?

@Kikobeats Kikobeats force-pushed the edge/cookies branch 2 times, most recently from df15526 to db5ed52 Compare May 17, 2022 10:46
@Kikobeats Kikobeats changed the title Edge Cookies: Add .getRaw Edge Cookies: Add .getWithOptions method May 18, 2022
Copy link
Contributor

@Schniz Schniz left a comment

Choose a reason for hiding this comment

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

lgtm, I am still having my concerns of extending Map, but even if we extend Map, feels like we should extend Map<string, string> and not Map<string, string | object>. That's probably for another PR.

Anyway I do like getWithOptions. Can we add explicit return type for get?

Kikobeats and others added 2 commits May 18, 2022 23:35
Co-authored-by: Shu Uesugi <shu.chibicode@gmail.com>
Co-authored-by: Shu Uesugi <shu.chibicode@gmail.com>
chibicode
chibicode previously approved these changes May 19, 2022
gaspar09
gaspar09 previously approved these changes May 19, 2022
@Kikobeats Kikobeats dismissed stale reviews from gaspar09 and chibicode via c0c2235 May 19, 2022 12:30
@kodiakhq kodiakhq bot merged commit cc8ab99 into vercel:canary May 19, 2022
@Kikobeats Kikobeats deleted the edge/cookies branch May 19, 2022 13:27
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants