Merge pull request #7707 from QwikDev/v2-dont-wrap-functions

wmertens · web-flow · commit f63b4a127b81 · 2025-07-05T08:43:58.000+02:00
fix: don't wrap function calls in signal
diff --git a/.changeset/fuzzy-files-kiss.md b/.changeset/fuzzy-files-kiss.md
@@ -0,0 +1,5 @@
+---
+'@qwik.dev/core': patch
+---
+
+fix: don't wrap function calls in signal
diff --git a/packages/qwik/src/optimizer/core/src/inlined_fn.rs b/packages/qwik/src/optimizer/core/src/inlined_fn.rs
@@ -47,7 +47,11 @@ pub fn convert_inlined_fn(
 		return (None, is_const);
 	}
 
-	if !is_used_as_object(&expr, &scoped_idents) {
+	let (used_as_object, used_as_call) = is_used_as_object_or_call(&expr, &scoped_idents);
+	if used_as_call {
+		return (None, false);
+	}
+	if !used_as_object {
 		return (None, is_const);
 	}
 
@@ -211,6 +215,7 @@ pub fn render_expr(expr: &ast::Expr) -> String {
 struct ObjectUsageChecker<'a> {
 	identifiers: &'a Vec<Id>,
 	used_as_object: bool,
+	used_as_call: bool,
 }
 
 impl<'a> ObjectUsageChecker<'a> {
@@ -252,6 +257,12 @@ impl<'a> ObjectUsageChecker<'a> {
 }
 
 impl<'a> Visit for ObjectUsageChecker<'a> {
+	fn visit_call_expr(&mut self, _: &ast::CallExpr) {
+		// If we're in a call expression, we can't wrap it in a signal
+		// because it's a function call, and later we need to serialize it
+		self.used_as_call = true;
+	}
+
 	fn visit_member_expr(&mut self, node: &ast::MemberExpr) {
 		if self.used_as_object {
 			return;
@@ -266,13 +277,14 @@ impl<'a> Visit for ObjectUsageChecker<'a> {
 	}
 }
 
-fn is_used_as_object(expr: &ast::Expr, identifiers: &Vec<Id>) -> bool {
+fn is_used_as_object_or_call(expr: &ast::Expr, identifiers: &Vec<Id>) -> (bool, bool) {
 	let mut checker = ObjectUsageChecker {
 		identifiers,
 		used_as_object: false,
+		used_as_call: false,
 	};
 
 	expr.visit_with(&mut checker);
 
-	checker.used_as_object
+	(checker.used_as_object, checker.used_as_call)
 }
diff --git a/packages/qwik/src/optimizer/core/src/snapshots/qwik_core__test__example_derived_signals_cmp.snap b/packages/qwik/src/optimizer/core/src/snapshots/qwik_core__test__example_derived_signals_cmp.snap
@@ -65,6 +65,7 @@ const App_component_ckEPmXZlub0 = ()=>{
         global: globalThing,
         globalAccess: globalThing.thing,
         globalComputed: globalThing.thing + 'stuff',
+        noInline: signal.value(),
         noInline2: signal.value + unknown(),
         noInline3: mutable(signal),
         noInline4: signal.value + dep
@@ -88,16 +89,13 @@ const App_component_ckEPmXZlub0 = ()=>{
         ], 'p0.address.city.name?"true":"false"'),
         dep: dep,
         depAccess: dep.thing,
-        depComputed: dep.thing + 'stuff',
-        noInline: _fnSignal((p0)=>p0.value(), [
-            signal
-        ], "p0.value()")
+        depComputed: dep.thing + 'stuff'
     }, null, 3, "u6_0");
 };
 export const App = /*#__PURE__*/ componentQrl(/*#__PURE__*/ inlinedQrl(App_component_ckEPmXZlub0, "App_component_ckEPmXZlub0"));
 
 
-Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"mappings\":\";;;;;AACA,SAAqB,QAAQ,EAAE,OAAO,QAAQ,iBAAiB;AAE/D,SAAQ,GAAG,QAAO,SAAS;AAC3B,SAAQ,GAAG,QAAO,QAAQ;kCAEI;IAC7B,MAAM,SAAS,UAAU;IACzB,MAAM,QAAQ,SAAS,CAAC;IACxB,qBACC,WAAC;QAmBA,QAAQ;QACR,cAAc,YAAY,KAAK;QAC/B,gBAAgB,YAAY,KAAK,GAAG;QAIpC,WAAW,OAAO,KAAK,GAAG;QAC1B,WAAW,QAAQ;QACnB,WAAW,OAAO,KAAK,GAAG;;QA1B1B,YAAW;QACX,aAAa,CAAC,IAAI,CAAC;QACnB,cAAc;QACd,eAAe;QACf,YAAY,CAAC,IAAI,EAAE,IAAI;QACvB,aAAa,OAAO,CAAC,IAAI,EAAE,IAAI,KAAK,WAAW,KAAK;QAEpD,QAAQ;QACR,WAAW,YAAE;QACb,mBAAmB,kBAAE,KAAK,GAAO,KAAK;;;QAEtC,KAAK,kBAAE,GAAM,OAAO,CAAC,IAAI,CAAC,IAAI;;;QAC9B,aAAa,kBAAE,GAAM,OAAO,CAAC,IAAI,CAAC,IAAI,GAAG,SAAS;;;QAElD,KAAK;QACL,WAAW,IAAI,KAAK;QACpB,aAAa,IAAI,KAAK,GAAG;QAOzB,QAAQ,kBAAE,GAAO,KAAK;;;;AAMzB;AAlCA,OAAO,MAAM,oBAAM,+FAkChB\"}")
+Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"mappings\":\";;;;;AACA,SAAqB,QAAQ,EAAE,OAAO,QAAQ,iBAAiB;AAE/D,SAAQ,GAAG,QAAO,SAAS;AAC3B,SAAQ,GAAG,QAAO,QAAQ;kCAEI;IAC7B,MAAM,SAAS,UAAU;IACzB,MAAM,QAAQ,SAAS,CAAC;IACxB,qBACC,WAAC;QAmBA,QAAQ;QACR,cAAc,YAAY,KAAK;QAC/B,gBAAgB,YAAY,KAAK,GAAG;QAGpC,UAAU,OAAO,KAAK;QACtB,WAAW,OAAO,KAAK,GAAG;QAC1B,WAAW,QAAQ;QACnB,WAAW,OAAO,KAAK,GAAG;;QA1B1B,YAAW;QACX,aAAa,CAAC,IAAI,CAAC;QACnB,cAAc;QACd,eAAe;QACf,YAAY,CAAC,IAAI,EAAE,IAAI;QACvB,aAAa,OAAO,CAAC,IAAI,EAAE,IAAI,KAAK,WAAW,KAAK;QAEpD,QAAQ;QACR,WAAW,YAAE;QACb,mBAAmB,kBAAE,KAAK,GAAO,KAAK;;;QAEtC,KAAK,kBAAE,GAAM,OAAO,CAAC,IAAI,CAAC,IAAI;;;QAC9B,aAAa,kBAAE,GAAM,OAAO,CAAC,IAAI,CAAC,IAAI,GAAG,SAAS;;;QAElD,KAAK;QACL,WAAW,IAAI,KAAK;QACpB,aAAa,IAAI,KAAK,GAAG;;AAa5B;AAlCA,OAAO,MAAM,oBAAM,+FAkChB\"}")
 == DIAGNOSTICS ==
 
 []
diff --git a/packages/qwik/src/optimizer/core/src/snapshots/qwik_core__test__example_derived_signals_div.snap b/packages/qwik/src/optimizer/core/src/snapshots/qwik_core__test__example_derived_signals_div.snap
@@ -83,6 +83,7 @@ const App_component_ckEPmXZlub0 = (props)=>{
         global: globalThing,
         globalAccess: globalThing.thing,
         globalComputed: globalThing.thing + 'stuff',
+        noInline: signal.value(),
         noInline2: signal.value + unknown(),
         noInline3: mutable(signal),
         noInline4: signal.value + dep,
@@ -108,16 +109,13 @@ const App_component_ckEPmXZlub0 = (props)=>{
         ], 'p0.address.city.name?"true":"false"'),
         dep: dep,
         depAccess: dep.thing,
-        depComputed: dep.thing + 'stuff',
-        noInline: _fnSignal((p0)=>p0.value(), [
-            signal
-        ], "p0.value()")
+        depComputed: dep.thing + 'stuff'
     }, null, 3, "u6_0");
 };
 export const App = /*#__PURE__*/ componentQrl(/*#__PURE__*/ inlinedQrl(App_component_ckEPmXZlub0, "App_component_ckEPmXZlub0"));
 
 
-Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"mappings\":\";;;;;AACA,SAAqB,QAAQ,EAAE,OAAO,QAAQ,iBAAiB;AAE/D,SAAQ,GAAG,QAAO,SAAS;AAC3B,OAAO,YAAY,sBAAsB;kCAEX,CAAC;IAC9B,MAAM,SAAS,UAAU;IACzB,MAAM,QAAQ,SAAS,CAAC;IACxB,MAAM,QAAQ,MAAM,OAAO,CAAC,KAAK;IAEjC,qBACC,WAAC;QACA,OAAO;YACN,MAAM,QAAQ,MAAM;YACpB,KAAK,QAAQ,MAAM;YACnB,SAAS;YACT,QAAQ;QACT;QAqBA,QAAQ;QACR,cAAc,YAAY,KAAK;QAC/B,gBAAgB,YAAY,KAAK,GAAG;QAIpC,WAAW,OAAO,KAAK,GAAG;QAC1B,WAAW,QAAQ;QACnB,WAAW,OAAO,KAAK,GAAG;QA3B1B,gBAAgB,OAAO,QAAQ;;QAD/B,aAAa,OAAO,GAAG;QAEvB,YAAW;QACX,aAAa,CAAC,IAAI,CAAC;QACnB,cAAc;QACd,eAAe;QACf,YAAY,CAAC,IAAI,EAAE,IAAI;QACvB,aAAa,OAAO,CAAC,IAAI,EAAE,IAAI,KAAK,WAAW,KAAK;QAEpD,QAAQ;QACR,WAAW,YAAE;QACb,mBAAmB,kBAAE,KAAK,GAAO,KAAK;;;QAEtC,KAAK,kBAAE,GAAM,OAAO,CAAC,IAAI,CAAC,IAAI;;;QAC9B,aAAa,kBAAE,GAAM,OAAO,CAAC,IAAI,CAAC,IAAI,GAAG,SAAS;;;QAElD,KAAK;QACL,WAAW,IAAI,KAAK;QACpB,aAAa,IAAI,KAAK,GAAG;QAOzB,QAAQ,kBAAE,GAAO,KAAK;;;;AAOzB;AA7CA,OAAO,MAAM,oBAAM,+FA6ChB\"}")
+Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"mappings\":\";;;;;AACA,SAAqB,QAAQ,EAAE,OAAO,QAAQ,iBAAiB;AAE/D,SAAQ,GAAG,QAAO,SAAS;AAC3B,OAAO,YAAY,sBAAsB;kCAEX,CAAC;IAC9B,MAAM,SAAS,UAAU;IACzB,MAAM,QAAQ,SAAS,CAAC;IACxB,MAAM,QAAQ,MAAM,OAAO,CAAC,KAAK;IAEjC,qBACC,WAAC;QACA,OAAO;YACN,MAAM,QAAQ,MAAM;YACpB,KAAK,QAAQ,MAAM;YACnB,SAAS;YACT,QAAQ;QACT;QAqBA,QAAQ;QACR,cAAc,YAAY,KAAK;QAC/B,gBAAgB,YAAY,KAAK,GAAG;QAGpC,UAAU,OAAO,KAAK;QACtB,WAAW,OAAO,KAAK,GAAG;QAC1B,WAAW,QAAQ;QACnB,WAAW,OAAO,KAAK,GAAG;QA3B1B,gBAAgB,OAAO,QAAQ;;QAD/B,aAAa,OAAO,GAAG;QAEvB,YAAW;QACX,aAAa,CAAC,IAAI,CAAC;QACnB,cAAc;QACd,eAAe;QACf,YAAY,CAAC,IAAI,EAAE,IAAI;QACvB,aAAa,OAAO,CAAC,IAAI,EAAE,IAAI,KAAK,WAAW,KAAK;QAEpD,QAAQ;QACR,WAAW,YAAE;QACb,mBAAmB,kBAAE,KAAK,GAAO,KAAK;;;QAEtC,KAAK,kBAAE,GAAM,OAAO,CAAC,IAAI,CAAC,IAAI;;;QAC9B,aAAa,kBAAE,GAAM,OAAO,CAAC,IAAI,CAAC,IAAI,GAAG,SAAS;;;QAElD,KAAK;QACL,WAAW,IAAI,KAAK;QACpB,aAAa,IAAI,KAAK,GAAG;;AAc5B;AA7CA,OAAO,MAAM,oBAAM,+FA6ChB\"}")
 == DIAGNOSTICS ==
 
 []
diff --git a/packages/qwik/src/optimizer/core/src/snapshots/qwik_core__test__should_not_wrap_ternary_function_operator_with_fn.snap b/packages/qwik/src/optimizer/core/src/snapshots/qwik_core__test__should_not_wrap_ternary_function_operator_with_fn.snap
@@ -0,0 +1,75 @@
+---
+source: packages/qwik/src/optimizer/core/src/test.rs
+assertion_line: 4405
+expression: output
+snapshot_kind: text
+---
+==INPUT==
+
+
+		import { component$, useSignal } from '@qwik.dev/core';
+
+
+		export default component$(() => {
+		const toggle = useSignal(true);
+		const t = (key: string) => key;
+		return (
+			<button
+				type="button"
+				title={
+				toggle.value !== ''
+					? t('app.message.exists@@there is a message for you')
+					: t('app.message.not_exists@@click to get a message!')
+				}
+			></button>
+		);
+		});
+		
+============================= test.js ==
+
+import { componentQrl } from "@qwik.dev/core";
+import { qrl } from "@qwik.dev/core";
+const i_LUXeXe0DQrg = ()=>import("./test.tsx_test_component_LUXeXe0DQrg");
+export default /*#__PURE__*/ componentQrl(/*#__PURE__*/ qrl(i_LUXeXe0DQrg, "test_component_LUXeXe0DQrg"));
+
+
+Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"mappings\":\";;;AAIE,6BAAe,6EAaZ\"}")
+============================= test.tsx_test_component_LUXeXe0DQrg.js (ENTRY POINT)==
+
+import { _jsxSorted } from "@qwik.dev/core";
+import { useSignal } from "@qwik.dev/core";
+export const test_component_LUXeXe0DQrg = ()=>{
+    const toggle = useSignal(true);
+    const t = (key)=>key;
+    return /*#__PURE__*/ _jsxSorted("button", {
+        title: toggle.value !== '' ? t('app.message.exists@@there is a message for you') : t('app.message.not_exists@@click to get a message!')
+    }, {
+        type: "button"
+    }, null, 3, "u6_0");
+};
+
+
+Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"mappings\":\";;0CAI4B;IAC1B,MAAM,SAAS,UAAU;IACzB,MAAM,IAAI,CAAC,MAAgB;IAC3B,qBACC,WAAC;QAEA,OACA,OAAO,KAAK,KAAK,KACd,EAAE,oDACF,EAAE;;QAJL,MAAK;;AAQP\"}")
+/*
+{
+  "origin": "test.tsx",
+  "name": "test_component_LUXeXe0DQrg",
+  "entry": null,
+  "displayName": "test.tsx_test_component",
+  "hash": "LUXeXe0DQrg",
+  "canonicalFilename": "test.tsx_test_component_LUXeXe0DQrg",
+  "path": "",
+  "extension": "js",
+  "parent": null,
+  "ctxKind": "function",
+  "ctxName": "component$",
+  "captures": false,
+  "loc": [
+    90,
+    389
+  ]
+}
+*/
+== DIAGNOSTICS ==
+
+[]
diff --git a/packages/qwik/src/optimizer/core/src/test.rs b/packages/qwik/src/optimizer/core/src/test.rs
@@ -4400,6 +4400,35 @@ fn should_wrap_logical_expression_in_template() {
 	});
 }
 
+#[test]
+fn should_not_wrap_ternary_function_operator_with_fn() {
+	test_input!(TestInput {
+		code: r#"
+		import { component$, useSignal } from '@qwik.dev/core';
+
+
+		export default component$(() => {
+		const toggle = useSignal(true);
+		const t = (key: string) => key;
+		return (
+			<button
+				type="button"
+				title={
+				toggle.value !== ''
+					? t('app.message.exists@@there is a message for you')
+					: t('app.message.not_exists@@click to get a message!')
+				}
+			></button>
+		);
+		});
+		"#
+		.to_string(),
+		transpile_ts: true,
+		transpile_jsx: true,
+		..TestInput::default()
+	});
+}
+
 // TODO(misko): Make this test work by implementing strict serialization.
 // #[test]
 // fn example_of_synchronous_qrl_that_cant_be_serialized() {

-Original file line number
+Diff line change
@@ @@ -0,0 +1,5 @@ @@
 +---
 +'@qwik.dev/core': patch
 +---
++
 +fix: don't wrap function calls in signal