feat(cookieJar): Change cookie.get to directly return value (#10493)

redwoodjs · Apr 22, 2024 · c6788cd · c6788cd
1 parent 859d93b
commit c6788cd
Show file tree

Hide file tree

Showing 5 changed files with 29 additions and 16 deletions.
diff --git a/.changesets/10493.md b/.changesets/10493.md
@@ -0,0 +1,19 @@
+- feat(cookieJar): Change cookie.get to directly return value (#10493) by @dac09
+
+**Motivation**
+My original design of the `CookeiJar.get` would return the full cookie object we store, including cookie options. This is not ideal because you need to access the cookie like this:
+
+```js
+const myCookie = mwRequest.cookies.get('myCookie')
+
+// 👇
+const actualValue = myCookie.value
+```
+
+This is unwieldy, and feels unergonomic for the 98% of cases where `get` will be used to just see the value. 
+
+**How do I still see the options of the cookie?**
+You can still access all the details of the cookie by doing `cookie.entries`. I don't really have a case for this yet, so let's not optimise for this case, but we know it's possible!
+
+
+This is me just stabilizing the API for Middleware stuff, before we ship it out of experimental
diff --git a/packages/vite/src/middleware/CookieJar.test.ts b/packages/vite/src/middleware/CookieJar.test.ts
@@ -12,21 +12,17 @@ describe('CookieJar', () => {
   )
 
   test('instatitates cookie jar from a cookie string', () => {
-    expect(cookieJar.get('color_mode')).toStrictEqual({
-      value: JSON.stringify({
+    expect(cookieJar.get('color_mode')).toStrictEqual(
+      JSON.stringify({
         color_mode: 'light',
         light_theme: { name: 'light', color_mode: 'light' },
         dark_theme: { name: 'dark_dimmed', color_mode: 'dark' },
       }),
-    })
+    )
 
-    expect(cookieJar.get('preferred_color_mode')).toStrictEqual({
-      value: 'dark',
-    })
+    expect(cookieJar.get('preferred_color_mode')).toStrictEqual('dark')
 
-    expect(cookieJar.get('tz')).toStrictEqual({
-      value: 'Asia/Bangkok',
-    })
+    expect(cookieJar.get('tz')).toStrictEqual('Asia/Bangkok')
   })
 
   describe('Helper methods like JS Map', () => {
@@ -60,11 +56,9 @@ describe('CookieJar', () => {
 
       myJar.unset('auth_provider')
 
-      const { value: authProviderValue, options: authProviderOptions } =
-        myJar.get('auth_provider')!
+      const authProviderValue = myJar.get('auth_provider')
 
       expect(authProviderValue).toBeFalsy()
-      expect(authProviderOptions!.expires).toStrictEqual(new Date(0))
     })
 
     test('clear All', () => {

diff --git a/packages/vite/src/middleware/CookieJar.ts b/packages/vite/src/middleware/CookieJar.ts
@@ -37,7 +37,7 @@ export class CookieJar {
   }
 
   public get(name: string) {
-    return this.map.get(name)
+    return this.map.get(name)?.value
   }
 
   public has(name: string) {

diff --git a/packages/vite/src/middleware/MiddlewareRequest.test.ts b/packages/vite/src/middleware/MiddlewareRequest.test.ts
@@ -18,7 +18,7 @@ describe('MiddlewareRequest', () => {
     })
     const mReq = createMiddlewareRequest(req)
 
-    expect(mReq.cookies.get('foo')).toStrictEqual({ value: 'bar' })
+    expect(mReq.cookies.get('foo')).toStrictEqual('bar')
     expect(mReq.method).toStrictEqual('POST')
     expect(mReq.headers.get('Content-Type')).toStrictEqual('application/json')
 
@@ -43,7 +43,7 @@ describe('MiddlewareRequest', () => {
 
     const mReq = createMiddlewareRequest(whatWgRequest)
 
-    expect(mReq.cookies.get('errybody')).toStrictEqual({ value: 'lets-funk' })
+    expect(mReq.cookies.get('errybody')).toStrictEqual('lets-funk')
     expect(mReq.method).toStrictEqual('PUT')
 
     expect(mReq.headers.get('x-custom-header')).toStrictEqual('beatdrop')

diff --git a/packages/vite/src/middleware/register.test.ts b/packages/vite/src/middleware/register.test.ts
@@ -139,7 +139,7 @@ describe('chain', () => {
     expect(output.headers.get('class-mw-value')).toBe('999')
 
     // The other one still gets chained
-    expect(output.cookies.get('add-cookie-mw').value).toBe('added')
+    expect(output.cookies.get('add-cookie-mw')).toBe('added')
 
     // Because /bazinga is more specific, the '*' handlers won't be executed
     expect(output.headers.get('add-header-mw')).toBeFalsy()