Skip to content

Is there a plan to support cookie parameters? #1689

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

Closed
1 task done
zaru opened this issue Jun 6, 2024 · 2 comments
Closed
1 task done

Is there a plan to support cookie parameters? #1689

zaru opened this issue Jun 6, 2024 · 2 comments
Labels
enhancement New feature or request openapi-fetch Relevant to the openapi-fetch library PRs welcome PRs are welcome to solve this issue!

Comments

@zaru
Copy link
Contributor

zaru commented Jun 6, 2024

Description

Hello! I’m using openapi-fetch conveniently. Thank you. I would be delighted if openapi-fetch supported cookie parameters. Currently, I am sending cookies from the headers as follows:

const { data, error } = await fetchClient.GET("/me", {
  params: {
    header: {
      Cookie: "_session_id=foobar",
    },
  },
});

Proposal

I propose that we add support for specifying cookie parameters directly. For example:

const { data, error } = await fetchClient.GET("/me", {
  params: {
    cookie: {
      _session_id: "foobar",
    },
  },
});

I found that adding the following code to the index.js of openapi-fetch works, but I’m not sure if this is the correct approach:

diff --git a/packages/openapi-fetch/src/index.js b/packages/openapi-fetch/src/index.js
index e0cb43c..e612304 100644
--- a/packages/openapi-fetch/src/index.js
+++ b/packages/openapi-fetch/src/index.js
@@ -76,6 +76,14 @@ export default function createClient(clientOptions) {
       ...init,
       headers: mergeHeaders(baseHeaders, headers, params.header),
     };
+
+    if (params.cookie) {
+      const cookieValue = Object.entries(params.cookie)
+        .map(([key, value]) => `${key}=${value}`)
+        .join("; ");
+      requestInit.headers.set("Cookie", cookieValue);
+    }
+
     if (requestInit.body) {
       requestInit.body = bodySerializer(requestInit.body);
     }

Checklist

@zaru zaru added enhancement New feature or request PRs welcome PRs are welcome to solve this issue! openapi-fetch Relevant to the openapi-fetch library labels Jun 6, 2024
@drwpow-figma
Copy link

drwpow-figma commented Jun 6, 2024

Love this request! I think as a start, I’d love to see documentation added on the middleware section on a way to achieve this with middleware—listening for the cookie header and creating the cookie / adding it to the request. We could also maybe add it as a generic example as well.

People may really need to manage this themselves because there are opinions on expiration and invalidation that isn’t contained in the OpenAPI spec.

I feel that people managing the nuances of cookie management via middleware is a better option than baking an opinionated approach to cookies in this library. But say we document everything and learn that there’s more indirection / a simpler way to allow this with less boilerplate, at which point I’d consider adding this as a feature. But either way, I think documentation is the place to start!

@zaru
Copy link
Contributor Author

zaru commented Jun 6, 2024

Thank you for the response! I agree that implementing this via middleware seems more appropriate. I will first try it with middleware, and if I feel it is worth adding to the documentation, I will make a proposal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request openapi-fetch Relevant to the openapi-fetch library PRs welcome PRs are welcome to solve this issue!
Projects
None yet
Development

No branches or pull requests

2 participants