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

request: Add label support to validators, constraints, etc #201

Open
1 task done
vladfrangu opened this issue Sep 29, 2022 · 1 comment
Open
1 task done

request: Add label support to validators, constraints, etc #201

vladfrangu opened this issue Sep 29, 2022 · 1 comment
Labels

Comments

@vladfrangu
Copy link
Member

Is there an existing issue or pull request for this?

  • I have searched the existing issues and pull requests

Feature description

One of the biggest issues with shapeshift right now is that when running a validator of the form s.string.parse(input), the error received is not really useful (it just says that the validator failed, not why or where or for what).

Other modules (notably ow uses callsites to infer the label from the callsite) either allow this or use some interesting mechanisms to detect/infer the label. Since we aim for speed ⚡ here, we should probably stick to letting users provide a label (or even a label factory?)

Desired solution

Support for adding labels on validators (mayhaps s.string.label('url') instead of s.string('label') as the latter is breaking which is not ideal).

Would be nice to also have something like s.string.label((input) => translate('bad-input', input)) to allow dynamic labels in one predicate (for instance for the 1 person planning on using shapeshift in an i18n context)

Alternatives considered

Not supporting this, which is a downside...so none :D

Additional context

No response

@favna
Copy link
Member

favna commented Sep 29, 2022

While doing it in a way that is non-breaking through labels I think we should also explore doing it with a breaking change which then allows a yup-like style (https://github.com/jquense/yup#error-message-customization) such as:

s.string('must be a string').url('Expected a valid URL');

s.string('must be a string').regex(/[0-9]/, 'Expected the input value to only consist of numbers');

s.number('must be a number').min(12, 'Age has to be at least {{age}}')
fold out for added context

I dont think it conflicts but personally I'd prefer a yup-style which is one of:

s.string.url('Expected a valid URL');

s.string.regex(/[0-9]/, 'Expected the input value to only consist of numbers');

s.number.min(12, 'Age has to be at least {{age}}')

With the last one {{age}} is a reference to the minimum value so people don't need to do

`Age has to be at least ${12}`

This may seem really useless but lets look at how an enterprise application might structure their application:

// constants.ts
export const MAXIMUM_DOG_AGE = 20;
export const MAXIMUM_GOLD_FISH_AGE = 15;
export const MAXIMUM_CAT_AGE = 30;

// enums.ts
export enum AnimalType {
  Dog,
  Cat,
  GoldFish,
}

// functions.ts
import { MAXIMUM_DOG_AGE, MAXIMUM_GOLD_FISH_AGE, MAXIMUM_CAT_AGE } from "./constants";
import { AnimalType } from "./enums";

export function getMaximumAnimalAge(animal: AnimalType): number {
  switch (animal) {
    case AnimalType.Dog:
      return MAXIMUM_DOG_AGE;
    case AnimalType.Cat:
      return MAXIMUM_CAT_AGE;
    case AnimalType.GoldFish:
      return MAXIMUM_GOLD_FISH_AGE;
  }
}

// base-validations.ts
export const animal = s.object({
  name: s.string,
});

// validations.ts
import { getMaximumAnimalAge } from "./functions";
import { animal } from "./base-validations";
import { AnimalType } from "./enums";

export const dog = animal.extend({
  age: s.number.lessThanOrEqual(getMaximumAnimalAge(AnimalType.Dog), "The maximum age of a dog is {{age}}"),
});

export const goldfish = animal.extend({
  age: s.number.lessThanOrEqual(getMaximumAnimalAge(AnimalType.GoldFish), "The maximum age of a gold fish is {{age}}"),
});

export const cat = animal.extend({
  age: s.number.lessThanOrEqual(getMaximumAnimalAge(AnimalType.Cat), "The maximum age of a cat is {{age}}"),
});

Now lets say we're a few months further and the customer says they want to change the message to The maximum age is {{age}}. Refactoring this code becomes way easier with the templating because the developer can simply search for {{age}} and find all such validations, even if they have been scattered across various files over the months


For the record, Yup does templating like this: https://github.com/jquense/yup#error-message-customization so with ${ref}. But I personally think {{ref}} is better because

  1. God forbid code base that uses backticks everywhere as default quote, they literally cannot use ${ref} without escaping.
  2. If you refactor your code to use backticks for whatever reason then either your ref is suddenly a variable, or when using IntelliJ it escapes it automatically but you probably didn't really want that either.
  3. Using double curly braces means the strings can be pulled 1:1 from i18next, meaning there is full compatibility.
  4. Double curly braces is also pretty standard across languages I think

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

No branches or pull requests

2 participants