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.

Sign up for GitHub

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

Jump to bottom

Looks like abortEarly doesn't abort async parsing #954

Open
victordidenko opened this issue Dec 3, 2024 · 3 comments · May be fixed by #956
Open

Looks like abortEarly doesn't abort async parsing #954

victordidenko opened this issue Dec 3, 2024 · 3 comments · May be fixed by #956
Assignees
@fabian-hiller
Labels
enhancement New feature or request priority This has priority

Comments

@victordidenko
Copy link

In the following code I expect that parsing will be stopped right away, because email field is not valid, and abortEarly is set to true. But parsing awaits for the whole 10 seconds before returning result.

Ideally I expect that promise will not be even started in that case.

And the related question:
Is there a way to abort running promise? Using AbortController or something?

import * as v from 'valibot';

const Schema = v.objectAsync({
  email: v.pipe(v.string(), v.trim(), v.email()),
  username: v.pipeAsync(
    v.string(),
    v.nonEmpty(),
    v.maxLength(30),
    v.checkAsync(() => new Promise((res) => {
      console.log('request');
      setTimeout(() => res(true), 10000)
    }))
  ),
});

const result = await v.safeParseAsync(Schema, {
  email: '',
  username: 'qwerty',
}, {
  abortEarly: true,
  abortPipeEarly: true,
});

console.log(result);

https://valibot.dev/playground/?code=JYWwDg9gTgLgBAKjgQwM5wG5wGZQiOAcg2QBtgAjCGQgbgCh6BjCAO1XgGUmALAUxDI4AXkwA6CBQBWfJjACCqAJ6smACgDe9OHAHJgpAFziwwMHzUYxHKMFYBzNQEoANOJi2Qzt1b0HnrtpwAK6ofFCsyCB8xlam5ooq6kE6VjZ2joE6qWKsbACi4DBK3iniggAeADJ8DjA8agDMAAxZ2Va8sgDWiapqziIAfHCsfADucAAKeCDAYf1QfKhOQ3Ba2dks7BCkfGKkEI6EiwCOwUs0TgwbOmEwACqgfBDBMP0rwsOLqGoe565wACMzRBrTKAF8nE4goFIQxmGwOHBvsFSPBRMgxvp4GlkNg+JNkFAwr11Nx+II3OtdIIDMZCIQXEFQuFItF6ScxuFioz6OCqUFkFRYPkiaQlMY-nwmTohdAYJMzHxRVBxZKoOcmXDGFtUDs9gdHCi0Vd6EA
@fabian-hiller
Copy link
Owner

Hey 👋 thanks for reaching out! The current behavior is expected, but it is true that we should investigate if we can improve it by also aborting the promises. Would you be interested in investigating this? Here is the objectAsync source code: https://github.com/fabian-hiller/valibot/blob/main/library/src/schemas/object/objectAsync.ts#L114-L123

@fabian-hiller fabian-hiller self-assigned this Dec 3, 2024
@fabian-hiller fabian-hiller added enhancement New feature or request priority This has priority labels Dec 3, 2024
@victordidenko
Copy link
Author

Ah, I see the issue: it executes all schemas asynchronously using Promise.all, awaiting all of them to resolve before checking any abortEarly flag.
I can try to investigate it but cannot promise success 😅

@yumauri yumauri linked a pull request Dec 3, 2024 that will close this issue
Open
@victordidenko
Copy link
Author

victordidenko commented Dec 3, 2024 •
edited
Loading

I made proof-of-concept for early abort of async object validation, can you check a PR #956, please?
I will add few more tests tomorrow

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@fabian-hiller fabian-hiller

Labels
enhancement New feature or request priority This has priority
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix 954 yumauri/valibot
2 participants
@victordidenko @fabian-hiller