enterwell / react-form-validation Goto Github PK

Blocked by following tasks:

• Adde stage branch
• #71
• Set protection (no merge without PR and review)

Tasks after blocking:

• #70

Issue

Type for the validationFn argument is defined as () => boolean or () => Promise<boolean>. Which is not true because the validation function receives an input value

Issue

A common issue, or a feature if you would, on many different forms is to keep track of the field values and only enable Save or some other CTA button if the values have changed since the form initialization.

Proposed

Consider returning an additional isDirty or isChanged property that would indicate if the value changed from its initial (default) value.

Also consider giving an option to the user to override this default value after validation initialization because most of the time we create inputs using empty strings and then once we fetch data, trigger the change manually.

    1) "after all" hook: generateReport for "submitForm_remoteInvalid"


  12 passing (15s)
  1 failing

  1) formUtils
       "after all" hook: generateReport for "submitForm_remoteInvalid":
     CypressError: `cy.task('coverageReport')` failed with the following error:

> Cannot read property 'loc' of undefined

Maybe we can update cypress

• use env test for testing in .babelrc as documented here

• v1.11.1

import { isNotEmptyString } from '@enterwell/react-form-validation';

Suggestion

Maybe offer better message when validation function is undefined

Each time after dependabot's PR is merged, build and publish are triggered and publish fails because package's version isn't bumped.

We should see what's the best solution for this problem.

Issues

Assigning initialValues is not possible when form data is still loading. There is no way to assign values after data is loaded except calling onChange on all fields. Calling onChange on all fields doesn't work as expected because we have to wrap values in expected event structure or pass receiveEvent: false. Another thing bad in that solution is that reset doesn't work as expected and resets form to initialValues instead of data we loaded.

Proposed

Expose new function setValues that will change initialValue for all fields and use them as initial values instead of ones provided in hook.

Use case:

const formData = {
    name: useValidation('', isNonEmptyString)
};

const item = useItem();
useEffect(() => {
    if (item) {
        setValues(formData, {
            name: item.name
        });
    }
}, [item]);

Additionally, this utility would require field hook to expose setValue which would actually do the bulk of work. So you can manually set values for each field as shown bellow.

const name = useValidation('', isNonEmpryString);

const item = useItem();
useEffect(() => {
    if (item) {
        name.setValue(item.name);
    }
}, [item]);

Issue

According to the docs, the second parameter for the submitForm form util function has (any) => void type, which is not true based on the typings

Tasks:

• [CI] Disable dependabot
• [CI] Enable Renovate
• [CI] Configure Renovate to use stage branch

Emails made of only capital letters should be valid, but it seems that current regex doesn't allow that.

Two conditions are combined with || operator but should be with &&.

More common used validation function. Would be nice it's part of the library.

If we want to update documentation or change something that is not package related - package release should be skipped.

Issue

Currently you can't spread object returned by hook useValidation to components because it contains additional properties that are not expected by most components.

Proposed

Return additional property props which would contain only properties that most input components accept.

props should contain:

• value
• onChange
• onBlur

const name = useValidation('', isNonEmpryString);

...

<Input {...name.props} />

Issue

Documentation states that the useValidation hook can be used by just passing in the default value and the validation function.
However I get a typescript error when trying to do so because the typings state that the third argument is mandatory.

EDIT: Also, there seems to be a typings error on returned onChange and onBlur callbacks. As stated in the typings each of them accepts a mandatory config value which clashes if spread on inputs as they do not pass in that value on their own.

EDIT2: It would also be benefitial if those config objects required in the onChange and onBlur callbacks other than being optional, had all props optional as well so that we can configure either one of them (instead of explicitly specifying them all).

Proposed

Change the FieldConfig argument to be optional

I don't think that react-dom is necessary for this library to work, although it comes in handy while developing and testing it. My suggestion is to move it from peerDependencies to devDependencies.

Use case

We have input that validates something with the server (eg. check if username already exists). It would be nice to have indicator so user known we're working on it and it's not yet confirmed.

Current state

Currently you can mimic this behaviour with following code (note the example doesn't have all features requested in this feature request, eg. 50ms delay for transitioning into validating state):

const [isUserNameValidating, setIsUserNameValidating] = useState(false);

const userName = useValidation('', validateUnique);

useEffect(() => {
    const validateUserName = () => {
        setIsUserNameValidating(true);
        await userName.validate();
        setIsUserNameValidating(false);
    };
    validateUserName();
}, [username.value]);

Please correct me if there's simpler way validating on blur

Proposal

useValidation would return one additional value validating: boolean.
The value is by default false, only changed to true when executing long running Promise validation function (eg. min. 50ms). If validation function returns in less than that - don't change state to true. Set to false immediately after validation.

Example usage:

const userName = useValidation('', validateUnique);
const isUserNameValidating = userName.validating;

Proposed

So that we can trigger on change without needing the event (virtual change) or with custom logic that config knows to handle.

const name = useValidation(name, isNonEmptyString)

name.onChange(newValue, { receiveEvent: false });

Useful when we don't actually get the event from used component, so we don't have to mock the event like { target: { value: newValue } }.

https://stackoverflow.com/a/53710641/563228

We are transitioning from Cypress to Playwright internally so it would be nice to use same tools in our OS projects too.

Current usage:

<MyInput
    error={formData.name.error}
    helperText={formData.name.error ? 'Field \'Name\' is required.' : ''}
    ... />

Which is fine when we have one validation on field. But when there is more than one validator, it would be nice to show user which one is failing eg. have validation message/text.

Proposed new format:

t is translator function here

<MyInput
    error={formData.name.error}
    helperText={formData.name.errorMessages.map(t).join('. ')}
    ... />

{value: "", error: false, errorMessages: ["required"], onChange: ƒ, onBlur: ƒ, validate: ƒ, reset: f}

Each validator would correspond to different message in "keyed" format so it can be passed forward to localization functions.

Validation function could return the errorMessages item as string. Validation needs to be changed. When returned value is typeof string there is error (like current false value), and when there is no error typeof boolean && === true is expected. For validators that are not updated (for backward compatibility) default error message would be used when they return false.

Currently

In the step 2 of the documentation's usage section the longer way of passing the value, onChange and onBlur properties is shown.

Proposed

Consider making it explicitly obvious that there is a much easier and shorter way of doing so by spreading the props property the validation hook returns

Acoording to discussion on PR #6 validateFields expects object in following format:

validateFields({
  validation1: {
    value: any,
    validate: (any) => boolean
  },
  validation2: ...
});

instead of what is stated in documentation:

validateFields({
    value: any,
    validate: (any) => boolean
});