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

fix: small improvements/fixes to cookie chunker in ssr #760

Merged
merged 1 commit into from
Apr 10, 2024

Conversation

hf
Copy link
Contributor

@hf hf commented Apr 5, 2024

Fixes/improves some edge cases in the cookie chunker for ssr:

  1. deleteChunks now attempts to delete both the key and potential leftover chunks, instead of deleting the first thing it notices. This should decrease the total size of cookies by removing orphaned chunks, and also is a bit safer in situations where leftover chunks are mistaken as the continuation of other chunks.
  2. Avoid using parallel cookie setting in setItem, and instead do it serially.
  3. setItem first clears all chunks before setting the new ones.

@hf hf requested a review from a team as a code owner April 5, 2024 15:13
@hf
Copy link
Contributor Author

hf commented Apr 7, 2024

One confirmed support ticket that is demonstrating the issues resolved by these changes.

Copy link
Contributor

@J0 J0 left a comment

Choose a reason for hiding this comment

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

Seems sane at brief glance, approving for now to unblock and will test in the morrow if not yet merged

Copy link
Member

@kangmingtay kangmingtay left a comment

Choose a reason for hiding this comment

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

the package doesn't build with pnpm build:ssr

edit: i've fixed the build errors in this commit

packages/ssr/src/createServerClient.ts Outdated Show resolved Hide resolved
@kangmingtay
Copy link
Member

@hf remember to generate a changeset too!

@glib-0
Copy link

glib-0 commented Apr 9, 2024

Does this change fix the bug I just submitted? #766

@hf
Copy link
Contributor Author

hf commented Apr 10, 2024

Does this change fix the bug I just submitted? #766

Maybe...

@hf hf requested a review from kangmingtay April 10, 2024 14:04
@hf hf dismissed kangmingtay’s stale review April 10, 2024 14:06

Build errors fixed.

@hf hf merged commit ef61542 into main Apr 10, 2024
3 checks passed
@hf hf deleted the hf/microfixes-chunker branch April 10, 2024 14:11
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