diff --git a/packages/qwik/src/core/render/jsx/jsx-runtime.ts b/packages/qwik/src/core/render/jsx/jsx-runtime.ts index 0dfdca7cf78..d563658155b 100644 --- a/packages/qwik/src/core/render/jsx/jsx-runtime.ts +++ b/packages/qwik/src/core/render/jsx/jsx-runtime.ts @@ -271,6 +271,26 @@ export const isJSXNode = (n: unknown): n is JSXNode => { } }; +/** + * Instead of using PropsProxyHandler getter (which could create a component-level subscription). + * Use this function to get the props directly from a const or var props. + */ +export const directGetPropsProxyProp = ( + varProps: Props, + constProps: Props | null, + prop: string +): T => { + return (constProps && prop in constProps ? constProps[prop] : varProps[prop]) as T; +}; + +export const getMergedProps = (props: Props): Props => { + return { + ...(props as any)[_VAR_PROPS], + ...(props as any)[_CONST_PROPS], + children: props.children, + }; +}; + /** @public */ export const Fragment: FunctionComponent<{ children?: any; key?: string | number | null }> = ( props @@ -357,10 +377,7 @@ class PropsProxyHandler implements ProxyHandler { if (this.$children$ != null && prop === 'children') { return this.$children$; } - const value = - this.$constProps$ && prop in this.$constProps$ - ? this.$constProps$[prop as string] - : this.$varProps$[prop as string]; + const value = directGetPropsProxyProp(this.$varProps$, this.$constProps$, prop as string); // a proxied value that the optimizer made return value instanceof WrappedSignal ? value.value : value; } diff --git a/packages/qwik/src/core/v2/client/vnode-diff.ts b/packages/qwik/src/core/v2/client/vnode-diff.ts index 3f70a4f1e97..13db7841e4b 100644 --- a/packages/qwik/src/core/v2/client/vnode-diff.ts +++ b/packages/qwik/src/core/v2/client/vnode-diff.ts @@ -5,7 +5,14 @@ import { assertDefined, assertFalse, assertTrue } from '../../error/assert'; import type { QRLInternal } from '../../qrl/qrl-class'; import type { QRL } from '../../qrl/qrl.public'; import { dangerouslySetInnerHTML, serializeAttribute } from '../../render/execute-component'; -import { Fragment, JSXNodeImpl, isJSXNode, type Props } from '../../render/jsx/jsx-runtime'; +import { + Fragment, + JSXNodeImpl, + directGetPropsProxyProp, + getMergedProps, + isJSXNode, + type Props, +} from '../../render/jsx/jsx-runtime'; import { Slot } from '../../render/jsx/slot.public'; import type { JSXNode, JSXOutput } from '../../render/jsx/types/jsx-node'; import type { JSXChildren } from '../../render/jsx/types/jsx-qwik-attributes'; @@ -425,7 +432,10 @@ export const vnode_diff = ( /// STEP 1: Bucketize the children based on the projection name. for (let i = 0; i < children.length; i++) { const child = children[i]; - const slotName = String((isJSXNode(child) && child.props[QSlot]) || QDefaultSlot); + const slotName = String( + (isJSXNode(child) && directGetPropsProxyProp(child.varProps, child.constProps, QSlot)) || + QDefaultSlot + ); const idx = mapApp_findIndx(projections, slotName, 0); let jsxBucket: JSXNodeImpl; if (idx >= 0) { @@ -529,7 +539,7 @@ export const vnode_diff = ( ); } } - return jsxValue.props.name || QDefaultSlot; + return directGetPropsProxyProp(jsxValue.varProps, jsxValue.constProps, 'name') || QDefaultSlot; } function drainAsyncQueue(): ValueOrPromise { @@ -987,8 +997,9 @@ export const vnode_diff = ( function expectComponent(component: Function) { const componentMeta = (component as any)[SERIALIZABLE_STATE] as [QRLInternal>]; let host = (vNewNode || vCurrent) as VirtualVNode | null; + const currentJSX: JSXNode = jsxValue; if (componentMeta) { - const jsxProps = jsxValue.props; + const jsxProps = currentJSX.props; // QComponent let shouldRender = false; const [componentQRL] = componentMeta; @@ -996,7 +1007,7 @@ export const vnode_diff = ( const componentHash = componentQRL.$hash$; const vNodeComponentHash = getComponentHash(host, container.$getObjectById$); - const lookupKey = jsxValue.key || componentHash; + const lookupKey = currentJSX.key || componentHash; const vNodeLookupKey = getKey(host) || vNodeComponentHash; const lookupKeysAreEqual = lookupKey === vNodeLookupKey; @@ -1033,7 +1044,7 @@ export const vnode_diff = ( container.$scheduler$(ChoreType.COMPONENT, host, componentQRL, jsxProps); } } - jsxValue.children != null && descendContentToProject(jsxValue.children, host); + currentJSX.children != null && descendContentToProject(currentJSX.children, host); } else { // Inline Component vnode_insertBefore( @@ -1043,7 +1054,7 @@ export const vnode_diff = ( vCurrent && getInsertBefore() ); isDev && vnode_setProp(vNewNode, DEBUG_TYPE, VirtualType.InlineComponent); - vnode_setProp(vNewNode, ELEMENT_PROPS, jsxValue.props); + vnode_setProp(vNewNode, ELEMENT_PROPS, currentJSX.props); host = vNewNode; let component$Host: VNode | null = host; @@ -1061,7 +1072,7 @@ export const vnode_diff = ( host, (component$Host || container.rootVNode) as HostElement, component as OnRenderFn, - jsxValue.props + getMergedProps(currentJSX.props) ); asyncQueue.push(jsxOutput, host); } diff --git a/packages/qwik/src/core/v2/shared/component-execution.ts b/packages/qwik/src/core/v2/shared/component-execution.ts index e81b9dbcfad..b212b764b26 100644 --- a/packages/qwik/src/core/v2/shared/component-execution.ts +++ b/packages/qwik/src/core/v2/shared/component-execution.ts @@ -2,7 +2,7 @@ import { isDev } from '@builder.io/qwik/build'; import { isQwikComponent, type OnRenderFn } from '../../component/component.public'; import { assertDefined } from '../../error/assert'; import { isQrl, type QRLInternal } from '../../qrl/qrl-class'; -import { JSXNodeImpl, isJSXNode } from '../../render/jsx/jsx-runtime'; +import { JSXNodeImpl, isJSXNode, type Props } from '../../render/jsx/jsx-runtime'; import type { JSXNode, JSXOutput } from '../../render/jsx/types/jsx-node'; import type { KnownEventNames } from '../../render/jsx/types/jsx-qwik-events'; import { invokeApply, newInvokeContext, untrack } from '../../use/use-core'; @@ -23,6 +23,7 @@ import { logWarn } from '../../util/log'; import { EffectProperty, isSignal } from '../signal/v2-signal'; import { vnode_isVNode } from '../client/vnode'; import { clearVNodeEffectDependencies } from '../signal/v2-subscriber'; +import { _CONST_PROPS, _VAR_PROPS } from '../../internal'; /** * Use `executeComponent2` to execute a component. @@ -50,7 +51,7 @@ export const executeComponent2 = ( renderHost: HostElement, subscriptionHost: HostElement, componentQRL: OnRenderFn | QRLInternal> | null, - props: Record | null + props: Props | null ): ValueOrPromise => { const iCtx = newInvokeContext( container.$locale$, @@ -74,7 +75,7 @@ export const executeComponent2 = ( componentFn = componentQRL.getFn(iCtx); } else if (isQwikComponent(componentQRL)) { const qComponentFn = componentQRL as ( - props: Record, + props: Props, key: string | null, flags: number ) => JSXNode; diff --git a/packages/qwik/src/core/v2/ssr/ssr-render-component.ts b/packages/qwik/src/core/v2/ssr/ssr-render-component.ts index 26dff89d1b4..aeb4aea692e 100644 --- a/packages/qwik/src/core/v2/ssr/ssr-render-component.ts +++ b/packages/qwik/src/core/v2/ssr/ssr-render-component.ts @@ -8,6 +8,7 @@ import { executeComponent2 } from '../shared/component-execution'; import { ChoreType } from '../shared/scheduler'; import type { ValueOrPromise } from '../../util/types'; import type { JSXOutput } from '../../render/jsx/types/jsx-node'; +import { getMergedProps } from '../../render/jsx/jsx-runtime'; export const applyInlineComponent = ( ssr: SSRContainer, @@ -16,7 +17,7 @@ export const applyInlineComponent = ( jsx: JSXNode ) => { const host = ssr.getLastNode(); - return executeComponent2(ssr, host, component$Host, component, jsx.props); + return executeComponent2(ssr, host, component$Host, component, getMergedProps(jsx.props)); }; export const applyQwikComponentBody = ( diff --git a/packages/qwik/src/core/v2/ssr/ssr-render-jsx.ts b/packages/qwik/src/core/v2/ssr/ssr-render-jsx.ts index 987ae5e7370..5798601fcc5 100644 --- a/packages/qwik/src/core/v2/ssr/ssr-render-jsx.ts +++ b/packages/qwik/src/core/v2/ssr/ssr-render-jsx.ts @@ -3,7 +3,7 @@ import { isQwikComponent } from '../../component/component.public'; import { isQrl } from '../../qrl/qrl-class'; import type { QRL } from '../../qrl/qrl.public'; import { serializeAttribute } from '../../render/execute-component'; -import { Fragment } from '../../render/jsx/jsx-runtime'; +import { Fragment, directGetPropsProxyProp } from '../../render/jsx/jsx-runtime'; import { Slot } from '../../render/jsx/slot.public'; import type { JSXNode, JSXOutput } from '../../render/jsx/types/jsx-node'; import type { JSXChildren } from '../../render/jsx/types/jsx-qwik-attributes'; @@ -26,7 +26,7 @@ import { isJsxPropertyAnEventName, isPreventDefault, } from '../shared/event-names'; -import { addComponentStylePrefix, hasClassAttr, isClassAttr } from '../shared/scoped-styles'; +import { addComponentStylePrefix, isClassAttr } from '../shared/scoped-styles'; import { qrlToString, type SerializationContext } from '../shared/shared-serialization'; import { DEBUG_TYPE, VirtualType, type fixMeAny } from '../shared/types'; import { WrappedSignal, EffectProperty, isSignal } from '../signal/v2-signal'; @@ -157,8 +157,7 @@ function processJSXNode( // Below, JSXChildren allows functions and regexes, but we assume the dev only uses those as appropriate. if (typeof type === 'string') { // append class attribute if styleScopedId exists and there is no class attribute - const classAttributeExists = - hasClassAttr(jsx.varProps) || (jsx.constProps && hasClassAttr(jsx.constProps)); + const classAttributeExists = directGetPropsProxyProp(jsx.varProps, jsx.constProps, 'class'); if (!classAttributeExists && styleScoped) { if (!jsx.constProps) { jsx.constProps = {}; @@ -233,7 +232,7 @@ function processJSXNode( ssr.closeFragment(); } } else if (type === SSRComment) { - ssr.commentNode((jsx.props.data as string) || ''); + ssr.commentNode(directGetPropsProxyProp(jsx.varProps, jsx.constProps, 'data') || ''); } else if (type === SSRStream) { ssr.commentNode(FLUSH_COMMENT); const generator = jsx.children as SSRStreamChildren; @@ -252,7 +251,7 @@ function processJSXNode( enqueue(value as StackValue); isPromise(value) && enqueue(Promise); } else if (type === SSRRaw) { - ssr.htmlNode(jsx.props.data as string); + ssr.htmlNode(directGetPropsProxyProp(jsx.varProps, jsx.constProps, 'data')); } else if (isQwikComponent(type)) { // prod: use new instance of an array for props, we always modify props for a component ssr.openComponent(isDev ? [DEBUG_TYPE, VirtualType.Component] : []); @@ -491,14 +490,17 @@ function getSlotName(host: ISsrNode, jsx: JSXNode, ssr: SSRContainer): string { return trackSignal(() => constValue.value, host as fixMeAny, EffectProperty.COMPONENT, ssr); } } - return (jsx.props.name as string) || QDefaultSlot; + return directGetPropsProxyProp(jsx.varProps, jsx.constProps, 'name') || QDefaultSlot; } function appendQwikInspectorAttribute(jsx: JSXNode) { if (isDev && qInspector && jsx.dev && jsx.type !== 'head') { const sanitizedFileName = jsx.dev.fileName?.replace(/\\/g, '/'); const qwikInspectorAttr = 'data-qwik-inspector'; - if (sanitizedFileName && !(qwikInspectorAttr in jsx.props)) { + if ( + sanitizedFileName && + !directGetPropsProxyProp(jsx.varProps, jsx.constProps, qwikInspectorAttr) + ) { if (!jsx.constProps) { jsx.constProps = {}; } diff --git a/packages/qwik/src/core/v2/tests/inline-component.spec.tsx b/packages/qwik/src/core/v2/tests/inline-component.spec.tsx index 78bf1c2df76..9086be88945 100644 --- a/packages/qwik/src/core/v2/tests/inline-component.spec.tsx +++ b/packages/qwik/src/core/v2/tests/inline-component.spec.tsx @@ -4,6 +4,7 @@ import { Fragment as InlineComponent, component$, useSignal, + useStore, } from '@builder.io/qwik'; import { domRender, ssrRenderToDom, trigger } from '@builder.io/qwik/testing'; import { describe, expect, it } from 'vitest'; @@ -27,6 +28,8 @@ const InlineWrapper = () => { return ; }; +const Id = (props: any) =>
Id: {props.id}
; + describe.each([ { render: ssrRenderToDom }, // { render: domRender }, // @@ -147,4 +150,80 @@ describe.each([ ); }); + + it('should not rerender component', async () => { + const Child = component$((props: { id: number }) => { + const renders = useStore( + { + count: 0, + }, + { reactive: false } + ); + renders.count++; + const rerenders = renders.count + 0; + return ( + <> + + {rerenders} + + ); + }); + const Cmp = component$(() => { + const id = useSignal(0); + return ( + <> + + + + ); + }); + + const { vNode, container } = await render(, { debug }); + + expect(vNode).toMatchVDOM( + + <> + + + + +
+ {'Id: '} + {'0'} +
+
+ 1 +
+
+ +
+ ); + + await trigger(container.document.body, 'button', 'click'); + + expect(vNode).toMatchVDOM( + + <> + + + + +
+ {'Id: '} + {'1'} +
+
+ 1 +
+
+ +
+ ); + }); }); diff --git a/starters/e2e/e2e.signals.e2e.ts b/starters/e2e/e2e.signals.e2e.ts index 2edaed25f12..e81b51aa57b 100644 --- a/starters/e2e/e2e.signals.e2e.ts +++ b/starters/e2e/e2e.signals.e2e.ts @@ -12,8 +12,7 @@ test.describe("signals", () => { }); function tests() { - // TODO(v2): fix this - test.skip("should do its thing", async ({ page }) => { + test("should do its thing", async ({ page }) => { const incrementBtn = page.locator("#count"); const clickBtn = page.locator("#click"); const incrementIdBtn = page.locator("#increment-id");