HuckRidgeSW / hvue

A GopherJS & go/wasm binding for Vue.js
MIT License
49 stars 10 forks source link

Computed property with setter only working on app, not on Component #7

Closed mame82 closed 6 years ago

mame82 commented 6 years ago

The custom Config field Setterswhich is represented by hvue_setters in JS is only populated with *js.Object, when NewVM is called. When NewComponent is called, the field isn't populated and thus doesn't exist in JS context. Ultimately using hvue.ComputedWithGetSet gets impossible in NewComponent as the field doesn't exist.

This ends up with ...

TypeError: c.Object.hvue_setters is undefined

.. when trying to define a computed property with Setter in NewComponent.

Creating the missing object is easily fixed, thus I'll file a PR doing it. Anyway, in my Opinion the Setter field on Config isn't needed at all and should be removed. The defined setter is used as intended, as soon as the property is written to. VueJS itself doesn't make the Setters accessible directly, at all.

It would be better to remove this line completly: https://github.com/HuckRidgeSW/hvue/blob/master/vm.go#L29

... and of course the write access from here: https://github.com/HuckRidgeSW/hvue/blob/master/vm.go#L48

The reason I'm not doing this in the PR: I'm not sure if there's any code using your project, which relies on the presence of Config.Setter.

HuckRidgeSW commented 6 years ago

PR #8 merged. Thanks!

I use Setters to make sure that any later uses of VM.Set() are setting valid fields.

One of the points (to me, at least) of GopherJS is to make JS a little safer, so if you typo a struct field name, for example, the compiler tells you. Since you pass VM.Set() a string, you could typo the field name and the compiler wouldn't know; having the Setters field list valid fields with setters lets hvue at least check for that at runtime.