search

perscrew / react-native-form-validator

React native library to validate form fields
127 stars 55 forks source link

Package has stopped working entirely since moving my project to TypeScript #69

Closed kenny1983 closed 3 years ago

kenny1983 commented 3 years ago

Hi guys!

I posted another issue almost a day ago re: getErrorsInField not returning errors for a single character input, and now my problems seem to be getting worse.

I have since migrated my project across to TypeScript in the hope that strict type checking would help me solve this and a few other (unrelated to this package) errors that I'm getting. Well I don't know whether that's the culprit or not, but now getErrorsInField returns [] regardless of the amount of characters in the validated input (i.e. the whole package has become useless)!

I've been looking into this on my own since I haven't heard from anyone in almost a day, and I need this to have been fixed yesterday, not some time in the distant future. Please don't take that comment as an aggressive one; I understand that we all get busy and replying to whining users is probably at the bottom of everyone's to-do list 😉.

Anyway, I just noticed something strange in the code: https://github.com/perscrew/react-native-form-validator/blob/fb14cf8244622f3f4eb0ab27122cd1f38f8259cf/src/use-validation.js#L76-L81

I don't understand the use of two variables to track the errors that have been caught. Sure, errors is a ref while internalErrors is a state variable, so that is an obvious difference between them. But I guess I don't understand React well enough to understand why this would be needed. Nor have I ever seen any other examples of this type of design pattern in a React [Native] package.

Anyway, from what limited knowledge/experience I do have, it seems as though this is the cause of the errors I'm getting. Setting a debug breakpoint on line 81 and triggering it to be hit, I can see that errors.current is set to the expected array of 1 error object. But if I then hit F10 to step over that line, I end up with internalErrors still being set to an empty array!

Why is this occurring?? Could it have something to do with my TypeScript type definitions being wrong somehow?

Futhermore, if I continue stepping through the code until reaching the following: https://github.com/perscrew/react-native-form-validator/blob/fb14cf8244622f3f4eb0ab27122cd1f38f8259cf/src/use-validation.js#L96-L98

It becomes even more apparent (to me at least) that you guys should just use the errors ref variable, and get rid of the seemingly-useless internalErrors state variable. That isFormValid function returns false just like it should, but getErrorsInField relies solely on the incorrect state variable, meaning that validate returns false but getErrorsInField returns []!

I hope that this is a helpful bug report that solves a lot of problems for a lot of future users. I may even use Patch Package to fix this bug and submit a PR to you guys if you're interested 😉.

kenny1983 commented 3 years ago

@perscrew LOL never mind; I just noticed that you've actually released a v0.5.1 2 days ago that has the exact same fix as I'm proposing! What a crazy coincedence! 😄

You could have saved me a lot of time and effort by having the most recent code available on GitHub, and I don't know where, if anywhere, you have it publicly posted besides in NPM. If there is another public repository for the more recent versions, please update your npmjs.com page to reflect that! Anyway I guess this issue can be considered closed.

I just want to say this though: please think carefully about the way you're helping the community, and whether you actually are, by doing things the way that you currently are. You've written a very useful package here, and I thank you for your efforts, but it's kinda a waste of your own time and others' if you're going to half-ass things.

I hope you don't take that comment too personally, and just as it is intended: some constructive critism meant to help the larger community as a whole.

Thanks again for your work on this project! 👍

EDIT: I am officially an idiot. After proposing a solution via patch package and implementing one, I then stupidly forgot that I had done so. Meaning that when I installed v0.5.1, my fix was automatically applied LOL. But this leaves me even more confused...what have you actually changed in the new version?? It looks to me as though the use-validation.js files at least are exactly the same between v0.4.0 and v0.5.1 😕

perscrew commented 3 years ago

Hi @kenny1983, sorry for the delay. For your information, I prepared a new reworked version of this library with typescript.