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(nuxt): stringify cookie values before broadcasting them #23449

Merged
merged 8 commits into from Sep 29, 2023
Merged

fix(nuxt): stringify cookie values before broadcasting them #23449

merged 8 commits into from Sep 29, 2023

Conversation

hendrikheil
Copy link
Contributor

πŸ”— Linked issue

#21651

❓ Type of change

  • πŸ“– Documentation (updates to the documentation, readme or JSdoc annotations)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

Resolves #21651 by preparing the cookie value for the structuredClone that's used by BroadcastChannel.postMessage.

It does this simply by cloning the value which might potentially hold nested Refs. toRaw unfortunately only applies to the root Ref, not to any nested Refs.

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@stackblitz
Copy link

stackblitz bot commented Sep 28, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@hendrikheil
Copy link
Contributor Author

hendrikheil commented Sep 28, 2023

This PR will be squashed right? I'd force push to avoid the little commits fixing my debugging artifacts if not

@danielroe
Copy link
Member

Yes, the pr will be squashed. I guess I'm thinking that the encoder has to produce a string to save the cookie. (It won't be undefined as we have defaults.)

@danielroe danielroe changed the title fix: prepare cookie value for structuredClone before posting fix(nuxt): stringify cookie values before broadcasting them Sep 29, 2023
@danielroe danielroe merged commit b95c85b into nuxt:main Sep 29, 2023
34 checks passed
@danielroe
Copy link
Member

Thank you ❀️

@github-actions github-actions bot mentioned this pull request Sep 29, 2023
@hendrikheil hendrikheil deleted the fix/21651 branch September 29, 2023 14:25
@hendrikheil
Copy link
Contributor Author

hendrikheil commented Sep 29, 2023

@danielroe I noticed you removed the ternary. This could theoretically lead to encode or decode being undefined. {...defaults, ..._opts }, unlike defu, overwrites when a key is set to undefined.

Not sure if thats something this should really worry about as it would clearly be the users fault, but it's theoretically possible, so I wanted to mention it :)

@danielroe
Copy link
Member

Ah, you're right. I initially thought it would error if the user did that, but I think implementation in cookie-es would actually just fallback to encodeURIComponent directly.

I think this is likely an edge case but if you want to, we could also cover this scenario. I just don't want to add too much runtime code to cover this scenario.

@Aietes
Copy link
Contributor

Aietes commented Jan 28, 2024

Has this actually been merged? I'm still facing the issue on the latest nuxt release:

DOMException: Failed to execute 'postMessage' on 'BroadcastChannel': #<Object> could not be cloned.
    at SupabaseAuthClient._notifyAllSubscribers (http://localhost:3000/_nuxt/node_modules/.cache/vite/client/deps/chunk-A274CJAX.js?v=fac2f4d6:2156:31)
    at SupabaseAuthClient._recoverAndRefresh (http://localhost:3000/_nuxt/node_modules/.cache/vite/client/deps/chunk-A274CJAX.js?v=fac2f4d6:2101:20)
    at async SupabaseAuthClient._initialize (http://localhost:3000/_nuxt/node_modules/.cache/vite/client/deps/chunk-A274CJAX.js?v=fac2f4d6:987:7)
    at async http://localhost:3000/_nuxt/node_modules/.cache/vite/client/deps/chunk-A274CJAX.js?v=fac2f4d6:950:16
_recoverAndRefresh @ chunk-A274CJAX.js?v=fac2f4d6:2105
await in _recoverAndRefresh (async)
_initialize @ chunk-A274CJAX.js?v=fac2f4d6:987
await in _initialize (async)

@danielroe
Copy link
Member

It was released in Nuxt 3.8. Are you sure the log you linked is related to this code, and not another issue in the supabase module?

@Aietes
Copy link
Contributor

Aietes commented Jan 29, 2024

@danielroe I'm not sure, it could be a supabase issue. It's hard to determine what really causes this issue, I'm not sure how to even provide a reproduction. I'lll also bring this up on the supabase repo, and see if we can find a specific cause.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BroadcastChannel.postMessage in useCookie has issue with ProxyObject
3 participants