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

onValidationComplete sometimes errors when inputs are removed #267

Closed
sdemjanenko opened this issue Nov 18, 2015 · 11 comments
Closed

onValidationComplete sometimes errors when inputs are removed #267

sdemjanenko opened this issue Nov 18, 2015 · 11 comments
Labels

Comments

@sdemjanenko
Copy link
Contributor

The inputKeys points to keys that no longer exist in this.inputs.

@farisj
Copy link

farisj commented Nov 24, 2015

Having this same problem.

@meikoudras
Copy link

+1

1 similar comment
@josercruz01
Copy link

+1

@Semigradsky Semigradsky added the bug label Dec 1, 2015
@dsteinbach
Copy link

+1

Seems to be a race condition. I have the following (mock) setup:

<ul>
   {workExp.map(function(exp, index){
      <li>
         <Input name={"work_exp[" + index + "].title"} value={exp.title} />
         <button onClick={removeWorkExp.bind(self, index)}>X</button>
         {exp.skills.map(function(skill, subindex){
            <Input name={"work_exp[" + index + "].skills[" + subindex + "]"} value={skill} />
         })}
     </li>
   })}
</ul>

When the user clicks X to remove a work experience, work_exp[0].title is unmounted first and validateForm is run too soon and is trying to validate this.inputs["work_exp[0].skills[0]"] which also has been unmounted.

The error is on this line:

https:/christianalfoni/formsy-react/blob/master/src/main.js#L373
"Uncaught TypeError: Cannot read property 'state' of undefined"

Perhaps we want that line to be more defensive?

@Semigradsky
Copy link
Collaborator

Hi! The problem arises from the fact that the library is heavily dependent on the input names.
Therefore, problems occur when changing the name of the input.

I tried to get rid of it. See #275

@Semigradsky
Copy link
Collaborator

@sdemjanenko @farisj @meikoudras @josercruz01 Hi! Could you test #275 on your code? If everything is fine, I will merge it.

@sdemjanenko
Copy link
Contributor Author

@Semigradsky: test still pass on #258 so all good 😄

@farisj
Copy link

farisj commented Dec 7, 2015

@Semigradsky looks good!

@josercruz01
Copy link

All good on my side as well @Semigradsky, thanks a lot for the fast response on this.

@dsteinbach
Copy link

Worked for me, thanks!

@C3PablO
Copy link

C3PablO commented Mar 1, 2016

Works nice! Thanks! When are you planning next release?

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

No branches or pull requests

7 participants