jycouet / kitql

A set of standalone tools to SpeedRun WebApps!
https://kitql.dev
MIT License
406 stars 37 forks source link

Actions search params not working #496

Closed tmarnet closed 11 months ago

tmarnet commented 11 months ago

Describe the bug

Configuring and using search params on actions currently doesn't work properly because we're appending the ? character at the begining of the search params, but actions paths already include this ?, so in that case we should be using & instead.

Your Example Website or App

https://stackblitz.com/edit/github-nxv4s3

Steps to Reproduce the Bug or Issue

  1. Starting from the template
  2. Create a /about/+page.server.ts file with the following content:
    
    import type { Actions } from './$types'

export const actions = { withSearchParam(event) { console.log('Received redirectUrl', event.url.searchParams.get('redirectUrl')); } } satisfies Actions;

3. In the `/about/+page.svelte` paste the following content:
```ts
<script lang="ts">
  import { enhance } from '$app/forms';
  import { page } from '$app/stores';
  import { ACTIONS } from '$lib/ROUTES';
</script>

<h2>About</h2>

<p>Yes, you are on the about page.</p>

<form
  use:enhance
  method="POST"
  action={ACTIONS['/about']('withSearchParam', { redirectUrl: $page.url.pathname })}
>
  <button>
    Go!
  </button>
</form>
  1. Update the kitRoutes with the following:
    ACTIONS: {
    auth: {
        explicit_search_params: {
            redirectUrl: {
                required: true,
                type: 'string',
            },
        },
    },
    },
  2. Run the app, inspect the Go! button form on the /about page and notice that the form action equals /about?/withSearchParam?redirectUrl=%2Fabout when it should in reality be /about?/withSearchParam&redirectUrl=%2Fabout.

Expected behavior

When appending search params to an action we should use the & character instead of the ? character.

Screenshots or Videos

No response

Platform

Additional context

No response

jycouet commented 11 months ago

I guess that this is applicable only to named action?

You rock, by finding all this πŸŽ‰

tmarnet commented 11 months ago

I updated the Stackblitz to include a default action as well.

It looks like this is broken for both default and named actions since the plugin treats default actions as named ones (by appending /default):

Screenshot 2023-11-25 at 12 16 36
jycouet commented 11 months ago

I almost feels embarrassed! lol. I'll fix this soon πŸ‘

tmarnet commented 11 months ago

You shouldn't 😊

But let me rectify my previous comment:

  1. If, for a given page, there is only a default action, then appendSp works fine (see this new Stackblitz: https://stackblitz.com/edit/github-b9auqn)
  2. But if there is also a named action, then the plugin treats the default action as a named one, hence appendSp doesn't behave properly (see the original Stackblitz: https://stackblitz.com/edit/github-nxv4s3)
jycouet commented 11 months ago

Hooo, but you can't have a named & default at the same time?!

https://kit.svelte.dev/docs/form-actions#named-actions Capture d’écran 2023-11-25 aΜ€ 18 32 50

Anyway, I'll fix the SearchParam stuff πŸ‘

tmarnet commented 11 months ago

Didn't know that, TIL!

Thanks πŸ™‚

jycouet commented 11 months ago

Hooo, you get a warning, only when you start using it:

Error: When using named actions, the default action cannot be used. See the docs for more info: https://kit.svelte.dev/docs/form-actions#named-actions

I guess that I can put the warning directly directly when I see this

CoooooooL stuff

jycouet commented 11 months ago

HΓ©hΓ©hΓ©, new log coming:

Capture d’écran 2023-11-25 aΜ€ 18 55 34

jycouet commented 11 months ago

πŸš€ vite-plugin-kit-routes@0.1.4 it out. πŸš€

Could you try and let me know ? I let you close the issue if it's solved πŸ‘

jycouet commented 11 months ago

Thx to you I even can put out more content: https://x.com/jycouet/status/1728489740184248599?s=20 πŸ˜…

πŸ™ πŸ™ πŸ™

tmarnet commented 11 months ago

Awesome!

It's working fine with vite-plugin-kit-routes@0.1.4, thanks!

Closing this issue.