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

[🐞] onSubmitCompleted$ is always triggered, even when fails #4257

Open
keuller opened this issue May 18, 2023 · 4 comments
Open

[🐞] onSubmitCompleted$ is always triggered, even when fails #4257

keuller opened this issue May 18, 2023 · 4 comments
Labels
COMP: qwik-city TYPE: bug Something isn't working VERSION: after next major Features we'd want to add after the next version is done

Comments

@keuller
Copy link
Contributor

keuller commented May 18, 2023

Which component is affected?

Qwik Runtime

Describe the bug

According to the documentation onSubmitCompleted$ should be executed right after the action is executed successfully and returns some data. The issue: it is always executed no matter Zod validation fails or I return fail inside actionRoute$.

Reproduction

N/A

Steps to reproduce

You can use this code:

import { component$ } from '@builder.io/qwik';
import { Form, routeAction$, zod$, z } from '@builder.io/qwik-city';

export const useSaveAction = routeAction$(async (lead, { request, fail }) => {
    console.log('lead:', lead);
    
    return {
    }
}, zod$({
    name: z.string().min(3),
    email: z.string().email(),
}));

export default component$(() => {
   const action = useSaveAction();

   return (
   <Form id='leadForm' action={action}
           onSubmitCompleted$={(ev) => console.log('onSubmitCompleted', ev.detail)}>

      <label for="id" class="text-sm text-slate-500">{ props.label }</label>
       <input type="text" name="name"
            class="border border-slate-500 rounded-md p-2" 
            maxLength={50} />

       <input type="email" name="email"
            class="border border-slate-500 rounded-md p-2" 
            maxLength={100} />

      <div class="col-span-2 py-2">
          <button type="submit" name="_intent" 
              value="create"
              disabled={action.isRunning}
              class="outline-none bg-indigo-500 hover:bg-indigo-600 px-6 py-2 rounded-md text-sm text-white disabled:bg-slate-400">
              <span>{action.isRunning ? 'Saving...' : 'Save'}</span>
          </button>
       </div>
   </Form>)
});

### System Info

```shell
System: Mac OSX Ventura 13.3
Qwik version: 1.1.4
Qwik-City: 1.1.4
Typescript: 5.0.4
Node: 16.15.0

Additional Information

No response

@keuller keuller added TYPE: bug Something isn't working STATUS-1: needs triage New issue which needs to be triaged labels May 18, 2023
@keuller keuller changed the title [🐞] onSubmitCplmeted$ is always triggered, even when fails [🐞] onSubmitCompleted$ is always triggered, even when fails May 19, 2023
@manucorporat manucorporat added the P4: urgent If Bug - it makes Qwik unstable and affects the majority of users label May 28, 2023
keuller added a commit to keuller/qwik that referenced this issue Jun 2, 2023
@keuller
Copy link
Contributor Author

keuller commented Jun 2, 2023

I opened the PR 4397 that fix it.
cc @manucorporat

@manucorporat
Copy link
Contributor

Wondering if this is an bug.
Might be important to also react when the submit failed.

We could add add onSubmitSucceed, onSubmitFailed

@keuller
Copy link
Contributor Author

keuller commented Jun 14, 2023

HI @manucorporat.

What is exactly your doubt if it is a bug or not? Can you explain in detail your thoughts?

Regarding the submit failure, I ran it many times the tests locally and it works, tbh I don't have any clue why it is failing via CI. I checked out the logs but I didn't find out the root cause.

I will review the code again and try to apply your suggestion: add onSubmitSucceed or onSubmitFailed.

Thank you to take a look at it.

@PatrickJS
Copy link
Member

the API should be reworked maybe in v2
some fixes done in #6241

@maiieul maiieul added COMP: qwik-city VERSION: after next major Features we'd want to add after the next version is done and removed STATUS-1: needs triage New issue which needs to be triaged P4: urgent If Bug - it makes Qwik unstable and affects the majority of users labels Aug 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
COMP: qwik-city TYPE: bug Something isn't working VERSION: after next major Features we'd want to add after the next version is done
Projects
Status: Backlog
Development

No branches or pull requests

4 participants