aurelia / validatejs

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

Adding decorators results in an exception #22

Closed MaximBalaganskiy closed 8 years ago

MaximBalaganskiy commented 8 years ago

Adding a required decorator on a view model property results in an exception

export class EditStable{
    reporter: any;

        constructor() {
               this.reporter = ValidationEngine.getValidationReporter(this.stable);
        }

    @required
    Name: string = "";
}
aurelia-logging-console.js:86 ERROR [app-router] Error: TypeError: Cannot read property 'initializer' of undefined
        at deco (http://localhost:1506/jspm_packages/npm/aurelia-validatejs@0.2.0/base-decorator.js:16:22)
        at base (http://localhost:1506/jspm_packages/npm/aurelia-validatejs@0.2.0/base-decorator.js:40:14)
        at required (http://localhost:1506/jspm_packages/npm/aurelia-validatejs@0.2.0/decorators.js:24:36)
        at __decorate (http://localhost:1506/app/views/editStable.js:7:114)
        at execute (http://localhost:1506/app/views/editStable.js:92:13)
    Error loading http://localhost:1506/app/views/editStable.js

I'm transpiling TS into es6 with module type system

EisenbergEffect commented 8 years ago

@PWKad This is likely a difference in the way that TS transpiles and handle property descriptors vs. Babel.

EisenbergEffect commented 8 years ago

Actually, I don't think that TS always provides the property descriptor. That's probably the issue. If one isn't provided your going to need to handle that situation.

EisenbergEffect commented 8 years ago

(Strictly speaking, I think this is non-spec compliance on the TS part most likely, but we need to handle it unfortunately.)

MaximBalaganskiy commented 8 years ago

I don't think that TS always provides the property descriptor

That's exactly what happens. It only gives a descriptor for properties

EisenbergEffect commented 8 years ago

Right. Even though the spec is changing. In the old spec, I believe there should alway be a descriptor in this case. So, TS is probably fudging it a little bit. We can fix our implementation to account for it though.

MaximBalaganskiy commented 8 years ago

:+1:

searus commented 8 years ago

Is there a workaround for this?

plwalters commented 8 years ago

Does this seem like it would appropriately handle the situation -

https://github.com/aurelia/binding/blob/master/src/decorator-observable.js#L8-L20

Basically if it isn't babel just make the descriptor an object?

searus commented 8 years ago

just tested applying that logic into validatejs/base-decorator and it solved the problem for me

EisenbergEffect commented 8 years ago

yes

plwalters commented 8 years ago

I'm not sure the best way to represent this in a unit test but I pushed out a fix. @searus can you double-check and make sure that commit looks the exact same way when you implemented it? Thanks!

searus commented 8 years ago

Looks identical to what I have - except you have nice comments :+1: