Add $model magic property #3914
Conversation
| @@ -13,6 +13,11 @@ export function initInterceptors(data) { | |||
|
|
|||
| if (typeof value === 'object' && value !== null && value._x_interceptor) { | |||
| obj[key] = value.initialize(data, path, key) | |||
| } else if ((typeof value === 'object' || typeof value === 'function') && value !== null && value._x_accessor) { | |||
| Object.defineProperty(obj, key, { | |||
There was a problem hiding this comment.
Wouldn't it be smarter to just adjust the interceptor interface to handle this? Instead of having two different kinds of interceptor types?
Have just one.
(I'm more partial to the getter/setter method than the existing interceptor interface with the passing of getter setters from the initialize to the interceptor callback...)
There was a problem hiding this comment.
Yeah, that's a good point. I agree it's extra overhead, but it would take reworking all of the interceptors that aren't really meant to be getters/setters.
In a perfect world, there is one unified interface. I'll explore it a little but I want to keep all the existing stuff stable
| let func = generateModelAccessor(el, cleanup) | ||
|
|
||
| Object.defineProperty(func, 'closest', { get() { | ||
| let func = generateModelAccessor(el.parentElement, cleanup) |
There was a problem hiding this comment.
Seems this is likely to end up just being exactly the same as the above, no?
There was a problem hiding this comment.
This is used by x-model so that there isn't an infinite loop when using x-model="$model". So yah, it's the same, except it's only generated when needed and it references elements above the current one to prevent that infinite loop. I don't see a problem with it, do you?
| return closestModelEl._x_model | ||
| } | ||
|
|
||
| accessor.get = () => { |
There was a problem hiding this comment.
Could it be better to have these use the normal DOM event system? Would need a lot less upfront work, and less internal hackery. x-model will already be listening on something, no?
How does this currently handle when inputs exist inside a modeled tree? would a change event from the input not also trigger the model?
There was a problem hiding this comment.
I can't use the event system because that only covers the "set" story, not the "get" story.
And you're right, x-modelable automatically removes event listeners, so I think I need some kind of solution for that problem here.
Options:
A) provide a dedicated API to remove listeners: $model.removeListeners() or something
B) automatically cleanup listeners when $model.get() is called for the first time
C) add a modifier to x-model like: x-model.no-events or some better name
I think B is my favorite - it's not a breaking change when introducing this feature, but would be after introducing the feature.
Thoughts?
There was a problem hiding this comment.
Right, that is one direction 😵
The initial binding of $model would make the most sense. There'd be no risk of a different event bubbling before this gets triggers, in the event the tree is deeper for custom components.
Might have some conflict, but it has a guarantee of being consistent.
|
|
||
| This way you can freely use and modify the newly defined property `value` property within the nested component and `.get()` and `.set()` will be called internally. | ||
|
|
||
| > You may run into errors when using `$model` within `x-data` on the same element as the `x-model` you are trying to reference. This is because `x-data` is evaluated by Alpine before `x-model` is. In these cases, you must either ensure `x-model` is on a parent element of `x-data`, or you are deffering evaluation with `this.$nextTick` (or a similar strategy). |
| </div> | ||
| ``` | ||
|
|
||
| Now everytime `count` changes, the newest count value will be logged to the console. |
| function generateGetAndSet(evaluateLater, scopeTarget, expression) { | ||
| let evaluateGet = evaluateLater(scopeTarget, expression) | ||
| let evaluateSet | ||
|
|
||
| // @todo: This is nasty | ||
| window.fromModel = true | ||
| mutateDom(() => bind(el, 'value', value)) | ||
| delete window.fromModel | ||
| if (typeof expression === 'string') { | ||
| evaluateSet = evaluateLater(scopeTarget, `${expression} = __placeholder`) | ||
| } else if (typeof expression === 'function' && typeof expression() === 'string') { | ||
| evaluateSet = evaluateLater(scopeTarget, `${expression()} = __placeholder`) | ||
| } else { | ||
| evaluateSet = () => { } | ||
| } |
There was a problem hiding this comment.
Personally I'd move the default evaluateSet condition up to the top and kill the else.
let evaluateSet = () => {}| function generateModelAccessor(el, cleanup) { | ||
| // Find the closest element with x-model on it, NOT including the current element... | ||
| let closestModelEl = findClosest(el, i => { | ||
| if (i._x_model) return true |
There was a problem hiding this comment.
or just even i => i._x_model since it's a truthiness check here.
| return (el.tagName.toLowerCase() === 'select') | ||
| || ['checkbox', 'radio'].includes(el.type) |
There was a problem hiding this comment.
Could also just add select-one and select-multiple to the array of types to check.
| delete window.fromModel | ||
| if (typeof expression === 'string') { | ||
| evaluateSet = evaluateLater(scopeTarget, `${expression} = __placeholder`) | ||
| } else if (typeof expression === 'function' && typeof expression() === 'string') { |
There was a problem hiding this comment.
Could it be problematic that expression is evaluated again here?
…n $model is inside x-data
No description provided.