BorisMoore / jsviews

Interactive data-driven views, MVVM and MVP, built on top of JsRender templates
http://www.jsviews.com/#jsviews
MIT License
856 stars 130 forks source link

know if converter is called in a read or write operation #431

Closed johan-ohrn closed 4 years ago

johan-ohrn commented 5 years ago

Consider the following expression {myConverter: whatever :myConverter} It would be helpful to know in the myConverter function if it's called as a result of rendering the value or writing it back to the view model. This would allow the same converter to be used for both reading and writing instead of having to create two versions.

BorisMoore commented 5 years ago

Thanks for pointing this out. That said, I'm not sure how often people will be using the same converter for convert and for convertBack. (More often of course, they have inverse semantics, like fromString and toString). And of course if they are the same, one can indeed create a clone with a different name.

But if you do use the same converter, it is I think possible to detect from the context which role it is being called in. In one case you have this.parentElem.value === arguments[0], (or this.linkcCtx.elem.value === arguments[0]), and in the other you have this.tagCtx.args[0] === arguments[0]. Not very robust perhaps, and not very intuitive, but possible...

Otherwise, do you have a suggested API that would be intuitive?

johan-ohrn commented 4 years ago

Most of our converters do follow the fromXXX / toXXX naming scheme but for some cases it doesn't feel intuitive and sometimes I'd like to just use the same converter for both read/write.

Perhaps you could extend "this" (which seem to be a subset of a Tag) with a property that lets the converter function know which role it's being called in? That way the details would be hidden away by jsviews. Such as if (this.isWrite) ... or if (this.isRead) .... What do you think about this?

johan-ohrn commented 4 years ago

FYI. this.linkCtx.elem.value === arguments[0] seem more robust because this.parentElem is not always set at the time the converter is called.

BorisMoore commented 4 years ago

The 'this' pointer is the instance of the {{:}} tag (tagName = ":") and that instance persists across calls, so it is the same object during the convert as during the convertBack. So to set this.isWrite to true would need to be done as a transient property, added by JsViews before calling the converter, then removed after that call, so that it does not persist through to when convertBack is called. Similarly for the this.isRead = true, on calling convertBack. Nowhere in JsViews do we have any such (public) transient properties. Also it would need to be documented here https://www.jsviews.com/#jsvtagobject, and could apply to any tag using converters, not just the {{:}} tag. But logically we could expect similar transient properties during other calls, (e.g. other tag methods or event handlers) not just for convert and convertback calls...

It would be simpler if we could just pass an additional boolean converter argument, cnvt(... isConvertBack) but in fact converters already support multiple arguments...

So I'm hesitant to add this API. It seems reasonable to me that you can simply clone your converter with a different name for use as convertBack. Or else if that is really not an option, fall back to using the above not very intuitive test... I'm not yet seeing a better solution to this....

johan-ohrn commented 4 years ago

Ah I see what you mean. I thought the 'this' pointer was to a transient "converter tag" and not the tag instance the converter is being used on. I'll just stick with the approach you mentioned.