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

Implement observable.error as an observable and not as a plain string variable #173

Closed
fbuchinger opened this issue Nov 20, 2012 · 5 comments

Comments

@fbuchinger
Copy link

I've spent five hours yesterday chasing down a bug similar to #47 ('required' message was displayed instead of the expected custom validators message). No I have not yet managed to reproduce this bug in a jsFiddle, but during my debugging sessions i noticed that the .error property of the validated observable contained the correct message, while the message inserted on the screen was wrong.

To me this seemed like a timing issue (insertMessage handler seems to be called miliseconds BEFORE error message gets updated correctly)

I wondered why observable.error is just a plain string variable instead of an observable... If the insertMessage handler referenced this observable, it could update the error message even it was changed after the creation of the error insert.

@TerenceLewis
Copy link
Contributor

@fbuchinger I had a similar problem previously - see PR 105 (https://github.com/ericmbarnard/Knockout-Validation/pull/105). Unfortunately I've been unable to pinpoint the root cause (it was a while ago but I believe I also came to the conclusion that it was timing-related).

In my scenario making observable.error an observable has made the problem go away, but I haven't chased up on the PR because I wasn't sure how widespread this bug was and I don't know whether there are implications I'm not aware of (performance, etc.?) in making the error property observable.

@fbuchinger
Copy link
Author

@TerenceLewis Thanks for this wonderful patch! Seems to solve my issues :) I'm still a bit cautious, because as we both know this bug is quite unpredictable and non-reproduceable. But my validation will undergo heavy testing in the next days and I will notify you and Eric once the patch has survived more extensive tests.

@fbuchinger
Copy link
Author

@TerenceLewis @ericmbarnard : Terence's patch worked for us even in rigid testing. I can only recommend to merge it into knockout-validation.

@alpendergrass
Copy link

@TerenceLewis @ericmbarnard : This patch fixed a similar issue for me. Looking forward to a merge. Thanks guys.

@crissdev
Copy link
Member

Fixed by #248

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