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

Potential breaking changes in recent sveltekit #235

Closed
samuelstroschein opened this issue Oct 7, 2024 · 4 comments
Closed

Potential breaking changes in recent sveltekit #235

samuelstroschein opened this issue Oct 7, 2024 · 4 comments

Comments

@samuelstroschein
Copy link
Member

Discussed in https://github.com/orgs/opral/discussions/3153

Originally posted by axel-rock October 7, 2024
Hello,

I ran an npm update, and seriously broke my app.

For once, I tried to investigate the problem.

If I'm correct (I may totally not be), it's due to an internal SvelteKit function rejecting the cookie "paraglide:lang" because of the ":".

Here is my full stack trace, and where it leads:

TypeError: argument name is invalid
    at Module.serialize (/Users/axel/Developer/persocoms/node_modules/.pnpm/cookie@0.7.1/node_modules/cookie/index.js:194:11)
    at set_internal (/Users/axel/Developer/persocoms/node_modules/.pnpm/@sveltejs+kit@2.6.2_@sveltejs+vite-plugin-svelte@4.0.0-next.7_svelte@5.0.0-next.262_vite@5.4.8/node_modules/@sveltejs/kit/src/runtime/server/cookie.js:196:23)
    at Object.set (/Users/axel/Developer/persocoms/node_modules/.pnpm/@sveltejs+kit@2.6.2_@sveltejs+vite-plugin-svelte@4.0.0-next.7_svelte@5.0.0-next.262_vite@5.4.8/node_modules/@sveltejs/kit/src/runtime/server/cookie.js:117:4)
    at /Users/axel/Developer/persocoms/node_modules/.pnpm/@inlang+paraglide-sveltekit@0.11.0_@sveltejs+kit@2.6.2/node_modules/@inlang/paraglide-sveltekit/dist/runtime/hooks/handle.server.js:57:27
    at apply_handle (/Users/axel/Developer/persocoms/node_modules/.pnpm/@sveltejs+kit@2.6.2_@sveltejs+vite-plugin-svelte@4.0.0-next.7_svelte@5.0.0-next.262_vite@5.4.8/node_modules/@sveltejs/kit/src/exports/hooks/sequence.js:85:11)
    at resolve (/Users/axel/Developer/persocoms/node_modules/.pnpm/@sveltejs+kit@2.6.2_@sveltejs+vite-plugin-svelte@4.0.0-next.7_svelte@5.0.0-next.262_vite@5.4.8/node_modules/@sveltejs/kit/src/exports/hooks/sequence.js:110:9)
    at supabaseHandle (/Users/axel/Developer/persocoms/src/hooks.server.ts:66:9)
    at apply_handle (/Users/axel/Developer/persocoms/node_modules/.pnpm/@sveltejs+kit@2.6.2_@sveltejs+vite-plugin-svelte@4.0.0-next.7_svelte@5.0.0-next.262_vite@5.4.8/node_modules/@sveltejs/kit/src/exports/hooks/sequence.js:85:11)
    at Object.eval [as handle] (/Users/axel/Developer/persocoms/node_modules/.pnpm/@sveltejs+kit@2.6.2_@sveltejs+vite-plugin-svelte@4.0.0-next.7_svelte@5.0.0-next.262_vite@5.4.8/node_modules/@sveltejs/kit/src/exports/hooks/sequence.js:74:10)
    at Module.respond (/Users/axel/Developer/persocoms/node_modules/.pnpm/@sveltejs+kit@2.6.2_@sveltejs+vite-plugin-svelte@4.0.0-next.7_svelte@5.0.0-next.262_vite@5.4.8/node_modules/@sveltejs/kit/src/runtime/server/respond.js:327:40)
    ```
    
    in `cookie@0.7.1`
    
    ```
    function serialize(name, val, opt) {
  var enc = (opt && opt.encode) || encodeURIComponent;

  if (typeof enc !== 'function') {
    throw new TypeError('option encode is invalid');
  }

  if (!cookieNameRegExp.test(name)) {
    throw new TypeError('argument name is invalid'); // ❌
  }
I tried to remove the ":" locally, and it indeed fixes it. 

```
/** The key with which `invalidate` is called when the language changes */

export const LANGUAGE_CHANGE_INVALIDATION_KEY = "paraglidelang"

/** The name of the cookie in which the language is stored */
export const LANG_COOKIE_NAME = "paraglidelang"


I'm using Svelte 5 and Vite Plugin Svelte in its next.7 version, so I'm not reporting a bug here, I know I can expect support for non stable releases. 

This is a side project, I'll dig deeper to find a fix (whether the right package versions to use to avoid it, or a suggestion for paraglide)</div>
@samuelstroschein samuelstroschein transferred this issue from opral/monorepo Oct 7, 2024
@samuelstroschein
Copy link
Member Author

@axel-rock it seems like : is not a valid character for cookie names. some cookie parsers allow it, some don't.

Proposal

Use a cookie name without :.

Are you open to do a PR? I assume that this won't break existing apps, and if it does it's a bug fix.

@axel-rock
Copy link

axel-rock commented Oct 7, 2024

@samuelstroschein Sure, I'll do a PR, sounds easy enough for a first one. Any preference for the new cookie name?

@axel-rock
Copy link

Just noticed this issue is a duplicate of #234

@samuelstroschein
Copy link
Member Author

Nice closing this as duplicate of #234 and will merge your PR promptly!

@samuelstroschein samuelstroschein closed this as not planned Won't fix, can't repro, duplicate, stale Oct 7, 2024
@samuelstroschein samuelstroschein closed this as not planned Won't fix, can't repro, duplicate, stale Oct 15, 2024
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

No branches or pull requests

2 participants