chore(eslint): add warning when using async preventDefault() #7723
chore(eslint): add warning when using async preventDefault() #7723Shane-Donlon wants to merge 3 commits intoQwikDev:mainfrom
Conversation
|
commit: |
built with Refined Cloudflare Pages Action⚡ Cloudflare Pages Deployment
|
maiieul
left a comment
maiieul
left a comment
There was a problem hiding this comment.
Really good idea 🔥 Thanks a lot @Shane-Donlon!
| }, | ||
| messages: { | ||
| noAsyncPreventDefault: | ||
| 'This is an asynchronous function and does not support preventDefault. \nUse preventDefault attributes instead', |
There was a problem hiding this comment.
Can you add a link to the docs? Also I think this should suggest the use of sync$ too. WDYT?
There was a problem hiding this comment.
Thanks Maiieul, I think it's a great idea to include sync$ also
Is this a more agreeable message?
This is an asynchronous function and does not support preventDefault.
Use sync$ or prevent:default attributes instead.
Documentation: https://qwik.dev/docs/components/events/#preventdefault--stoppropagation
sync$: https://qwik.dev/docs/cookbook/sync-events/
Once agreed I'll commit and push up the message to avoid too many commits for editing the message.
Follow-up question:
Is a changeset needed for this?
There was a problem hiding this comment.
sounds good.
Do note that it's preventdefault:click etc, not what you wrote. Furthermore, those props only work if there is also a handler defined, in this case onClick$. Perhaps that should be classified as a bug though.
wmertens
left a comment
wmertens
left a comment
There was a problem hiding this comment.
Looking good, but you also need to do stoppropagation
| }, | ||
| messages: { | ||
| noAsyncPreventDefault: | ||
| 'This is an asynchronous function and does not support preventDefault. \nUse preventDefault attributes instead', |
There was a problem hiding this comment.
sounds good.
Do note that it's preventdefault:click etc, not what you wrote. Furthermore, those props only work if there is also a handler defined, in this case onClick$. Perhaps that should be classified as a bug though.
|
What's missing here? |
|
Remaining to-do:
I was away last week and forgot to acknowledge the review, but should be able to get around to it this week.
|
gioboa
left a comment
gioboa
left a comment
There was a problem hiding this comment.
Thanks @Shane-Donlon for your help.
is this PR still valid?
4 participants
What is it?
See also [🐞] onSubmit$ form handler not preventing default #7718
Description
Hey guys,
Feedback welcome - but thought I'd propose this as an idea.
While the knowledge exists in the documentation, the goal here is to prompt Developers that this is not a bug and to search the docs or to ask within Discord.
This does exclude
sync$((e)=>{e.preventDefault()}functions, and I did add this as a warning over error just in case there was something else I wasn't considering, and wouldn't result in breaking any builds.Checklist