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

Support for translation options #632

Open
rreckonerr opened this issue May 10, 2021 · 8 comments
Open

Support for translation options #632

rreckonerr opened this issue May 10, 2021 · 8 comments

Comments

@rreckonerr
Copy link

Hi! First of all want to thank you for a great validation library!

Currently, I'm in a process of migrating my ember app to your validations library and I'm struggling with error keys emitted by custom enforce methods.

I'm migrating from an older validation library so I have to support their translations API. It includes translation keys and passed translation options. It seemed like a good idea to extend enforce with methods that'll match the previous translations, but I can't find a way of providing additional data to send along with the transition key.

Let's say I have an ICU translation defined as follows:

{
  "errors": {
    "greater-than-or-equal-to": "At least {gte} options must be selected"
  }
}

And I'd like to return an object with additional data as a result message message

const greaterThanOrEquals = (received, floor) => {
  return {
    pass: received >= floor,
    message: () => ({ key: "errors.greater-than-or-equal-to", gte: floor }), // something like this
  };
};

Currently, I'm translating error messages from enforce like this

  get errorMessages() {
    return Object.entries(this.validator.getErrors()).map(
      ([fieldName, [errorKey]]) =>
        this.intl.t(errorKey, {
          description: fieldName,
        })
    );
  }

but unfortunately, it doesn't work if I try to return an object as the enforce result message.

Do you have any idea how to work around this? I can provide custom enforce methods with additional context and return translated message as a result, but there are few problems with this approach

  1. There's no access to the name of the field from within enforce method (e.g. test("Field Name", ....))
  2. If I'll provide additional data to the enforcers and vest won't be that much framework-agnostic anymore
@ealush
Copy link
Owner

ealush commented May 20, 2021

Hey, thank you for reaching out! Sorry for taking so long to reply.

This is a tough one. The reason it doesn't work for you when you try to return an object from your enforce message is that Vest specifically looks for a string thrown within the test body.

May I ask how you're going to use it in the test itself? Maybe an alternative would be to not use the message api in your extended enforce, and instead only translate the test itself.

test('username', translationFunction('translation_key'), () => {
  enforce(value).gte(10)
});

Would something like that work for you?

@rreckonerr

This comment has been minimized.

@rreckonerr
Copy link
Author

rreckonerr commented May 23, 2021

Oh, sorry, you obviously can define multiple tests for the same field, just like

test('username', t('errors.greater-than-or-equal-to', { gte: 10 }), () => {
  enforce(value).gte(10)
});

test('username', t('errors.less-than-or-equal-to', { lte: 5 }), () => {
  enforce(value).lte(5)
});

test('username', t('errors.email', { description: 'This field' }), () => {
  enforce(value).isPresent()
});

So... hmm, it appears to be more of a feature request than an issue. Like, a way to pass context for the enforce rules

@ealush
Copy link
Owner

ealush commented May 23, 2021

OK, so I gave it some thought.

First, as you noted - yes, you can write multiple tests for the same field, and in general, that's the preferred "vest" way of doing it - because it provides you the most granular control over your tests.

Second - enforce intentionally doesn't "speak" with vest, or draw any context from it - enforce can be consumed as its own separate package, and as such, it should not rely on Vest for its normal function.

However - you CAN pass extra information to your enforce rules in the form of extra params:

enforce.extend({
    equals: (value, arg, options) => {
		return {
			pass: value === arg,
			message: () => t('errors.equals', options)
		}
	}
})

enforce(1).equals(1, { description: 'this field' });

Enforce rules can be given an infinite number of arguments, so it is infinitely extendable if needed.

Of course, this is not ideal - but on the other hand, that's not a very common use case that I haven't encountered yet apart from this requirement - but it may come up more as more people start using Vest.

Let's keep this issue open as a feature request for a while, and if it receives enough traction within a couple of months I'll add it to the future of Vest's roadmap (currently working on major version 4, so it could be a nice milestone to add such a feature).

@rreckonerr
Copy link
Author

Thanks for checking this out! Yeah, that's exactly the way I'm dealing with translations within enforce right now.

@roblevintennis
Copy link
Contributor

roblevintennis commented May 17, 2022

Hey, I don't know if this is helpful, but I arrived here trying to add translations into my use of Vest too!

I found a solution which feels pretty straight forward:

  1. Simply pass in the translation function as an additional arg to create e.g. t the third argument and then use it for any test message arg:
const suite = create((data = {}, currentField, t) => {
  only(currentField);

  test("usernameVue", t('usernameRequired'), () => {
    enforce(data.usernameVue).isNotBlank();
  });
...

Also note I of course used my i18n mapping calling t('someKey').

Here's how I called it (same as Vest examples plus the additional arg):
result.value = suiteVue(formState, fieldName, t);

I liked this because there was no need to extend enforce which feels like it would require a change also in my usage.

  1. I found an interesting edge case…say I had english selected, then caused a field error hint e.g. "Username required". Then, I selected Spanish. Result: Error hint still in English.

I was able to solve this (in Vue) by simply adding a reactive watcher and "revalidating" any touched fields:

watch(currentLanguage, () => {
  const fields = Object.keys(touched); // note touched is an object literal with all fields and booleans indicating if "touched" which essentially means blured and interacted with
  fields.forEach((field) => validate(field));
});

In Svelte this could be $: and in React it could be a useEffect...every framework will have a way to do this.

My code is pretty complicated because I'm putting together a multi-framework demo in Astro that uses Astro, Svelte, and Vue 3 components. That said, it might be worth looking at. It only has translations between English and Spanish (in case you play with it):

https://github.com/AgnosticUI/agnosticui/tree/master/playgrounds/DemoSignup

I believe this code sandbox should work too:
https://codesandbox.io/s/github/AgnosticUI/agnosticui/tree/master/playgrounds/DemoSignup?file=/README.md

So I think it's very much possible to use Vest and pass in your localize or translation function and it will just work.

@ealush
Copy link
Owner

ealush commented May 17, 2022

Nice, @roblevintennis!

Here's an idea for a perf improvement.

I saw this in your example:

watch(currentLanguage, () => {
  const fields = Object.keys(touched); // note touched is an object literal with all fields and booleans indicating if "touched" which essentially means blured and interacted with
  fields.forEach((field) => validate(field));
});

A thing to note is that the only() function can take an array of field names source. If you modify your validate function to not use them one by one, you can omit the forEach call and run your suite just once on language change.

only(["fieldA", "fieldB"]);

Why does that matter?

Each time you run your suite, you generate a new summary object. This seems trivial, but really, it sums up all the fields and tests, creates new references, and recreates the runtime functions. If you use "only" for all touched fields at once, then Vest will only have to rebuild the summary once.

Any other suggestions?

You could use the translation key as the error message and pass it to the translation function in the UI. But that's really a matter of taste.

test("username", "errors.username_too_short", () => {
  enforce(data.username).longerThan(2)
});
const res = suite.get();

return (
	<span>{t(res.getErrors("username")[0])}</span>
)

@roblevintennis
Copy link
Contributor

roblevintennis commented May 18, 2022

I really dig the advice on the only call especially. But I have a question which requires showing more of the code I think:

const validate = (fieldName) => {
  if (touched[fieldName]) {
    result.value = suiteVue(formState, fieldName, t); // <---- As a result of this call is where `only` kicks in on the suite side
    result.value.done((res) => {
      result.value = res;
    });
  }
};

watch(currentLanguage, () => {
  const fields = Object.keys(touched);
  fields.forEach((field) => validate(field));
});

And then in suiteVue.ts I have:

import { create, test, enforce, warn, only } from "vest";

const suite = create((data = {}, currentField, t) => {
  only(currentField); // <--- only here is set up to do "per field" validation
  //...and then several test assertions
export default suite;

I do want to be able to generally do per field validation as the nominal case. The only time (I think) that I want to do the only(arrayOfFields) is when the language has changed (the watch in my example).

All to ask, is the best way forward to create another suite which takes the array of fields and update the only? So:

export const allFieldsSuite = create((data = {}, fieldsArray, t) => {
  only(fieldsArray);

But then is there a way to remove duplication if I do this since I'd then have two suites? The test assertions are likely the same so it'd be nice to be able to share those.

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

No branches or pull requests

3 participants