aurelia / validatejs

Enables expressive validation using decorators and/or a fluent API.
MIT License
22 stars 23 forks source link

Parameterless validation decorators are disabled in Chrome #71

Closed gbegerow closed 8 years ago

gbegerow commented 8 years ago

With the validationsample the require decorator is not working (simply seems to do nothing). Declaration ist @required lastName = 'Skywalker'; with usage <input class="form-control" value.bind="lastName & validate" />
requiered was imported with import {required} from 'aurelia-validatejs';

It shows the Problem is the testing for optional arguments. From the original sourcecode static presence(config = true) some transpiler (I assume babel) created the following test:

var config = arguments.length <= 0 || arguments[0] === undefined ? true : arguments[0];

It tests for arguments[0] exactly undefined, but it gets null in Chrome 50.0.2661.102. So instead of supplying the default argument of true it sends null to validate.js/presence, which disables the test. This will be the case for every parameterless validator.

The simple workaround is to change the call to use true as argument, like

 @required(true)  lastName = 'Skywalker';

If I read the spec right, chrome should return undefined instead of null.

plwalters commented 8 years ago

Good catch. I wonder if there is a way we can open an issue on Chrome / V8.

For now I think you're right, passing in true is (unfortunately) the best option.

mttmccb commented 8 years ago

I suspect this is the same as https://github.com/aurelia/validatejs/issues/54, I'll try this fix on it and see if it sort it out, yes it works!

plwalters commented 8 years ago

Indeed

gbegerow commented 8 years ago

Very strange thing this bug. Tried to replicate it in isolation instead of debugging. When I try to test

function a(x) { "use strict"; return arguments.length <= 0 || arguments[0] === undefined ? true : arguments[0]; }; console.log(a());

in different browser console, all of them give arguments[0] as undefined. Even Chrome. Will investigate further.

gbegerow commented 8 years ago

Of cause I was barking at the wrong tree. I still do not see very clearly here because of me missing some understandings about how decorators work. But the bug seems to be in the base-decorator.js

arguments[0] is null not because of Chrome setting it, but because of base-decorator setting targetOrConfig to null explicitly if key is undefined.

If there is no parameter to the decorator, base is called with key 'lastName' and thus assigns null to targetOrConfig. If there is a parameter, key is undefined and the config parameter is passed through. That's why the workaround works. But I don't know how the base decorator is called, so I don't understand the behavior of the key parameter.

I assume, changing base-decorator to assign undefined to targetOrConfig instead of null would solve the problem, but I am not sure about further consequences.

taz commented 8 years ago

Just a note that for me this issue seems to have been resolved with 0.5.0.