HerringtonDarkholme / av-ts

A modern, type-safe, idiomatic Vue binding library
MIT License
216 stars 11 forks source link

[feature request] make @prop more concise #36

Closed Evertt closed 7 years ago

Evertt commented 7 years ago

Right now a property is defined like so:

@Prop aRequiredNumber = p({
  type: Number,
  required: true
})
// or
@Prop aStringWithDefault = p({
  type: String,
  default: 'Hello World!'
})
// or
@Prop anWithObjectDefault = p({
  type: Object,
  default() {
    msg: 'Hello World!'
  }
})

But this could all be much more concise, like so:

@Prop aRequiredNumber: number // required is inferred since no default value was given
// or
@Prop aStringWithDefault = 'Hello World!' // the type could be inferred from the default value
// or
@Prop anWithObjectDefault = { // the decorator can put this default value into a function, the developer doesn't have to worry about that...
  msg: 'Hello World!'
}

I have forked another repository to add this feature to it and I succeeded. However, I found out I like this repository more, so if you'd let me then I'd happily add that feature to this one too.

HerringtonDarkholme commented 7 years ago

Actually it does not provide features like required or validator.

It fails with cases like @Prop anOptional: number when no default value is available nor the prop functionality is required. Think about an optional label in an pop-up window or so.

Default value in initializer is also problematic. @Prop repeatedEvaluated = sideEffectCall() will unexpectedly incur side effect.

I have considered these patterns when designing this library. But it turns out the current approach is the best compromise, well, a compromise any way.

HerringtonDarkholme commented 7 years ago

BTW, the state of art TS techniques for MVVM library will probably take advantage of mapped types. https://github.com/HerringtonDarkholme/vivio is an experiment to exploit that.

Evertt commented 7 years ago

Yes it does fall short in a number of cases and in those cases you could just add an override in the @Prop, like so:

@Prop({required: false}) aRequiredNumber: number

However, I'm pretty sure that this override wouldn't be necessary for +80% of cases. The main reason for this feature request is that for the +80% of cases where such an override is not necessary I like to remove as much visual noise as possible. Don't you think this is a better compromise?

HerringtonDarkholme commented 7 years ago

Personally I'm not interested in adding more class/decorator style API surface because alternative API provides type safety without compromise.

Your proposal does not handle side-effectful call. This isn't acceptable.

Evertt commented 7 years ago

I don't see a problem there. It's possible to make sure the side-effect function isn't called until / unless its really necessary, right? Maybe I'm mistaken. I'll just fork this repo and do some experiments myself.

HerringtonDarkholme commented 7 years ago
function call() {console.log('side effect'); return 123;}

@Component class T extends Vue {
  @Prop prop = call()
}

Try this.

Evertt commented 7 years ago

Okay I see what you mean, the function gets called whether its result is useful for the default value or not.

Evertt commented 7 years ago

I just figured out that I think it's possible to solve that problem by creating a wrapping function, like so:

function call() {console.log('side effect'); return 123;}

@Component class T extends Vue {
  @Prop prop = resultOf(call)
}

Which is still much more concise than your solution. So yeah, I'm gonna build that into my fork.