Explorar o código

fix(runtime-vapor): preserve fallthrough attrs on nested dynamic fragments (#14904)

edison hai 2 semanas
pai
achega
58e6d0b05c

+ 215 - 1
packages/runtime-vapor/__tests__/componentAttrs.spec.ts

@@ -21,7 +21,7 @@ import {
   setStyle,
   template,
 } from '../src'
-import { makeRender } from './_utils'
+import { compile, makeRender } from './_utils'
 import { VaporDynamicComponentFlags, stringifyStyle } from '@vue/shared'
 import { setElementText } from '../src/dom/prop'
 
@@ -125,6 +125,48 @@ describe('attribute fallthrough', () => {
     expect(node.style.fontWeight).toBe('bold')
   })
 
+  it('should only allow whitelisted fallthrough on functional component dynamic root branch', async () => {
+    const show = ref(false)
+    const parentClass = ref('c0')
+
+    const t0 = template('<div class="c2">off</div>', 1)
+    const t1 = template('<div class="c2">on</div>', 1)
+    const { component: Child } = define(() => {
+      return createIf(
+        () => show.value,
+        () => t1(),
+        () => t0(),
+      )
+    })
+
+    const { host } = define(() =>
+      createComponent(Child, {
+        foo: () => 'bar',
+        id: () => 'test',
+        class: () => parentClass.value,
+      }),
+    ).render()
+
+    const assertRoot = (text: string) => {
+      const node = host.children[0] as HTMLElement
+      expect(node.textContent).toBe(text)
+      expect(node.getAttribute('id')).toBe(null)
+      expect(node.getAttribute('foo')).toBe(null)
+      expect(node.classList.contains('c2')).toBe(true)
+      expect(node.classList.contains(parentClass.value)).toBe(true)
+    }
+
+    assertRoot('off')
+
+    show.value = true
+    await nextTick()
+    assertRoot('on')
+
+    parentClass.value = 'c1'
+    await nextTick()
+    assertRoot('on')
+  })
+
   it('should allow all attrs on functional component with declared props', async () => {
     const click = vi.fn()
     const childUpdated = vi.fn()
@@ -369,6 +411,40 @@ describe('attribute fallthrough', () => {
     expect(`Extraneous non-props attributes (class)`).toHaveBeenWarned()
   })
 
+  it('should warn when fallthrough fails on dynamic teleport root branch', async () => {
+    const show = ref(false)
+    const target = document.createElement('div')
+    document.body.appendChild(target)
+
+    const fallback = template('<div>fallback</div>', 1)
+    const teleported = template('<div>teleported</div>')
+    const { component: Child } = define(() =>
+      createIf(
+        () => show.value,
+        () =>
+          createComponent(
+            VaporTeleport,
+            { to: () => target },
+            {
+              default: () => teleported(),
+            },
+          ),
+        () => fallback(),
+      ),
+    )
+
+    const { host } = define(() =>
+      createComponent(Child, { class: () => 'parent' }),
+    ).render()
+
+    expect((host.children[0] as HTMLElement).className).toBe('parent')
+
+    show.value = true
+    await nextTick()
+
+    expect(`Extraneous non-props attributes (class)`).toHaveBeenWarned()
+  })
+
   it('should dedupe same listeners when $attrs is used during render', () => {
     const click = vi.fn()
     const count = ref(0)
@@ -835,6 +911,144 @@ describe('attribute fallthrough', () => {
     expect(host.innerHTML).toBe('<div id="a">foo</div><!--if-->')
   })
 
+  it('should fallthrough attrs on v-else-if component root branch', async () => {
+    const data = ref({
+      loading: false,
+      id: 'foo',
+    })
+    const Child = compile(
+      `<script setup vapor>
+        defineProps({
+          loading: {
+            type: Boolean,
+            default: false
+          }
+        })
+      </script>
+      <template>
+        <div v-if="loading" class="simple-button">
+          loading === true
+        </div>
+        <div v-else-if="loading === false" class="simple-button">
+          loading === false
+        </div>
+      </template>`,
+      data,
+    )
+    const Parent = compile(
+      `<script setup vapor>
+        const data = _data
+        const Child = _components.Child
+      </script>
+      <template>
+        <Child :loading="data.loading" :id="data.id" class="custom-btn" />
+      </template>`,
+      data,
+      { Child },
+    )
+
+    const { host } = define(Parent).render()
+
+    const root = () => host.querySelector('.simple-button') as HTMLElement
+    const assertRoot = (text: string) => {
+      const el = root()
+      expect(el.classList.contains('simple-button')).toBe(true)
+      expect(el.classList.contains('custom-btn')).toBe(true)
+      expect(el.id).toBe(data.value.id)
+      expect(el.textContent!.trim()).toBe(text)
+    }
+
+    assertRoot('loading === false')
+
+    data.value.id = 'bar'
+    await nextTick()
+    assertRoot('loading === false')
+
+    data.value.loading = true
+    await nextTick()
+    assertRoot('loading === true')
+
+    data.value.loading = false
+    await nextTick()
+    assertRoot('loading === false')
+  })
+
+  it('should not fallthrough attrs into nested slot branch', async () => {
+    const show = ref(false)
+    const Child = compile(
+      `<script setup vapor>
+        defineProps({
+          show: Boolean
+        })
+      </script>
+      <template>
+        <div v-if="!show">fallback</div>
+        <slot v-else-if="show" />
+      </template>`,
+      show,
+    )
+    const Parent = compile(
+      `<script setup vapor>
+        const show = _data
+        const Child = _components.Child
+      </script>
+      <template>
+        <Child :show="show" class="custom-btn">
+          <span>slot</span>
+        </Child>
+      </template>`,
+      show,
+      { Child },
+    )
+
+    const { host } = define(Parent).render()
+
+    expect((host.querySelector('div') as HTMLElement).className).toBe(
+      'custom-btn',
+    )
+
+    show.value = true
+    await nextTick()
+    const span = host.querySelector('span') as HTMLElement
+    expect(span.className).toBe('')
+    expect(`Extraneous non-props attributes (class)`).toHaveBeenWarned()
+  })
+
+  it('should not fallthrough attrs into initially active nested slot branch', async () => {
+    const show = ref(true)
+    const Child = compile(
+      `<script setup vapor>
+        defineProps({
+          show: Boolean
+        })
+      </script>
+      <template>
+        <div v-if="!show">fallback</div>
+        <slot v-else-if="show" />
+      </template>`,
+      show,
+    )
+    const Parent = compile(
+      `<script setup vapor>
+        const show = _data
+        const Child = _components.Child
+      </script>
+      <template>
+        <Child :show="show" class="custom-btn">
+          <span>slot</span>
+        </Child>
+      </template>`,
+      show,
+      { Child },
+    )
+
+    const { host } = define(Parent).render()
+    const span = host.querySelector('span') as HTMLElement
+
+    expect(span.className).toBe('')
+    expect(`Extraneous non-props attributes (class)`).toHaveBeenWarned()
+  })
+
   it('should not allow attrs to fallthrough on component with multiple roots', async () => {
     const t0 = template('<span>')
     const t1 = template('<div>')

+ 72 - 42
packages/runtime-vapor/src/component.ts

@@ -1275,32 +1275,14 @@ function handleSetupResult(
     component.inheritAttrs !== false &&
     Object.keys(instance.attrs).length
   ) {
-    const root = getRootElement(
-      instance.block,
-      // attach attrs to root dynamic fragments for applying during each update
-      frag => registerDynamicFragmentFallthroughAttrs(frag, instance.attrs),
-      false,
-    )
-    if (root) {
-      renderEffect(() => {
-        const attrs =
-          isFunction(component) &&
-          !(isTransitionEnabled ? isVaporTransition(component) : false)
-            ? getFunctionalFallthrough(instance.attrs)
-            : instance.attrs
-        if (attrs) applyFallthroughProps(root, attrs)
-      })
-    } else if (
-      __DEV__ &&
-      ((!instance.accessedAttrs &&
-        isArray(instance.block) &&
-        instance.block.length) ||
-        // preventing attrs fallthrough on Teleport
-        // consistent with VDOM Teleport behavior
-        (isTeleportEnabled && isTeleportFragment(instance.block)))
-    ) {
-      warnExtraneousAttributes(instance.attrs)
-    }
+    const getFallthroughAttrs =
+      isFunction(component) &&
+      !(isTransitionEnabled ? isVaporTransition(component) : false)
+        ? () => getFunctionalFallthrough(instance.attrs)
+        : () => instance.attrs
+    // attach attrs to the root element, or to root dynamic fragments so they
+    // can be (re-)applied during each branch update
+    applyFallthroughAttrs(instance.block, instance, getFallthroughAttrs)
   }
 
   if (__DEV__) {
@@ -1313,24 +1295,72 @@ export function getCurrentScopeId(): string | undefined {
   return scopeOwner ? scopeOwner.type.__scopeId : undefined
 }
 
+// Attach fallthrough attrs to the single root element. When the root is a
+// dynamic fragment (e.g. v-if), the attrs are (re-)applied on each branch
+// update via its insert hook. Slots and teleports warn instead of receiving
+// the attrs, consistent with VDOM behavior.
+function applyFallthroughAttrs(
+  block: Block,
+  instance: VaporComponentInstance,
+  getFallthroughAttrs: () => Record<string, any> | undefined,
+  scope?: EffectScope,
+): void {
+  let hasSlotFragment = false
+  const root = getRootElement(
+    block,
+    frag => {
+      if (frag.isSlot) {
+        hasSlotFragment = true
+      } else {
+        // Nested dynamic fragments need their own fallthrough hook.
+        registerDynamicFragmentFallthroughAttrs(
+          frag,
+          instance,
+          getFallthroughAttrs,
+        )
+      }
+    },
+    false,
+  )
+
+  if (root && !hasSlotFragment) {
+    const applyEffect = () =>
+      renderEffect(() => {
+        const attrs = getFallthroughAttrs()
+        if (attrs) applyFallthroughProps(root, attrs)
+      })
+    // ensure the render effect is cleaned up when the branch scope is stopped
+    scope ? scope.run(applyEffect) : applyEffect()
+  } else if (
+    __DEV__ &&
+    (hasSlotFragment ||
+      (isTeleportEnabled && containsTeleportFragment(block)) ||
+      (!instance.accessedAttrs && isArray(block) && block.length))
+  ) {
+    warnExtraneousAttributes(instance.attrs)
+  }
+}
+
+function containsTeleportFragment(block: Block): boolean {
+  if (isTeleportFragment(block)) return true
+  if (isArray(block)) {
+    return block.some(
+      child => !(child instanceof Comment) && containsTeleportFragment(child),
+    )
+  }
+  return isFragment(block) && containsTeleportFragment(block.nodes)
+}
+
 function registerDynamicFragmentFallthroughAttrs(
   frag: DynamicFragment,
-  attrs: Record<string, any>,
+  instance: VaporComponentInstance,
+  getFallthroughAttrs: () => Record<string, any> | undefined,
 ): void {
+  // avoid registering duplicate hooks
+  if (frag.hasFallthroughAttrs) return
+
   frag.hasFallthroughAttrs = true
-  ;(frag.onBeforeInsert ||= []).push(nodes => {
-    if (nodes instanceof Element) {
-      // ensure render effect is cleaned up when branch scope is stopped
-      frag.scope!.run(() => {
-        renderEffect(() => applyFallthroughProps(nodes, attrs))
-      })
-    } else if (
-      __DEV__ &&
-      // preventing attrs fallthrough on slots
-      // consistent with VDOM slots behavior
-      (frag.isSlot || (isArray(nodes) && nodes.length))
-    ) {
-      warnExtraneousAttributes(attrs)
-    }
-  })
+  ;(frag.onBeforeInsert ||= []).push(nodes =>
+    applyFallthroughAttrs(nodes, instance, getFallthroughAttrs, frag.scope!),
+  )
 }