Skip to content

Commit ef18cce

Browse files
JerryWu1234wuls
JerryWu1234
and
wuls
authored
fix: optimizer is now better at recognizing constProp (#7316)
Co-authored-by: wuls <[email protected]>
1 parent b2b8bd0 commit ef18cce

File tree

5 files changed

+175
-19
lines changed

5 files changed

+175
-19
lines changed

‎.changeset/lemon-tools-bathe.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@qwik.dev/core': patch
3+
---
4+
5+
FIX: optimizer is now better at recognizing constProp

‎packages/qwik/src/optimizer/core/src/snapshots/qwik_core__test__issue_7216_add_test.snap

+124
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
---
2+
source: packages/qwik/src/optimizer/core/src/test.rs
3+
assertion_line: 4275
4+
expression: output
5+
snapshot_kind: text
6+
---
7+
==INPUT==
8+
9+
10+
import { component$ } from '@builder.io/qwik';
11+
export default component$((props) => {
12+
return (<p
13+
onHi$={() => 'hi'}
14+
{...props.foo}
15+
onHello$={props.helloHandler$}
16+
{...props.rest}
17+
onVar$={props.onVarHandler$}
18+
onConst$={() => 'const'}
19+
asd={"1"}
20+
/>);
21+
});
22+
23+
============================= test.js ==
24+
25+
import { componentQrl } from "@qwik.dev/core";
26+
import { qrl } from "@qwik.dev/core";
27+
export default /*#__PURE__*/ componentQrl(/*#__PURE__*/ qrl(()=>import("./test.tsx_test_component_LUXeXe0DQrg"), "test_component_LUXeXe0DQrg"));
28+
29+
30+
Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"mappings\":\";;AAEA,6BAAe,mHAUZ\"}")
31+
============================= test.tsx_test_component_p_onHi_MzKNRhr770k.js (ENTRY POINT)==
32+
33+
export const test_component_p_onHi_MzKNRhr770k = ()=>'hi';
34+
35+
36+
Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"mappings\":\"iDAIS,IAAM\"}")
37+
/*
38+
{
39+
"origin": "test.tsx",
40+
"name": "test_component_p_onHi_MzKNRhr770k",
41+
"entry": null,
42+
"displayName": "test.tsx_test_component_p_onHi",
43+
"hash": "MzKNRhr770k",
44+
"canonicalFilename": "test.tsx_test_component_p_onHi_MzKNRhr770k",
45+
"path": "",
46+
"extension": "js",
47+
"parent": "test_component_LUXeXe0DQrg",
48+
"ctxKind": "eventHandler",
49+
"ctxName": "onHi$",
50+
"captures": false,
51+
"loc": [
52+
111,
53+
121
54+
]
55+
}
56+
*/
57+
============================= test.tsx_test_component_p_onConst_WVa6OMq4yIM.js (ENTRY POINT)==
58+
59+
export const test_component_p_onConst_WVa6OMq4yIM = ()=>'const';
60+
61+
62+
Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"mappings\":\"oDASY,IAAM\"}")
63+
/*
64+
{
65+
"origin": "test.tsx",
66+
"name": "test_component_p_onConst_WVa6OMq4yIM",
67+
"entry": null,
68+
"displayName": "test.tsx_test_component_p_onConst",
69+
"hash": "WVa6OMq4yIM",
70+
"canonicalFilename": "test.tsx_test_component_p_onConst_WVa6OMq4yIM",
71+
"path": "",
72+
"extension": "js",
73+
"parent": "test_component_LUXeXe0DQrg",
74+
"ctxKind": "eventHandler",
75+
"ctxName": "onConst$",
76+
"captures": false,
77+
"loc": [
78+
239,
79+
252
80+
]
81+
}
82+
*/
83+
============================= test.tsx_test_component_LUXeXe0DQrg.js (ENTRY POINT)==
84+
85+
import { _jsxSplit } from "@qwik.dev/core";
86+
import { qrl } from "@qwik.dev/core";
87+
export const test_component_LUXeXe0DQrg = (props)=>{
88+
return /*#__PURE__*/ _jsxSplit("p", {
89+
onHi$: /*#__PURE__*/ qrl(()=>import("./test.tsx_test_component_p_onHi_MzKNRhr770k"), "test_component_p_onHi_MzKNRhr770k"),
90+
...props.foo,
91+
onHello$: props.helloHandler$,
92+
...props.rest,
93+
onVar$: props.onVarHandler$
94+
}, {
95+
onConst$: /*#__PURE__*/ qrl(()=>import("./test.tsx_test_component_p_onConst_WVa6OMq4yIM"), "test_component_p_onConst_WVa6OMq4yIM"),
96+
asd: "1"
97+
}, null, 0, "u6_0");
98+
};
99+
100+
101+
Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"mappings\":\";;0CAE0B,CAAC;IACzB,qBAAQ,UAAC;QACT,KAAK;QACJ,GAAG,MAAM,GAAG;QACb,UAAU,MAAM,aAAa;QAC5B,GAAG,MAAM,IAAI;QACd,QAAQ,MAAM,aAAa;;QAC3B,QAAQ;QACR,KAAK;;AAEP\"}")
102+
/*
103+
{
104+
"origin": "test.tsx",
105+
"name": "test_component_LUXeXe0DQrg",
106+
"entry": null,
107+
"displayName": "test.tsx_test_component",
108+
"hash": "LUXeXe0DQrg",
109+
"canonicalFilename": "test.tsx_test_component_LUXeXe0DQrg",
110+
"path": "",
111+
"extension": "js",
112+
"parent": null,
113+
"ctxKind": "function",
114+
"ctxName": "component$",
115+
"captures": false,
116+
"loc": [
117+
75,
118+
274
119+
]
120+
}
121+
*/
122+
== DIAGNOSTICS ==
123+
124+
[]

‎packages/qwik/src/optimizer/core/src/snapshots/qwik_core__test__ternary_prop.snap

+3-3
Original file line numberDiff line numberDiff line change
@@ -64,15 +64,15 @@ export const Cmp_component_4ryKJTOKjWE = ()=>{
6464
toggleSig
6565
]);
6666
return /*#__PURE__*/ _jsxSorted("button", null, {
67+
onClick$: handleClick$,
6768
"data-open": _fnSignal((p0)=>p0.value ? true : undefined, [
6869
toggleSig
69-
], "p0.value?true:undefined"),
70-
onClick$: handleClick$
70+
], "p0.value?true:undefined")
7171
}, "Removing data-open re-renders", 3, "u6_0");
7272
};
7373

7474

75-
Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"mappings\":\";;;;yCAEgC;IAC7B,MAAM,YAAY,UAAU;IAE5B,MAAM;;;IAIN,qBACC,WAAC;QAA+B,WAAS,kBAAE,GAAU,KAAK,GAAG,OAAO;;;QAA5D,UAAU;OAA6D;AAIjF\"}")
75+
Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"mappings\":\";;;;yCAEgC;IAC7B,MAAM,YAAY,UAAU;IAE5B,MAAM;;;IAIN,qBACC,WAAC;QAAO,UAAU;QAAc,WAAS,kBAAE,GAAU,KAAK,GAAG,OAAO;;;OAAW;AAIjF\"}")
7676
/*
7777
{
7878
"origin": "test.tsx",

‎packages/qwik/src/optimizer/core/src/test.rs

+24
Original file line numberDiff line numberDiff line change
@@ -4269,6 +4269,30 @@ export const Cmp = component$(() => {
42694269
..TestInput::default()
42704270
});
42714271
}
4272+
4273+
#[test]
4274+
fn issue_7216_add_test() {
4275+
test_input!(TestInput {
4276+
code: r#"
4277+
import { component$ } from '@builder.io/qwik';
4278+
export default component$((props) => {
4279+
return (<p
4280+
onHi$={() => 'hi'}
4281+
{...props.foo}
4282+
onHello$={props.helloHandler$}
4283+
{...props.rest}
4284+
onVar$={props.onVarHandler$}
4285+
onConst$={() => 'const'}
4286+
asd={"1"}
4287+
/>);
4288+
});
4289+
"#
4290+
.to_string(),
4291+
transpile_ts: true,
4292+
transpile_jsx: true,
4293+
..TestInput::default()
4294+
});
4295+
}
42724296
// TODO(misko): Make this test work by implementing strict serialization.
42734297
// #[test]
42744298
// fn example_of_synchronous_qrl_that_cant_be_serialized() {

‎packages/qwik/src/optimizer/core/src/transform.rs

+19-16
Original file line numberDiff line numberDiff line change
@@ -1184,7 +1184,6 @@ impl<'a> QwikTransform<'a> {
11841184
let mut var_props = vec![];
11851185
let mut const_props = vec![];
11861186
let mut children = None;
1187-
let mut event_handlers = vec![];
11881187
// The identifiers that are static
11891188
let const_idents: Vec<_> = self
11901189
.decl_stack
@@ -1200,6 +1199,7 @@ impl<'a> QwikTransform<'a> {
12001199
.iter()
12011200
.filter(|prop| !matches!(prop, ast::PropOrSpread::Prop(_)))
12021201
.count();
1202+
12031203
let has_spread_props = spread_props_count > 0;
12041204
let should_runtime_sort = has_spread_props;
12051205
let mut static_listeners = !has_spread_props;
@@ -1376,9 +1376,6 @@ impl<'a> QwikTransform<'a> {
13761376
event_handler.clone(),
13771377
None,
13781378
);
1379-
if !is_const {
1380-
static_listeners = false;
1381-
}
13821379
let converted_prop = ast::PropOrSpread::Prop(Box::new(
13831380
ast::Prop::KeyValue(ast::KeyValueProp {
13841381
value: Box::new(ast::Expr::Call(converted_expr)),
@@ -1389,7 +1386,14 @@ impl<'a> QwikTransform<'a> {
13891386
}),
13901387
}),
13911388
));
1392-
event_handlers.push(converted_prop);
1389+
if !is_const {
1390+
static_listeners = false;
1391+
var_props.push(converted_prop.clone().fold_with(self));
1392+
} else {
1393+
maybe_const_props
1394+
.push(converted_prop.clone().fold_with(self));
1395+
}
1396+
// event_handlers.push(converted_prop);
13931397
} else if !is_fn && (key_word == *REF || key_word == *QSLOT) {
13941398
// skip
13951399
var_props.push(prop.fold_with(self));
@@ -1425,8 +1429,10 @@ impl<'a> QwikTransform<'a> {
14251429
} else {
14261430
var_props.push(converted_prop.fold_with(self));
14271431
}
1432+
} else if !is_const || spread_props_count > 0 {
1433+
var_props.push(converted_prop.fold_with(self));
14281434
} else {
1429-
event_handlers.push(converted_prop.fold_with(self));
1435+
const_props.push(converted_prop.fold_with(self));
14301436
}
14311437
} else {
14321438
let const_prop = is_const_expr(
@@ -1444,8 +1450,10 @@ impl<'a> QwikTransform<'a> {
14441450
} else {
14451451
var_props.push(prop.fold_with(self));
14461452
}
1453+
} else if !const_prop || spread_props_count > 0 {
1454+
var_props.push(prop.fold_with(self));
14471455
} else {
1448-
event_handlers.push(prop.fold_with(self));
1456+
const_props.push(prop.fold_with(self));
14491457
}
14501458
}
14511459
} else if is_const_expr(
@@ -1458,12 +1466,12 @@ impl<'a> QwikTransform<'a> {
14581466
self.convert_to_getter(&node.value)
14591467
{
14601468
let key = node.key.clone();
1461-
let entry = ast::PropOrSpread::Prop(Box::new(
1462-
ast::Prop::KeyValue(ast::KeyValueProp {
1469+
let entry: ast::PropOrSpread = ast::PropOrSpread::Prop(
1470+
Box::new(ast::Prop::KeyValue(ast::KeyValueProp {
14631471
key,
14641472
value: Box::new(getter),
1465-
}),
1466-
));
1473+
})),
1474+
);
14671475
if is_fn || is_const {
14681476
maybe_const_props.push(entry);
14691477
} else {
@@ -1501,15 +1509,10 @@ impl<'a> QwikTransform<'a> {
15011509
let mut flags = 0;
15021510
if static_listeners {
15031511
flags |= 1 << 0;
1504-
const_props.extend(event_handlers);
1505-
} else {
1506-
var_props.extend(event_handlers);
15071512
}
1508-
15091513
if static_subtree {
15101514
flags |= 1 << 1;
15111515
}
1512-
15131516
if !should_runtime_sort {
15141517
var_props.sort_by(|a: &ast::PropOrSpread, b: &ast::PropOrSpread| {
15151518
match (a, b) {

0 commit comments

Comments
 (0)