core icon indicating copy to clipboard operation
core copied to clipboard

fix(compat): click modifiers

Open alexanderturinske opened this issue 1 year ago • 2 comments

 - when the click "native" modifier is used in conjunction
   with the click "capture" modifier, the click is not
   registered in tests
 - see https://gitlab.com/groups/gitlab-org/-/epics/14951
   for more details

More information

https://gitlab.com/groups/gitlab-org/-/epics/14951

Workaround

-          @click.native.capture.stop="onItemClick(role)"
+          @click.capture.native.stop="onItemClick(role)"

Screenshot

image

alexanderturinske avatar Aug 23 '24 18:08 alexanderturinske

May be related to https://github.com/vuejs/core/issues/4566#issuecomment-917997056

This PR does not fix the solution, but starts TDD with a test

alexanderturinske avatar Aug 23 '24 18:08 alexanderturinske

@alexanderturinske I spent a bit more time looking into this and believe I have a fix.

Roughly speaking, the problem is that the compiler and runtime have different expectations about how "native" events are annotated. The compiler constructs a string of modifiers that matches the source order. So, @click.native.capture gives onClickNativeCapture, while @click.capture.native gives onClickCaptureNative.

The runtime, however, expects the Native substring to only be at the end of the event listener string (removing it if present).

So, there are two possible fixes:

  1. Update the compiler to ensure the native modifier is always last, OR
  2. Update the runtime to look anywhere in the event listener string for Native.

I'm more inclined to update the compiler, since the latter means doing a bit more work at runtime.

Here's a patch that fixes the compiler and the failing test:

diff --git a/packages/compiler-dom/src/transforms/vOn.ts b/packages/compiler-dom/src/transforms/vOn.ts
index e2bf1573b..2c7feb657 100644
--- a/packages/compiler-dom/src/transforms/vOn.ts
+++ b/packages/compiler-dom/src/transforms/vOn.ts
@@ -42,6 +42,7 @@ const resolveModifiers = (
   const keyModifiers = []
   const nonKeyModifiers = []
   const eventOptionModifiers = []
+  let hasNativeModifier = false
 
   for (let i = 0; i < modifiers.length; i++) {
     const modifier = modifiers[i]
@@ -55,7 +56,7 @@ const resolveModifiers = (
         loc,
       )
     ) {
-      eventOptionModifiers.push(modifier)
+      hasNativeModifier = true
     } else if (isEventOptionModifier(modifier)) {
       // eventOptionModifiers: modifiers for addEventListener() options,
       // e.g. .passive & .capture
@@ -83,6 +84,11 @@ const resolveModifiers = (
     }
   }
 
+  if (__COMPAT__ && hasNativeModifier) {
+    // Ensure native modifer is last
+    eventOptionModifiers.push('native')
+  }
+
   console.log('resolveModifiers', {
     keyModifiers,
     nonKeyModifiers,

For completeness, here's a patch that fixes the runtime, but I don't think it's the right approach.

Patch for runtime
diff --git a/packages/runtime-core/src/componentProps.ts b/packages/runtime-core/src/componentProps.ts
index ce39f150e..79434471b 100644
--- a/packages/runtime-core/src/componentProps.ts
+++ b/packages/runtime-core/src/componentProps.ts
@@ -409,8 +409,8 @@ function setFullProps(
         // into a separate `attrs` object for spreading. Make sure to preserve
         // original key casing
         if (__COMPAT__) {
-          if (isOn(key) && key.endsWith('Native')) {
-            key = key.slice(0, -6) // remove Native postfix
+          if (isOn(key) && key.includes('Native')) {
+            key = key.replace('Native', '')
           } else if (shouldSkipAttr(key, instance)) {
             continue
           }

markrian avatar Aug 28 '24 12:08 markrian