fix: bind:value and onInput$ on the same element part2 #8245
Conversation
|
built with Refined Cloudflare Pages Action⚡ Cloudflare Pages Deployment
|
commit: |
# Conflicts: # packages/qwik/src/optimizer/core/src/test.rs # packages/qwik/src/optimizer/core/src/transform.rs
Varixo
force-pushed
the
v2-fix-bind-input
branch
from
fe77b1e to
ab40948
Compare
wmertens
left a comment
wmertens
left a comment
There was a problem hiding this comment.
I think we need a different approach
| key?: string | number | null, | ||
| dev?: DevJSX | ||
| ): JSXNodeInternal<T> => { | ||
| return untrack(() => { |
There was a problem hiding this comment.
Didn't we decide we don't need the untrack?
Varixo
force-pushed
the
v2-fix-bind-input
branch
from
41e8906 to
beefac1
Compare
Varixo
force-pushed
the
v2-fix-bind-input
branch
from
33fc2fe to
9823fc7
Compare
wmertens
left a comment
wmertens
left a comment
There was a problem hiding this comment.
Actually the bind: processing should happen 100% at runtime because they merge and regular handlers don't.
Maybe: All handlers could be in a const object per element, updated by reference. No need for comparisons. The qwikloader uses that directly, but there's still issues with merging vs overriding, or removing a merged bind: when it's removed.
If we do that, then upon vnode instantiation, handlers should be read from props so that both const and var are populated in the handlers object. We still need the window and document handler attributes for queryselectors though, unless we attach those individually.
I must say I'm still unclear on what the correct behavior is :-/ Things would be a lot simpler if bind:x doesn't merge. We can output warnings when onInput and bind exist together.
| constProps.checked = value; | ||
| constProps['on:input'] = createQRL(null, '_chk', _chk, null, null, [value]); | ||
|
|
||
| // Handle bind:* - only in varProps (constProps are transformed by Rust) |
There was a problem hiding this comment.
Actually, bind: should not be transformed in Rust, because it needs to merge with on:input after all props are processed
There was a problem hiding this comment.
I think we currently need to be backward compatible, so the logic must stay as is for now - until changes in the future
The part2 of #8240