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

feat: configuration validation #2133

Draft
wants to merge 25 commits into
base: main
Choose a base branch
from
Draft

Conversation

achingbrain
Copy link
Member

Closes #1573
Closes #1757
Closes #1903

I decided to go with yup for a few reasons:

  • Integration was more seamless
  • Is used by substantially more projects ( 545k vs 105k)
  • Preferred the schema building via chaining as opposed to defining complex functions (especially for defining integers, mins and defaults)
  • Was more actively maintained ( most recent commit 3 weeks ago as opposed to last year November
  • More downloads ( 4,201,246 vs 889,754 weekly downloads)

Co-authored-by: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com>
packages/libp2p/src/config/helpers.ts Outdated Show resolved Hide resolved
packages/libp2p/src/autonat/index.ts Outdated Show resolved Hide resolved
packages/libp2p/src/circuit-relay/server/index.ts Outdated Show resolved Hide resolved
packages/libp2p/src/connection-manager/utils.ts Outdated Show resolved Hide resolved
packages/libp2p/src/connection-manager/utils.ts Outdated Show resolved Hide resolved
packages/libp2p/src/dcutr/dcutr.ts Outdated Show resolved Hide resolved
packages/libp2p/src/fetch/index.ts Outdated Show resolved Hide resolved
packages/libp2p/src/identify/identify.ts Outdated Show resolved Hide resolved
packages/libp2p/src/ping/index.ts Outdated Show resolved Hide resolved
packages/libp2p/src/upnp-nat/index.ts Outdated Show resolved Hide resolved
@maschad maschad requested a review from wemeetagain October 10, 2023 20:52
@maschad maschad marked this pull request as draft November 21, 2023 03:43
@maschad maschad marked this pull request as ready for review November 24, 2023 04:12
packages/libp2p/src/connection-manager/utils.ts Outdated Show resolved Hide resolved
packages/libp2p/src/address-manager/utils.ts Outdated Show resolved Hide resolved
packages/libp2p/src/config/config.ts Outdated Show resolved Hide resolved
packages/protocol-dcutr/src/dcutr.ts Outdated Show resolved Hide resolved
packages/protocol-dcutr/src/dcutr.ts Outdated Show resolved Hide resolved
@achingbrain achingbrain force-pushed the main branch 9 times, most recently from 242fd96 to bca8d6e Compare November 30, 2023 21:12
@maschad maschad requested a review from wemeetagain December 7, 2023 15:06
@maschad maschad force-pushed the feat/config-validation branch from b0f60e7 to 94e19d8 Compare December 7, 2023 19:06
maschad
maschad previously approved these changes Jan 14, 2024
@achingbrain
Copy link
Member Author

Did a bit of bundle analysis of different config validators.

Zod

👍 Great TS support, can derive output types with no tweaking
👍 Big user base - 10M downloads/week
👎 Tree shaking doesn't work so is very large

import { object, array, function as func, custom } from 'zod'

const validateMultiaddr = (value?: string): boolean => {
  try {
    multiaddr(value ?? '')
  } catch (err) {
    return false
  }

  return true
}

const multiaddrStringArray = array(custom<string>(validateMultiaddr)).default(() => [])
const multiaddrArray = array(custom<Multiaddr>(validateMultiaddr)).default(() => [])

const configValidator = object({
  listen: multiaddrStringArray,
  announce: multiaddrStringArray,
  noAnnounce: multiaddrStringArray,
  announceFilter: func().args(multiaddrArray).returns(multiaddrArray).default(() => defaultAddressFilter)
})
image

Yup

😐 TS support not amazing (see use of unknown in return types)
😐 Reasonable user base - 6M downloads/week
👎 Tree shaking doesn't work so is very large

import { object, array, string, mixed } from 'yup'

const validateMultiaddr = (value?: Array<string | undefined>): boolean => {
  try {
    (value ?? []).forEach(value => multiaddr(value ?? ''))
  } catch (err) {
    return false
  }

  return true
}

const multiaddrStringArray = array().of(string()).test('is multiaddr', validateMultiaddr).default([])

const configValidator = object({
  listen: multiaddrStringArray,
  announce: multiaddrStringArray,
  noAnnounce: multiaddrStringArray,
  announceFilter: mixed().default(() => defaultAddressFilter)
})
image

Valibot

👍 Tiny addition to the bundle
👍 Great TS support, can derive output types with no tweaking
👎 Small user base - 200k downloads/week
👎 Parts of API missing (function validation, for example)

import * as v from 'valibot'

const validateMultiaddr = (value?: any): boolean => {
  try {
    multiaddr(value ?? '')
  } catch {
    return false
  }

  return true
}

const multiaddrStringArray = v.optional(v.array(v.custom<string>(validateMultiaddr)), [])

const configValidator = v.object({
  listen: multiaddrStringArray,
  announce: multiaddrStringArray,
  noAnnounce: multiaddrStringArray,
  announceFilter: v.optional(v.any(), defaultAddressFilter)
})
image

Superstruct

👍 Tiny addition to the bundle
👍 Great TS support, can derive output types with no tweaking
😐 Reasonable user base - 1.3m downloads/week

import { define, object, array, func, defaulted, assert } from 'superstruct'

const validateMultiaddr = (value?: any): boolean => {
  try {
    multiaddr(value ?? '')
  } catch {
    return false
  }

  return true
}

const multiaddrString = define('MultiaddrString', validateMultiaddr)
const multiaddrStringArray = defaulted(array(multiaddrString), () => [])

const configValidator = object({
  listen: multiaddrStringArray,
  announce: multiaddrStringArray,
  noAnnounce: multiaddrStringArray,
  announceFilter: defaulted(func(), defaultAddressFilter)
})
image

Zod is working on v4 which will improve tree shaking. Valibot is still under active development but it's early days yet.

I think revisit this in a few months time, or after Zod v4 is released and reassess.

@maschad
Copy link
Member

maschad commented Jun 13, 2024

Good call @achingbrain I agree, let's revisit once Zod has greater stability.

@maschad maschad marked this pull request as draft June 13, 2024 14:28
@maschad maschad self-assigned this Jun 13, 2024
@maschad maschad dismissed their stale review June 13, 2024 14:29

We aree exploring other potential libraries

@achingbrain achingbrain force-pushed the main branch 2 times, most recently from 6453a80 to c2bc7fe Compare September 10, 2024 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🧱Blocked
3 participants