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

Validation support #130

Open
dmatora opened this issue Jan 17, 2021 · 8 comments
Open

Validation support #130

dmatora opened this issue Jan 17, 2021 · 8 comments

Comments

@dmatora
Copy link

dmatora commented Jan 17, 2021

On regular input onChange I can do

		if (isNaN(value) || value < 0) {
			return;
		}

and it will not allow typing in letters or negative value
but on DebounceInput it will allow typing anything (while still preventing getting letters or negative into controlled var value) which basically breaks component

@nkbt
Copy link
Owner

nkbt commented Jan 17, 2021

Can you provide a working example please as without context I struggle to understand why this might happen. Thank you 🙏

@andybenedict
Copy link

andybenedict commented Jun 27, 2022

I have another situation that results in this rendering error occurring.

It is happening because of this check.

In short, the component only re-renders if a value is different than the previous value and different from the new state value. In @dmatora's case, he's disallowing negative or non-numbers, so when the value prop is not changed, it skips the re-render, but the entered value continues to display.

I'm getting this error when splitting off the decimal portion of a number to place in a companion dropdown (some users enter this value manually, so fractions are preferable to them, but other users are getting the data typed as decimals from a laser measurement tool). So I have a value of 21 and change it to 21.5, my script takes the floor and the mod 1 (21 and 0.5) and tries to update this component with the value 21 and a select box that displays .5 as 1/2, but because the previous value was 21, the component does not re-render, so the box continues to display 21.5

This could be corrected by changing the line to
if (typeof value !== 'undefined' && (oldValue !== value || stateValue !== value)) {
but it would largely negate the oldValue !== value check. The presence of a double check there suggests that it is dealing with some other edge case, but I don't know what it's there to handle, so I defer to your judgement rather than just submitting a pull request with that fix.

I couldn't glean from the commit what it was added for, but if it won't interfere with anything, I'd be happy to submit the PR.

@andybenedict
Copy link

andybenedict commented Jun 29, 2022

I think this is the most stripped down component that results in my exact problem.

import React, { FunctionComponent} from 'react';
import { DebounceInput } from 'react-debounce-input';

interface InputElement {
    data: {
        [index: string]: string | number | string[] | null;
    };
    source: string;
    callback: (key: string, value: any) => void;
}

const InputField: FunctionComponent<InputElement> = ({
    data,
    source,
    callback,
}: InputElement) => {
    const min = 20
    const max = 100

    const sanitize = value => {
        value = Number(value);

        if (min > value) {
            return Number(min);
        }

        if (max < value) {
            return Number(max);
        }
    }

    const handleOnChange = e => {
        const newValue = e.target.value !== ''
            ? sanitize(e.target.value)
            : null;
        return (source && callback(source, newValue));
    }

    return (
        <DebounceInput
            debounceTimeout={250}
            type={'number'}
            value={data?.[source] === null ? '' : data?.[source]?.toString()}
            onChange={handleOnChange}
        />
    );
};

export default InputField;

Call this passing in these props from a parent element:

const [data, setData] = useState({});

const source = 'myValue';

const callback = (key: string, value: any, label: string) => {
    setData((state) => ({ ...state, [key]: value }));
};

If you put in a number less than 20, it replaces the number with 20 and uses the callback to update the parent, then the component updates on re-render. Likewise if you go over 100, it replaces it with 100.

The issue arrives when I subsequently enter another number less than 20 or over 100... so for example, enter 101, which gets replaced by 100 then enter a couple more 0's 10000. The data object on the parent gets updated, but the re-render does not occur because the value prop has not changed, even though the internal state has changed and is now displaying incorrectly.

Repository owner deleted a comment from noisekit Jul 2, 2022
@nkbt
Copy link
Owner

nkbt commented Jul 2, 2022

@andybenedict please provide a working example in codepen. Use one from readme as an example: https://codepen.io/nkbt/pen/VvmzLQ?editors=0010

So far from reading the code and compiling it in my head - I cannot see the problem and believe it all comes from the the way how app behaves.

@andybenedict
Copy link

Sure thing, this concisely demonstrates the problem:
https://codepen.io/AndyBenedict/pen/zYWGKwe?editors=0010

I've given it a min of 10 and a max of 100... so enter any number between 10 and 100 and it will work normally. Any two consecutive numbers < 10 or > 100.

Try this sequence of numbers:
10 //works
20 //works
42 //works
8 //works, both state and component update to 10
7 //breaks, state updates to 10, but the component continues to say 7
110 //works, both state and component update to 100
1001 // breaks, state updates to 100, but the component continues to read 1001

I've provided a traditional input, you'll need to paste in the numbers because without the debouncing it's nearly impossible to type in a number.

@andybenedict
Copy link

andybenedict commented Jul 2, 2022

The callback is being executed. The output of the debounce, and its internal state, is changing, I've added a console log to the codepen to confirm.

Prior to v3.2.1, the check to compare the new value to the previous value did not exist. And if I load it up at v3.2.0, it works as expected. I unfortunately don't know enough about class based react to fully understand the commit.

Can you elaborate on what that was added to do? Previously if the new value contradicted its internal state it would re-render. Is the second check just an optimization? Or was there a bug or specific desired behavior that's necessary to accomplish. I can certainly supply a PR to remove the extra check, but I'm hesitant to do so without knowing why it was added. It's the classic adage of not tearing down a fence until you understand why it was put up.

@nkbt
Copy link
Owner

nkbt commented Jul 2, 2022

I don't know much more because it was written like 3 years ago... I think it was just an optimisation to avoid unnecessary re-renders. As long as existing examples work - it's all good.

@Meryovi
Copy link

Meryovi commented Nov 1, 2023

Is there any known workaround for this issue? I'm having the same issue described by @andybenedict

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

No branches or pull requests

4 participants