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

feat: full rewrite using getAll and setAll cookie methods #1

Merged
merged 1 commit into from
Jun 12, 2024
Merged

Conversation

hf
Copy link
Collaborator

@hf hf commented May 24, 2024

Re-implements the createServerClient and createBrowserClient to solve a few important problems that were not solved with the implementation previously:

Cookie encoding
Cookies can't contain just any characters -- they're a limited set of US ASCII and " is not allowed. Internally the Supabase client uses JSON to encode the session, so this must not be mapped directly to cookies. In this implementation, cookies are encoded as Base64 URL and the leading chunk contains the base64- prefix.

Cookie chunk management
There are a few state transitions when storage needs to be chunked into multiple cookies which were not handled well. This is the reason why a change of API is necessary from get, set and remove cookie access methods to getAll and setAll. The library must be able to see all of the cookies and manage deleting unnecessary chunks when that is required.

Server-side developer ergonomics
In some SSR frameworks (notably middleware in NextJS) it can be very tricky to set cookies on objects correctly. createServerClient only calls the setAll cookie access method only once and only when necessary. This reduces the number of bugs developers can introduce themselves when using the server client.

For more details check the docs/design.md document which contains a lot of details that can't easily be covered in the PR description.

@hf hf force-pushed the hf/rewrite branch 2 times, most recently from e2beb93 to d317d4b Compare May 24, 2024 15:58
@hf hf force-pushed the hf/rewrite branch 3 times, most recently from f535af2 to 3deeda2 Compare May 25, 2024 10:35
@hf
Copy link
Collaborator Author

hf commented May 27, 2024

Moved around the types and TSDoc to make sure it shows up crossed out in VS Code:

image

@hf
Copy link
Collaborator Author

hf commented May 27, 2024

Obviously it is not crossed out when using it correctly:
image

@hf hf force-pushed the hf/rewrite branch 9 times, most recently from b1d9623 to 887b42f Compare May 28, 2024 17:45
@hf hf force-pushed the hf/rewrite branch 6 times, most recently from 52d29c4 to 5ef5db3 Compare June 5, 2024 10:56
@hf hf marked this pull request as ready for review June 5, 2024 11:26
@kangmingtay
Copy link
Member

@hf would be great if we can also clean up the unused imports in each file

@hf hf force-pushed the hf/rewrite branch 3 times, most recently from cce4a90 to 48b0968 Compare June 12, 2024 12:19
@hf
Copy link
Collaborator Author

hf commented Jun 12, 2024

@hf would be great if we can also clean up the unused imports in each file

Which ones? TypeScript / eslint usually yell about that.

@hf hf merged commit b6ae192 into main Jun 12, 2024
3 checks passed
@hf hf deleted the hf/rewrite branch June 12, 2024 12:40
hf pushed a commit that referenced this pull request Jun 24, 2024
🤖 I have created a release *beep* *boop*
---


## [0.4.0](v0.3.0...v0.4.0)
(2024-06-24)


### Features

* full rewrite using `getAll` and `setAll` cookie methods
([#1](#1))
([b6ae192](b6ae192))


### Bug Fixes

* allow use of `createBrowserClient` without `window` present
([#20](#20))
([27d868d](27d868d))
* deprecate `parse`, `serialize` exports for more useful functions
([#14](#14))
([0b5f881](0b5f881))
* fix `createBrowserClient` deprecation tsdoc
([#17](#17))
([1df70ad](1df70ad))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants