Skip to content

feat: support AbortSignal #1203

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

feat: support AbortSignal #1203

wants to merge 3 commits into from

Conversation

rhyzx
Copy link

@rhyzx rhyzx commented May 24, 2025

import * as v from "valibot"

const schema = v.pipeAsync(
  v.string(),
  v.checkAsync(async (v, signal) => {
    console.log(`request start: ${v}`)
    const res = await fetch("https://httpbin.org/delay/2", { signal })
    console.log(`request end: ${v}`)
    return res.ok
  }),
)

const abortA = new AbortController()
v.parseAsync(schema, "a", { signal: abortA.signal }).catch((err) => {
  if (err instanceof Error && err.name === "AbortError") {
    console.log("aborted: a")
  } else {
    throw err
  }
})
abortA.abort()

const abortB = new AbortController()
v.parseAsync(schema, "b", { signal: abortB.signal })

Expected:

  1. "request start: a"
  2. "aborted: a"
  3. "request start: b"
  4. "request end: b"

Copy link

vercel bot commented May 24, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
valibot ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 24, 2025 10:22am

@rhyzx
Copy link
Author

rhyzx commented May 24, 2025

A more universal approach I’ve just thought of is to use generic config and pass it as an additional parameter to the async actions. eg.

v.parseAsync(
  v.pipeAsync(
    v.string(),
    v.checkAsync(async (v, { signal, foo }) => {
      console.log(foo) // 1
      const res = await fetch("https://httpbin.org/delay/2", { signal })
      return res.ok
    }),
  ),
  "123",
  {
    signal: AbortSignal.abort(),
    foo: 1,
  },
)

Which approach would you prefer?

@fabian-hiller
Copy link
Owner

Thank you for this PR! My plan is to provide general support for AbortSignal for any asynchronous validation, which would give us more control. This would also resolve issue #954. While we should keep this PR focused, we should bear this in mind when making decisions. Perhaps we could support AbortSignal without making breaking changes for v1.2 to individual actions, and release a full AbortSignal support for Valibot v2.

@fabian-hiller fabian-hiller self-assigned this May 26, 2025
@fabian-hiller fabian-hiller requested a review from Copilot May 26, 2025 02:09
@fabian-hiller fabian-hiller added the enhancement New feature or request label May 26, 2025
@fabian-hiller fabian-hiller added this to the v1.2 milestone May 26, 2025
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Adds AbortSignal support across asynchronous validation and transformation flows, enabling early termination of pipes and checks.

  • Introduces an optional signal in the shared Config and propagates it through all async schemas and actions.
  • Updates pipeAsync, lazyAsync, customAsync, checkAsync, transformAsync, partialCheckAsync, and checkItemsAsync to forward and respect the signal.
  • Extends tests to verify that getter functions and pipelines receive the signal and that aborting causes the proper errors.

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.

Show a summary per file
File Description
library/src/types/config.ts Added signal?: AbortSignal to Config
library/src/storages/globalConfig/globalConfig.ts Merged config.signal into the global config object
library/src/storages/globalConfig/globalConfig.test.ts Added default signal field in config tests
library/src/schemas/lazy/lazyAsync.ts Extended getter signature to accept signal
library/src/schemas/lazy/lazyAsync.test.ts Updated tests to assert signal is forwarded
library/src/schemas/custom/customAsync.ts Extended check callback to accept signal
library/src/methods/pipe/pipeAsync.ts Called config.signal?.throwIfAborted() before items
library/src/methods/pipe/pipeAsync.test.ts Added abortSignal test for pipeline
library/src/methods/parse/parseAsync.test.ts Added abortSignal test in parseAsync
library/src/actions/types.ts Updated ArrayRequirementAsync signature
library/src/actions/transform/transformAsync.ts Extended operation to accept signal
library/src/actions/partialCheck/partialCheckAsync.ts Extended requirement to accept signal
library/src/actions/checkItems/checkItemsAsync.ts Forwarded signal in map calls
library/src/actions/check/checkAsync.ts Extended requirement to accept signal
Comments suppressed due to low confidence (2)

library/src/storages/globalConfig/globalConfig.test.ts:16

  • [nitpick] The test only verifies the default undefined signal. Add a case where store.signal is set (and config.signal omitted) to ensure the fallback to store.signal works as expected.
signal: undefined,

library/src/storages/globalConfig/globalConfig.ts:38

  • When merging global config, you should preserve a stored default signal if config.signal is undefined. Consider using signal: config?.signal ?? store?.signal to maintain fallback behavior.
signal: config?.signal,

Copy link

pkg-pr-new bot commented May 26, 2025

Open in StackBlitz

npm i https://pkg.pr.new/valibot@1203

commit: bf3dc4b

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants