fix: the use hook didn't work when type is Slot. #7250

JerryWu1234 · 2025-01-13T13:39:14Z

What is it?

fix: modify jsx output when it is Slot.
example :

</Slot>

<>
  <script>
  </Slot>
<>

Description

Checklist

My code follows the developer guidelines of this project
I performed a self-review of my own code
I added a changeset with pnpm change
I made corresponding changes to the Qwik docs
I added new tests to cover the fix / functionality

changeset-bot · 2025-01-13T13:39:19Z

🦋 Changeset detected

Latest commit: defcde7

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages

Name	Type
@qwik.dev/core	Patch
eslint-plugin-qwik	Patch
@qwik.dev/react	Patch
@qwik.dev/router	Patch
create-qwik	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

pkg-pr-new · 2025-01-15T09:59:16Z

Open in Stackblitz

npm i https://pkg.pr.new/QwikDev/qwik/@qwik.dev/core@7250

npm i https://pkg.pr.new/QwikDev/qwik/@qwik.dev/router@7250

npm i https://pkg.pr.new/QwikDev/qwik/eslint-plugin-qwik@7250

npm i https://pkg.pr.new/QwikDev/qwik/create-qwik@7250

commit: 1d7a60d

github-actions · 2025-01-15T09:59:40Z

built with Refined Cloudflare Pages Action

⚡ Cloudflare Pages Deployment

Name	Status	Preview	Last Commit
qwik-docs	✅ Ready (View Log)	Visit Preview	`1d7a60d`

Varixo

Hey, thank you for your contribution, but this is not a good solution. We can't add special condition for every edge case in the vnode-diff.

Tip: the problem is somewhere in this function

qwik/packages/qwik/src/core/shared/component-execution.ts

Line 133 in ad4444c

function addUseOnEvents(

JerryWu1234 · 2025-01-16T14:23:06Z

Hey, thank you for your contribution, but this is not a good solution. We can't add special conditionl for every edge case in the vnode-diff.

Tip: the problem is somewhere in this function

qwik/packages/qwik/src/core/shared/component-execution.ts

Line 133 in ad4444c

function addUseOnEvents(

There are a couple of problems. Q: template also has a problem with usedocument. However I will find a better way to do it. And will tell you when I finish it

Varixo · 2025-01-16T15:03:25Z

Hey, thank you for your contribution, but this is not a good solution. We can't add special conditionl for every edge case in the vnode-diff.
Tip: the problem is somewhere in this function

qwik/packages/qwik/src/core/shared/component-execution.ts

Line 133 in ad4444c

function addUseOnEvents(

There are a couple of problems. Q: template also has a problem with usedocument. However I will find a better way to do it. And will tell you when I finish it

I think we need to just modify jsx output to be:

<>
  <script ... />
  <Slot />
</>

instead of just <Slot />

Varixo · 2025-01-16T15:04:39Z

if you need more help with that let me know

Varixo

Good job! I have added some comments

packages/qwik/src/core/tests/use-on.spec.tsx

packages/qwik/src/core/shared/component-execution.ts

Varixo

Awesome work! Nice job! 😃
I added some comments

Varixo · 2025-01-21T18:03:22Z

packages/qwik/src/core/shared/component-execution.ts

@@ -176,7 +182,7 @@ function addUseOnEvents(
        }
      }
    }
-    return jsxElement;
+    return _jsx || jsx;


Just _jsx isn't enough?

Varixo · 2025-01-21T18:04:36Z

packages/qwik/src/core/shared/component-execution.ts

  const jsxElement = findFirstStringJSX(jsx);
+  let _jsx = jsx;


I think we don't use _ convention. Maybe we can name it jsxResult?

Varixo · 2025-01-21T18:07:17Z

packages/qwik/src/core/shared/component-execution.ts

+    const [jsxElement, _jsx] = addScriptNodeForInvisibleComponents(jsx[0]);
+    return [jsxElement, jsx];


just return addScriptNodeForInvisibleComponents(jsx[0]) doesn't work?

Varixo · 2025-01-21T18:07:42Z

packages/qwik/src/core/ssr/ssr-render-jsx.ts

@@ -300,7 +301,8 @@ function processJSXNode(
            options.styleScoped,
            options.parentComponentFrame
          );
-          const jsxOutput = applyQwikComponentBody(ssr, jsx, type);
+
+          const jsxOutput: any = applyQwikComponentBody(ssr, jsx, type);


Can you remove any type here?

Varixo · 2025-01-21T18:07:55Z

packages/qwik/src/core/tests/use-on.spec.tsx

@@ -21,7 +23,7 @@ Error.stackTraceLimit = 100;

 describe.each([
  { render: ssrRenderToDom }, //
-  { render: domRender }, //
+  { render: domRender }, ///


Remove additional /

fix: enhance slot rendering and add useOnDocument test

892d99a

JerryWu1234 changed the base branch from main to build/v2 January 13, 2025 13:39

Merge branch 'build/v2' into v2v2v2

593293e

JerryWu1234 changed the title ~~V2v2v2~~ fix: a draft for discovering a V2 bug Jan 15, 2025

wuls added 4 commits January 15, 2025 16:31

fix

a0fde9a

fix

8b41e6e

fix

051d1bc

fix

9678045

Varixo requested changes Jan 15, 2025

View reviewed changes

JerryWu1234 and others added 4 commits January 16, 2025 10:54

Merge branch 'build/v2' into v2v2v2

be87191

fix

d80338e

Merge branch 'v2v2v2' of https://github.com/JerryWu1234/qwik into v2v2v2

01f1bfd

fix

80aaf37

wuls and others added 6 commits January 18, 2025 18:14

fix

79d3fb7

Merge branch 'build/v2' into v2v2v2

b62adb7

fix

fc83532

fix

2e7d32e

fix

08ac62c

Create old-mayflies-fetch.md

294ff31

JerryWu1234 changed the title ~~fix: a draft for discovering a V2 bug~~ fix: the use hook didn't work when type is Slot. Jan 20, 2025

JerryWu1234 marked this pull request as ready for review January 20, 2025 05:28

JerryWu1234 requested a review from a team as a code owner January 20, 2025 05:28

JerryWu1234 requested a review from Varixo January 20, 2025 05:29

Varixo requested changes Jan 20, 2025

View reviewed changes

packages/qwik/src/core/tests/use-on.spec.tsx Outdated Show resolved Hide resolved

packages/qwik/src/core/tests/use-on.spec.tsx Outdated Show resolved Hide resolved

packages/qwik/src/core/shared/component-execution.ts Outdated Show resolved Hide resolved

wuls and others added 5 commits January 21, 2025 13:31

fix wrong name

aad81e3

optimize the code

4e41e48

remove redundancy code

4b1f8fb

remove redundancy code

1d7a60d

Merge branch 'build/v2' into v2v2v2

defcde7

Varixo requested changes Jan 21, 2025

View reviewed changes

Varixo assigned JerryWu1234 Jan 21, 2025

Varixo added the VERSION: upcoming major label Jan 21, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: the use hook didn't work when type is Slot. #7250

fix: the use hook didn't work when type is Slot. #7250

JerryWu1234 commented Jan 13, 2025 •

edited

Loading

changeset-bot bot commented Jan 13, 2025 •

edited

Loading

pkg-pr-new bot commented Jan 15, 2025 •

edited

Loading

github-actions bot commented Jan 15, 2025 •

edited

Loading

Varixo left a comment

JerryWu1234 commented Jan 16, 2025 •

edited

Loading

Varixo commented Jan 16, 2025

Varixo commented Jan 16, 2025

Varixo left a comment

Varixo left a comment

Varixo Jan 21, 2025

Varixo Jan 21, 2025

Varixo Jan 21, 2025

Varixo Jan 21, 2025

Varixo Jan 21, 2025

		const [jsxElement, _jsx] = addScriptNodeForInvisibleComponents(jsx[0]);
		return [jsxElement, jsx];

fix: the use hook didn't work when type is Slot. #7250

Are you sure you want to change the base?

fix: the use hook didn't work when type is Slot. #7250

Conversation

JerryWu1234 commented Jan 13, 2025 • edited Loading

What is it?

Description

Checklist

changeset-bot bot commented Jan 13, 2025 • edited Loading

🦋 Changeset detected

pkg-pr-new bot commented Jan 15, 2025 • edited Loading

github-actions bot commented Jan 15, 2025 • edited Loading

built with Refined Cloudflare Pages Action

⚡ Cloudflare Pages Deployment

Varixo left a comment

Choose a reason for hiding this comment

JerryWu1234 commented Jan 16, 2025 • edited Loading

Varixo commented Jan 16, 2025

Varixo commented Jan 16, 2025

Varixo left a comment

Choose a reason for hiding this comment

Varixo left a comment

Choose a reason for hiding this comment

Varixo Jan 21, 2025

Choose a reason for hiding this comment

Varixo Jan 21, 2025

Choose a reason for hiding this comment

Varixo Jan 21, 2025

Choose a reason for hiding this comment

Varixo Jan 21, 2025

Choose a reason for hiding this comment

Varixo Jan 21, 2025

Choose a reason for hiding this comment

JerryWu1234 commented Jan 13, 2025 •

edited

Loading

changeset-bot bot commented Jan 13, 2025 •

edited

Loading

pkg-pr-new bot commented Jan 15, 2025 •

edited

Loading

github-actions bot commented Jan 15, 2025 •

edited

Loading

JerryWu1234 commented Jan 16, 2025 •

edited

Loading