feat(compiler-core): allow directive modifiers to be dynamic (fix #8281) #12913
Conversation
|
I developed on top of the main branch but the guidelines say to submit against minor. Now I have this extra commits in my history. Not sure how to handle this |
fixed by merging main into the minor branch. |
Size ReportBundles
Usages
|
@vue/compiler-core
@vue/compiler-dom
@vue/compiler-ssr
@vue/compiler-sfc
@vue/reactivity
@vue/runtime-core
@vue/runtime-dom
@vue/server-renderer
@vue/shared
vue
@vue/compat
commit: |
|
Thank you very much for your contribution. This PR is really great. I did a quick test and found the following issues.
the following template <template>
<Comp v-model.[mod]="msg"/>
</template>should be compiled to: _createVNode($setup["Comp"], {
modelValue: $setup.msg,
"onUpdate:modelValue": _cache[1] || (_cache[1] = $event => (($setup.msg) = $event)),
- modelModifiers: { "$setup.mod": true }
+ modelModifiers: Object.assign({}, $setup.mod)
}, null, 8 /* PROPS */, ["modelValue"])
the following template <template>
<Comp v-model.number.[mod]="msg"/>
</template>should be compiled to: _createVNode($setup["Comp"], {
modelValue: $setup.msg,
"onUpdate:modelValue": _cache[1] || (_cache[1] = $event => (($setup.msg) = $event)),
- modelModifiers: { number: true, "$setup.mod": true }
+ modelModifiers: Object.assign({number:true}, $setup.mod)
}, null, 8 /* PROPS */, ["modelValue"])
the following template should not cause a compilation error. see Playground with this PR <Comp v-model.[{ number: true }]="msg"/> |
|
@edison1105 thanks for the super fast feedback. Am I correct to assume, that your failing cases are specific to the v-model directive or does it happen with any other? (it shouldnt) For the last error: The "expression missing" error shadows the "whitespace forbidden" error. The same happens if you use whitespaces in the dynamic argument: When I tried try this with the template-explorer it correctly showed the error for other directives that are not v-on or v-model: Since this is already bahavior thats valid for args, maybe its ok for modifiers as well? Wdyt? I am not against allowing spaces in dynamic args and modifiers but I am pretty sure it would break formatters and syntax highlighters. I guess thats also the reason why its not supported. I would have written a test for that as well but I couldnt find where to put it. |
|
I have a bit of duplicated code now. Let me know if I should refactor that and if so, where I put helper functions for such things |
We can just put them in |
|
Had to complicate the if statements a bit to deduplicate the logic but I think its all good now. |
|
/ecosystem-ci run |
|
📝 Ran ecosystem CI: Open
|
|
Thanks @Fuzzyma |
|
Seems like I broke some runtime directive in vuetify :D. Have to check that... Guess I have some more work to do. What is the process for this? Do I open a PR in language tools to support the new ast shape and get that merged before this PR gets merged? |
|
@edison1105 can i somehow checkout this PR ins vue-sfc to test it easily? |
We can use the following version to install the PR preview package npm i https://pkg.pr.new/vue@12913
// or
npm i https://pkg.pr.new/vue@e4fd227 // latest commit
We should wait for this PR to be shipped. |
Co-authored-by: Jian Zhang <dsonet@msn.com>
|
Hi, Thanks for this PR 👍 Can you guys please check this StackBlitz playground to see if it's okay to pass down pre-defined
To the custom Input component? https://stackblitz.com/edit/vitejs-vite-cga23nxl Updated the StackBlitz, added input with This PR is so great, please consider merging it sooner 💯 |
|
@sadeghbarati what exactly should we test in that example? It seems to work just fine, right? I need more context |
|
I am here to ask whether this is a good practice or not? Cause I saw somewhere that was mentioned the modelValue modifiers should be re-defined in the child component even if the modifier already exists in Vue internal like trim/number/lazy |
|
well it does work so i dont see anything bad with doing that. However, this is not the right place to discuss this! |
fix #8281
This PR adds dynamic modifiers for directives
It also adds error states for empty modifiers (
v-foo.="5") and handles empty dynamic modifiers or invalid values e.g.v-foo.[]="5",v-foo.[123]="5".I skipped modifiers with invalid values. I am not sure if that is a problem. Otherwise codegen would be corrupted (it was before for empty values already. That's also fixed with that).
I also had to change modifiers to be an Expression instead of SimpleExpression so that I can use parseExpression with it. That means I had to cast to SimpleExpression at a few places