Enforcing validatable fields on validationElement and validationMessage seems overkill

[brettmorien]

We are running across a breaking change between knockout.validation 1 and 2 where fields tagged in markup with validationElement or validationMessage are throwing when there are no validation rules attached. This doesn't seem necessary.

Firstly, it forces a coupling between markup and backing .js to ensure any applied rules are accompanied by markup, but absence of rules means a throw.
Second, it complicates creating shared components which may or may not have validated dynamic fields in the absence of an isValidatable utility, which would end up pretty boilerplate anyway.

Our particular case is a property grid, generated by some Asp.Net code. Without coupling the .cs/.cshtml to the backing .js, I'm not sure how to determine when to add the validationElement.

fix would be to remove the 2 instance of:

    if (!obsv.isValid || !obsv.isModified) {
        throw new Error("Observable is not validatable");
    }

Though, I can't vouch for what terrible effects that would cause since its introduction in this commit: dba75b6

I'm willing to work a pull request removing this seemingly unnecessary check, if necessary. It feels analogous to the difference between a null collection and an empty collection. Calling code shouldn't need to go through the extra trivia of checking code.

[john-jay]

I have run into this problem also. I have a code base with
validationElement used extensively, for example in a string editor
template. So it would be difficult to figure out where it could be removed.

On Friday, January 23, 2015, askheaves [email protected] wrote:

We are running across a breaking change between knockout.validation 1 and
2 where fields tagged in markup with validationElement or validationMessage
are throwing when there are no validation rules attached. This doesn't seem
necessary.

Firstly, it forces a coupling between markup and backing .js to ensure any
applied rules are accompanied by markup, but absence of rules means none.
Second, it complicates creating shared components which may or may not
have validated dynamic fields in the absence of an isValidatable utility,
which ends up pretty boilerplate.

Our particular case is a property grid, generated by some Asp.Net code.
Without coupling the .cs/.cshtml to the backing .js, I'm not sure how to
determine when to add the validationElement.

fix would be to remove the 2 instance of:

if (!obsv.isValid || !obsv.isModified) {
    throw new Error("Observable is not validatable");
}

Though, I can't vouch for what terrible effects that would cause since its
introduction in this commit: dba75b6
dba75b6

I'm willing to work a pull request removing this seemingly unnecessary
check, if necessary. It feels analogous to the difference between a null
collection and an empty collection. Calling code shouldn't need to go
through the extra trivia.

—
Reply to this email directly or view it on GitHub
#519.

John Holliday, Jr. - 2213 W 120th St, Leawood, KS 66209 - 816.210.2184

[crissdev]

Seems like a good idea to me - currently placing validationMessage and validationElement bindings before the observable becomes validatable is not possible. Also, I think this should be considered a bug and not a feature.

@askheaves Do you think we can have this for 2.0.1 release? Consider checking the milestones. Thanks.

[brettmorien]

Running into a couple issues reconciling unit tests between command-line build and browser. Will squash the entire changeset before the pull.