haacked / aspnet-client-validation

A client validation library for ASP.NET MVC that does not require jQuery
MIT License
236 stars 25 forks source link

v0.8.14 broke Umbraco Forms or "how to exclude forms" #127

Open jrunestone opened 2 months ago

jrunestone commented 2 months ago

HI!

I realize I haven't updated the package in a year, but I got around to it and now our generated "Umbraco Forms" (a UI form builder for Umbraco) forms don't submit in versions above 0.8.13 (nothing happens on click, no errors). "disabled" is added to the submit button when I click it but I suspect it might be Umbraco Forms that does some debouncing thing to prevent multiple submissions.

If I remove the creation of the ValidationService along with the call to bootstrap() the Umbraco Forms form will submit again, so it's something with the lib I think.

Anyway I tracked it down to 0.8.14 and there's a suspect fix there but I can't be sure of course. It seems to be happening in most browsers, not just FF.

This brings me to a question that I had while trying to fix this problem on my end - can I exclude certain forms from being considered by the ValidationService at all?

EDIT: I realize I haven't provided a repro, but it requires a full Umbraco site setup + Umbraco Forms. But maybe you have some ideas anyway.

jrunestone commented 2 months ago

I found the undocumented ValidationService::remove(form) but how is it supposed to work? I'm trying to remove my Umbraco Forms form completely from the service with that method but it still has entries in the ValidationService.formInputs array and the form still won't submit.

dahlbyk commented 1 month ago

Anyway I tracked it down to 0.8.14 and there's a suspect fix there but I can't be sure of course. It seems to be happening in most browsers, not just FF.

The suspect PR does seem most likely, as it changes validation from often synchronous to always asynchronous. But I have no idea why it would be a specific problem in an Umbraco world in a way that doesn't seem to be causing problems more generally. Presumably its relationship with some sort of Umbraco Forms event handling.

Shot in the dark: does behavior change if you rearrange where the bootstrap() happens relative to initializing Umbraco Forms? I assume the issue is fixed specifically by avoiding bootstrap(); just creating ValidationService should be harmless.

I found the undocumented ValidationService::remove(form) but how is it supposed to work?

AFAIK the old jQuery Validation didn't have any form/input caching, so remove(form) behavior is largely undefined/untested. But I would generally expect it to completely revert the form's behavior to as if the form had never been scanned/bootstrapped.

https://github.com/haacked/aspnet-client-validation/pull/110 in particular tried to fix that up, but I'm sure there are gaps.


Are you initializing your ValidationService with a logger, e.g. new ValidationService(console)? Any hints in the logs as to what is or isn't happening when you try to submit?

jrunestone commented 1 month ago

Thanks for replying!

I also believe it is due to Umbraco Forms including aspnet-client-validation on their end, but the fact remains it can be worked around by leaving out bootstrap and it's working before that particular version I mentioned.

Anyhow I would be happy to just be able to exclude certain forms from even being bootstrapped by my initialization, because right now I can't have 2 forms (UC/non-UC) on the same page or they will conflict :) The remove(form) doesn't seem to do anything. Is it too far fetched of an idea to have the ability to config a selector for which forms I want the validation service to target? E.g form-selector: '[data-validate-form]'.

I'll try the v0.10.0 release using remove(form) again to see if that works. I've also not used the logger yet, I'll see what I can find.

jrunestone commented 1 month ago

I attached a logger and the behavior is a little more sinister than I thought :) Upon clicking submit on an Umbraco Forms form the validation service I created (not Umbraco Forms) goes into an infinite validation loop for some reason (it doesn't crash the browser though).

image

I did however try the latest package, 0.11.0, and did remove(form) after bootstrapping, and now I'm able to reliably remove the Umbraco Forms forms from the validation service and they work as they should. It's a bit backwards and I would prefer to be able to to just select the forms I want to validate at config, but this works.

dahlbyk commented 1 month ago

It's a bit backwards and I would prefer to be able to to just select the forms I want to validate at config, but this works.

We should support "don't scan" better. Ideas:

  1. Update options.root to accept null to opt out of initial scan
  2. Add options.scanOnInit (default: true) to explicitly opt out

https://github.com/haacked/aspnet-client-validation/blob/061c242280ef535bbaadd7e5384695aba119eb3b/src/index.ts#L1409-L1422

In the meantime, you can specify a different root to scan on bootstrap, e.g. document.head shouldn't have any forms.

jrunestone commented 1 month ago

Sure, I think option 2 is the better of the two. Thanks for the tip, it looks like I can then manually call scan(form) for each form I want to enable validation for. That would be an acceptable solution, albeit slightly unintuitive to new users.. Maybe if we had a scanForms(<selector>) or something ;)

This should work: v.bootstrap({ root: document.head }) myForms.forEach(v.scan);