NewOldMax / react-form-validator-core

Core validator component for react forms
MIT License
94 stars 44 forks source link

Fix placement of console.log #88

Closed rwieruch closed 3 years ago

rwieruch commented 3 years ago

This issue was mentioned over here and here, but got never addressed.

After I introduced this.getErrorMessage in our project, we got flooded by the unknown errorMessages type loggings even though everything worked as expected. After I checked the library's code, I found out that the logging happens even though type === 'object' evaluates to true for our errorMessages array.

However, the undesired logging still happens because the inner condition this.invalid.length > 0 can still evaluate to false and thus we wouldn't return early and therefore would end up in the logging -- which shouldn't happen for type === object.

In order to prevent this situation, we should only perform the logging if one of the type conditions don't apply.

NewOldMax commented 3 years ago

@rwieruch hi can you give a real code example where this logging happens?

rwieruch commented 3 years ago

It's weaved into our private code base, so I cannot give a reproducible example :( However, regarding the code, this logging should popup every time errorMessages is an array and this.invalid.length > 0 evaluates to false, because we don't return then and run into this console.log.

NewOldMax commented 3 years ago

I believe you can write minimal example in codesandbox For my experience, you can see that console.log in use only when you do something wrong So I want to see how do you achieve it

rwieruch commented 3 years ago

I need to ask my client if I can allocate time on this, otherwise we just have to work on a fork :(

FWIW, if it helps, this is what we are flooded with:

unknown errorMessages type []
11:26:51.275 0.chunk.js:247892 unknown errorMessages type []
11:26:51.291 0.chunk.js:247892 unknown errorMessages type []
11:26:51.295 0.chunk.js:247892 unknown errorMessages type []
11:26:51.298 0.chunk.js:247892 unknown errorMessages type []
11:26:51.313 0.chunk.js:247892 unknown errorMessages type []
11:26:51.327 0.chunk.js:247892 unknown errorMessages type (3) ["Required field.", "Not a valid number.", "The number is too low."]
11:26:51.330 0.chunk.js:247892 unknown errorMessages type (3) ["Required field.", "Not a valid number.", "The number is too low."]
11:26:51.598 0.chunk.js:247892 unknown errorMessages type ["Required field."]
11:26:51.618 0.chunk.js:247892 unknown errorMessages type []
11:26:51.622 0.chunk.js:247892 unknown errorMessages type []
11:26:51.625 0.chunk.js:247892 unknown errorMessages type []
11:26:51.656 0.chunk.js:247892 unknown errorMessages type []
11:26:51.681 0.chunk.js:247892 unknown errorMessages type []
11:26:51.684 0.chunk.js:247892 unknown errorMessages type []
11:26:51.686 0.chunk.js:247892 unknown errorMessages type (3) ["Required field.", "Not a valid number.", "The number is too low."]
11:26:51.691 0.chunk.js:247892 unknown errorMessages type (3) ["Required field.", "Not a valid number.", "The number is too low."]
11:26:52.384 0.chunk.js:247892 unknown errorMessages type ["Required field."]
11:26:52.386 0.chunk.js:247892 unknown errorMessages type []
11:26:52.392 0.chunk.js:247892 unknown errorMessages type []
11:26:52.410 0.chunk.js:247892 unknown errorMessages type []
11:26:52.413 0.chunk.js:247892 unknown errorMessages type []
11:26:52.415 0.chunk.js:247892 unknown errorMessages type []
11:26:52.417 0.chunk.js:247892 unknown errorMessages type []
11:26:52.420 0.chunk.js:247892 unknown errorMessages type (3) ["Required field.", "Not a valid number.", "The number is too low."]
11:26:52.437 0.chunk.js:247892 unknown errorMessages type (3) ["Required field.", "Not a valid number.", "The number is too low."]
11:26:52.865 0.chunk.js:247892 unknown errorMessages type ["Required field."]
11:26:52.869 0.chunk.js:247892 unknown errorMessages type []
11:26:52.910 0.chunk.js:247892 unknown errorMessages type []
11:26:52.914 0.chunk.js:247892 unknown errorMessages type []
11:26:52.917 0.chunk.js:247892 unknown errorMessages type []
11:26:52.920 0.chunk.js:247892 unknown errorMessages type []
11:26:52.923 0.chunk.js:247892 unknown errorMessages type []
11:26:52.930 0.chunk.js:247892 unknown errorMessages type (3) ["Required field.", "Not a valid number.", "The number is too low."]
11:26:52.936 0.chunk.js:247892 unknown errorMessages type (3) ["Required field.", "Not a valid number.", "The number is too low."]
11:26:53.139 0.chunk.js:247892 unknown errorMessages type ["Required field."]
11:26:53.146 0.chunk.js:247892 unknown errorMessages type []
11:26:53.149 0.chunk.js:247892 unknown errorMessages type []
11:26:53.183 0.chunk.js:247892 unknown errorMessages type []
11:26:53.185 0.chunk.js:247892 unknown errorMessages type []
11:26:53.195 0.chunk.js:247892 unknown errorMessages type []
11:26:53.198 0.chunk.js:247892 unknown errorMessages type []
11:26:53.203 0.chunk.js:247892 unknown errorMessages type (3) ["Required field.", "Not a valid number.", "The number is too low."]
11:26:53.214 0.chunk.js:247892 unknown errorMessages type (3) ["Required field.", "Not a valid number.", "The number is too low."]
11:26:54.197 0.chunk.js:247892 unknown errorMessages type ["Required field."]
11:26:54.213 0.chunk.js:247892 unknown errorMessages type []
11:26:54.218 0.chunk.js:247892 unknown errorMessages type []
11:26:54.221 0.chunk.js:247892 unknown errorMessages type []
11:26:54.246 0.chunk.js:247892 unknown errorMessages type []
11:26:54.248 0.chunk.js:247892 unknown errorMessages type []
11:26:54.251 0.chunk.js:247892 unknown errorMessages type []
11:26:54.253 0.chunk.js:247892 unknown errorMessages type (3) ["Required field.", "Not a valid number.", "The number is too low."]
11:26:54.256 0.chunk.js:247892 unknown errorMessages type (3) ["Required field.", "Not a valid number.", "The number is too low."]

In a parent component, we are using:

import { ValidatorForm } from 'react-material-ui-form-validator';

...
const My Component = () => {
  ...

  return (
    <ValidatorForm onSubmit={handleSubmit} 
      {formControls}
    </ValidatorForm>
  );
};

And in our formControl components, we are using the validator the following way:

import { ValidatorComponent } from 'react-form-validator-core';

export default class FormControl extends ValidatorComponent {
    render() {
        const {
            error,
            errorMessages,
            validators,
            requiredError,
            helperText,
            textArea,
            maxDate,
            minDate,
            shrink,
            validatorListener,
            withRequiredValidator,
            ...rest
        } = this.props;
        const { isValid } = this.state;

        return (
            <FormControl
                className="formControl"
                margin="normal"
                error={!isValid || error}
                {...rest}
            >
                <InputLabel
                    htmlFor={this.props.textArea ? 'textarea' : 'text'}
                >
                    {this.props.label}
                </InputLabel>
                <Input
                    name={this.props.name}
                    autoComplete={this.props.name}
                    type={this.props.type}
                    value={this.props.value}
                    defaultValue={this.props.defaultValue}
                    rows={this.props.textArea ? this.props.rows || 3 : 1}
                    rowsMax={this.props.textArea ? this.props.rows || 3 : 1}
                    multiline={this.props.textArea}
                />
                <FormHelperText>{this.getErrorMessage()}</FormHelperText>
            </FormControl>
        );
    }
}

Everything works as expected with the validation. But already upon rendering, because we are using this.getErrorMessage, we get flooded by these messages.

NewOldMax commented 3 years ago

@rwieruch you should use getErrorMessage in the following way:

if (!isValid) {
    this.getErrorMessage()
}

as mentioned in examples

rwieruch commented 3 years ago

Thanks! I missed this piece, because I didn't wrote this code in the first place. So this makes https://github.com/NewOldMax/react-form-validator-core/pull/89 redundant too.

Issue solved and both PRs could be closed. Thanks for your help! However, I wonder if merging these two PRs would leave you with less people running into these issues. Because as mentioned above, there were these kind of issues before. And imo I think using this.getErrorMessage() shouldn't raise so many issues when not shielding it with isValid. It should just return an empty string if no error is present.