fix: the useOn hook not working with Slot (#7250)

* fix: enhance slot rendering and add useOnDocument test * Create old-mayflies-fetch.md * fix wrong name * optimize the code * remove redundancy code * remove redundancy code * fix: solve all problems that mentioned * remove rebundant code * directly return value --------- Co-authored-by: wuls <[email protected]>
QwikDev · Jan 28, 2025 · 43846be · 43846be
1 parent f5326c3
commit 43846be
Show file tree

Hide file tree

Showing 4 changed files with 101 additions and 10 deletions.
diff --git a/.changeset/old-mayflies-fetch.md b/.changeset/old-mayflies-fetch.md
@@ -0,0 +1,5 @@
+---
+'@qwik.dev/core': patch
+---
+
+fix: the use hook didn't work when type is Slot.
diff --git a/packages/qwik/src/core/shared/component-execution.ts b/packages/qwik/src/core/shared/component-execution.ts
@@ -2,7 +2,7 @@ import { isDev } from '@qwik.dev/core/build';
 import { isQwikComponent, type OnRenderFn } from './component.public';
 import { assertDefined } from './error/assert';
 import { isQrl, type QRLInternal } from './qrl/qrl-class';
-import { JSXNodeImpl, isJSXNode, type Props } from './jsx/jsx-runtime';
+import { Fragment, JSXNodeImpl, _jsxSorted, isJSXNode, type Props } from './jsx/jsx-runtime';
 import type { JSXNodeInternal, JSXOutput } from './jsx/types/jsx-node';
 import type { KnownEventNames } from './jsx/types/jsx-qwik-events';
 import { invokeApply, newInvokeContext, untrack } from '../use/use-core';
@@ -23,6 +23,7 @@ import { logWarn } from './utils/log';
 import { EffectProperty, isSignal } from '../signal/signal';
 import { vnode_isVNode } from '../client/vnode';
 import { clearVNodeEffectDependencies } from '../signal/signal-subscriber';
+import { Slot } from '../shared/jsx/slot.public';
 
 /**
  * Use `executeComponent` to execute a component.
@@ -101,7 +102,7 @@ export const executeComponent = (
       (jsx) => {
         const useOnEvents = container.getHostProp<UseOnMap>(renderHost, USE_ON_LOCAL);
         if (useOnEvents) {
-          return maybeThen(addUseOnEvents(jsx, useOnEvents), () => jsx);
+          return addUseOnEvents(jsx, useOnEvents);
         }
         return jsx;
       },
@@ -133,8 +134,9 @@ export const executeComponent = (
 function addUseOnEvents(
   jsx: JSXOutput,
   useOnEvents: UseOnMap
-): ValueOrPromise<JSXNodeInternal<string> | null> {
+): ValueOrPromise<JSXNodeInternal<string> | null | JSXOutput> {
   const jsxElement = findFirstStringJSX(jsx);
+  let jsxResult = jsx;
   return maybeThen(jsxElement, (jsxElement) => {
     let isInvisibleComponent = false;
     if (!jsxElement) {
@@ -153,12 +155,14 @@ function addUseOnEvents(
       if (Object.prototype.hasOwnProperty.call(useOnEvents, key)) {
         if (isInvisibleComponent) {
           if (key === 'onQvisible$') {
-            jsxElement = addScriptNodeForInvisibleComponents(jsx);
+            const [jsxElement, jsx] = addScriptNodeForInvisibleComponents(jsxResult);
+            jsxResult = jsx;
             if (jsxElement) {
               addUseOnEvent(jsxElement, 'document:onQinit$', useOnEvents[key]);
             }
           } else if (key.startsWith('document:') || key.startsWith('window:')) {
-            jsxElement = addScriptNodeForInvisibleComponents(jsx);
+            const [jsxElement, jsx] = addScriptNodeForInvisibleComponents(jsxResult);
+            jsxResult = jsx;
             if (jsxElement) {
               addUseOnEvent(jsxElement, key, useOnEvents[key]);
             }
@@ -176,7 +180,7 @@ function addUseOnEvents(
         }
       }
     }
-    return jsxElement;
+    return jsxResult;
   });
 }
 
@@ -221,7 +225,9 @@ function findFirstStringJSX(jsx: JSXOutput): ValueOrPromise<JSXNodeInternal<stri
   return null;
 }
 
-function addScriptNodeForInvisibleComponents(jsx: JSXOutput): JSXNodeInternal<string> | null {
+function addScriptNodeForInvisibleComponents(
+  jsx: JSXOutput
+): [JSXNodeInternal<string> | null, JSXOutput | null] {
   if (isJSXNode(jsx)) {
     const jsxElement = new JSXNodeImpl(
       'script',
@@ -233,6 +239,9 @@ function addScriptNodeForInvisibleComponents(jsx: JSXOutput): JSXNodeInternal<st
       null,
       3
     );
+    if (jsx.type === Slot) {
+      return [jsxElement, _jsxSorted(Fragment, null, null, [jsx, jsxElement], 0, null)];
+    }
 
     if (jsx.children == null) {
       jsx.children = jsxElement;
@@ -241,11 +250,12 @@ function addScriptNodeForInvisibleComponents(jsx: JSXOutput): JSXNodeInternal<st
     } else {
       jsx.children = [jsx.children, jsxElement];
     }
-    return jsxElement;
+    return [jsxElement, jsx];
   } else if (Array.isArray(jsx) && jsx.length) {
     // get first element
-    return addScriptNodeForInvisibleComponents(jsx[0]);
+    const [jsxElement, _] = addScriptNodeForInvisibleComponents(jsx[0]);
+    return [jsxElement, jsx];
   }
 
-  return null;
+  return [null, null];
 }
diff --git a/packages/qwik/src/core/ssr/ssr-render-jsx.ts b/packages/qwik/src/core/ssr/ssr-render-jsx.ts
@@ -245,6 +245,7 @@ function processJSXNode(
             const node = ssr.getLastNode();
             const slotName = getSlotName(host, jsx, ssr);
             projectionAttrs.push(QSlot, slotName);
+
             enqueue(new ParentComponentData(options.styleScoped, options.parentComponentFrame));
             enqueue(ssr.closeProjection);
             const slotDefaultChildren: JSXChildren | null = jsx.children || null;
@@ -300,6 +301,7 @@ function processJSXNode(
             options.styleScoped,
             options.parentComponentFrame
           );
+
           const jsxOutput = applyQwikComponentBody(ssr, jsx, type);
           const compStyleComponentId = addComponentStylePrefix(host.getProp(QScopedStyle));
           enqueue(new ParentComponentData(options.styleScoped, options.parentComponentFrame));

diff --git a/packages/qwik/src/core/tests/use-on.spec.tsx b/packages/qwik/src/core/tests/use-on.spec.tsx
@@ -2,9 +2,11 @@ import {
   $,
   Fragment as Awaited,
   Fragment as Component,
+  Fragment as Projection,
   component$,
   Fragment,
   Fragment as Signal,
+  Slot,
   useOn,
   useOnDocument,
   useOnWindow,
@@ -647,4 +649,76 @@ describe.each([
     await trigger(document.body, 'div', 'click');
     await expect(document.querySelector('div')).toMatchDOM(<div>1</div>);
   });
+
+  it('#7230 - when multiple useOn are used in a component that is not rendered, it should add multiple script nodes', async () => {
+    const BreakpointProvider = component$(() => {
+      useOnDocument(
+        'click',
+        $(() => {})
+      );
+
+      useOnWindow(
+        'resize',
+        $(() => {})
+      );
+
+      useVisibleTask$(() => {});
+
+      return <Slot />;
+    });
+
+    const LayoutTest = component$(() => {
+      return (
+        <BreakpointProvider>
+          <div>test</div>
+        </BreakpointProvider>
+      );
+    });
+    const { vNode } = await render(<LayoutTest />, { debug });
+    expect(vNode).toMatchVDOM(
+      <Component ssr-required>
+        <Component ssr-required>
+          <Component ssr-required>
+            <Component ssr-required>
+              <div>test</div>
+            </Component>
+            <script type="placeholder" hidden></script>
+            <script type="placeholder" hidden></script>
+            <script type="placeholder" hidden></script>
+          </Component>
+        </Component>
+      </Component>
+    );
+  });
+  it('#7230 - when useOnDocument is used in a component that is not rendered, it should add a script node', async () => {
+    const BreakpointProvider = component$(() => {
+      useOnDocument(
+        'click',
+        $(() => {})
+      );
+
+      return <Slot />;
+    });
+
+    const Layout = component$(() => {
+      return (
+        <BreakpointProvider>
+          <div>test</div>
+        </BreakpointProvider>
+      );
+    });
+    const { vNode } = await render(<Layout />, { debug });
+    expect(vNode).toMatchVDOM(
+      <Component ssr-required>
+        <Component ssr-required>
+          <Component ssr-required>
+            <Projection ssr-required>
+              <div>test</div>
+            </Projection>
+            <script type="placeholder" hidden></script>
+          </Component>
+        </Component>
+      </Component>
+    );
+  });
 });