Skip to content

Commit ff77362

Browse files
VarixoVarixo
committed
refactor: check for var prop after last spread
1 parent 8fa44f3 commit ff77362

8 files changed

+151
-30
lines changed

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,9 @@ export const Works = /*#__PURE__*/ componentQrl(/*#__PURE__*/ inlinedQrl((_rawPr
8787
_rawProps
8888
], "{some:p0.some??1+2}"),
8989
class: _wrapProp(_rawProps, "count"),
90-
..._getVarProps(rest),
91-
..._getConstProps(rest)
90+
..._getVarProps(rest)
9291
}, {
92+
..._getConstProps(rest),
9393
override: true
9494
}, _wrapProp(_rawProps, "count"), 0, "u6_0");
9595
}, "Works_component_t45qL4vNGv0"));
@@ -121,7 +121,7 @@ export const NoWorks3 = /*#__PURE__*/ componentQrl(/*#__PURE__*/ inlinedQrl(({ c
121121
}, "NoWorks3_component_fc13h5yYn14"));
122122

123123

124-
Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"mappings\":\";;;;;;;;;;;AAGA,OAAO,MAAM,sBAAQ,sCAAW;;;;;;;;IAO/B,QAAQ,GAAG,WAHX,iBAFA,QAAO;IAMP,oCAAS,CAAC,EAAC,KAAK,EAAC;;QAChB,MAAM,cARP;QASC,QAAQ,GAAG,WATZ,OASoB,gBANpB,iBAFA,QAAO,aAGP,gBAAqB;;;;;IAOrB,qBACC,UAAC;QAAI,IAAI,qBAXV,QAAO;;;QAWW,MAAM,kBAAE,CAAA;gBAAE,IAAI,KAXhC,QAAO;YAW0B,CAAA;;;QAAG,KAAK;wBAAa;0BAAA;;QAAM,QAAQ;;AAErE,mCAAG;AAEH,OAAO,MAAM,yBAAW,sCAAW,CAAC,EAAC,KAAK,EAAE,OAAO,EAAC,GAAG,EAAC,EAAC;IACxD,QAAQ,GAAG,CAAC;IACZ,oCAAS,CAAC,EAAC,KAAK,EAAC;;QAChB,MAAM,IAAM;QACZ,QAAQ,GAAG,CAAC;;;;IAEb,qBACC,WAAC;QAAI,OAAO;aAAQ;AAEtB,sCAAG;AAEH,OAAO,MAAM,yBAAW,sCAAW,CAAC,EAAC,KAAK,EAAE,QAAQ,MAAM,EAAC;IAC1D,QAAQ,GAAG,CAAC;IACZ,oCAAS,CAAC,EAAC,KAAK,EAAC;;QAChB,MAAM,IAAM;QACZ,QAAQ,GAAG,CAAC;;;;IAEb,qBACC,WAAC;QAAI,OAAO;aAAQ;AAEtB,sCAAG\"}")
124+
Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"mappings\":\";;;;;;;;;;;AAGA,OAAO,MAAM,sBAAQ,sCAAW;;;;;;;;IAO/B,QAAQ,GAAG,WAHX,iBAFA,QAAO;IAMP,oCAAS,CAAC,EAAC,KAAK,EAAC;;QAChB,MAAM,cARP;QASC,QAAQ,GAAG,WATZ,OASoB,gBANpB,iBAFA,QAAO,aAGP,gBAAqB;;;;;IAOrB,qBACC,UAAC;QAAI,IAAI,qBAXV,QAAO;;;QAWW,MAAM,kBAAE,CAAA;gBAAE,IAAI,KAXhC,QAAO;YAW0B,CAAA;;;QAAG,KAAK;wBAAa;;0BAAA;QAAM,QAAQ;;AAErE,mCAAG;AAEH,OAAO,MAAM,yBAAW,sCAAW,CAAC,EAAC,KAAK,EAAE,OAAO,EAAC,GAAG,EAAC,EAAC;IACxD,QAAQ,GAAG,CAAC;IACZ,oCAAS,CAAC,EAAC,KAAK,EAAC;;QAChB,MAAM,IAAM;QACZ,QAAQ,GAAG,CAAC;;;;IAEb,qBACC,WAAC;QAAI,OAAO;aAAQ;AAEtB,sCAAG;AAEH,OAAO,MAAM,yBAAW,sCAAW,CAAC,EAAC,KAAK,EAAE,QAAQ,MAAM,EAAC;IAC1D,QAAQ,GAAG,CAAC;IACZ,oCAAS,CAAC,EAAC,KAAK,EAAC;;QAChB,MAAM,IAAM;QACZ,QAAQ,GAAG,CAAC;;;;IAEb,qBACC,WAAC;QAAI,OAAO;aAAQ;AAEtB,sCAAG\"}")
125125
== DIAGNOSTICS ==
126126

127127
[]

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,8 @@ export const test_component_LUXeXe0DQrg = (_rawProps)=>{
6161
id: _wrapProp(_rawProps, "id")
6262
}, null, [
6363
/*#__PURE__*/ _jsxSplit("span", {
64-
..._getVarProps(rest),
65-
..._getConstProps(rest)
66-
}, null, [
64+
..._getVarProps(rest)
65+
}, _getConstProps(rest), [
6766
_wrapProp(_rawProps, "message"),
6867
" ",
6968
_wrapProp(_rawProps, "count")
@@ -75,7 +74,7 @@ export const test_component_LUXeXe0DQrg = (_rawProps)=>{
7574
};
7675

7776

78-
Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"mappings\":\";;;;;;0CAK4B;;;;;;IACzB,MAAM,UAAU,SAAS;QAAE,SAAS;IAAE,GAAG;QAAE,UAAU;IAAM;IAC3D,QAAQ,OAAO;IACf,MAAM,YAAY,QAAQ,OAAO,GAAG;IACpC,qBACC,WAAC;QAAI,EAAE;;sBACN,UAAC;4BAAS;8BAAA;;;YACD;;;sBAET,WAAC;YAAI,OAAM;WAAW;;AAGzB\"}")
77+
Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"mappings\":\";;;;;;0CAK4B;;;;;;IACzB,MAAM,UAAU,SAAS;QAAE,SAAS;IAAE,GAAG;QAAE,UAAU;IAAM;IAC3D,QAAQ,OAAO;IACf,MAAM,YAAY,QAAQ,OAAO,GAAG;IACpC,qBACC,WAAC;QAAI,EAAE;;sBACN,UAAC;4BAAS;0BAAA;;YACD;;;sBAET,WAAC;YAAI,OAAM;WAAW;;AAGzB\"}")
7978
/*
8079
{
8180
"origin": "test.tsx",

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

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
---
2+
source: packages/qwik/src/optimizer/core/src/test.rs
3+
assertion_line: 4617
4+
expression: output
5+
---
6+
==INPUT==
7+
8+
9+
import { component$ } from '@qwik.dev/core';
10+
11+
export default component$((props) => {
12+
return <div {...props} class={[props.class, 'component']} {...props} />;
13+
});
14+
15+
============================= test.js ==
16+
17+
import { componentQrl } from "@qwik.dev/core";
18+
import { qrl } from "@qwik.dev/core";
19+
const i_LUXeXe0DQrg = ()=>import("./test.tsx_test_component_LUXeXe0DQrg");
20+
export default /*#__PURE__*/ componentQrl(/*#__PURE__*/ qrl(i_LUXeXe0DQrg, "test_component_LUXeXe0DQrg"));
21+
22+
23+
Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"mappings\":\";;;AAGE,6BAAe,6EAEZ\"}")
24+
============================= test.tsx_test_component_LUXeXe0DQrg.js (ENTRY POINT)==
25+
26+
import { _fnSignal } from "@qwik.dev/core";
27+
import { _getConstProps } from "@qwik.dev/core";
28+
import { _getVarProps } from "@qwik.dev/core";
29+
import { _jsxSplit } from "@qwik.dev/core";
30+
export const test_component_LUXeXe0DQrg = (props)=>{
31+
return /*#__PURE__*/ _jsxSplit("div", {
32+
..._getVarProps(props),
33+
..._getConstProps(props),
34+
class: _fnSignal((p0)=>[
35+
p0.class,
36+
'component'
37+
], [
38+
props
39+
], '[p0.class,"component"]'),
40+
..._getVarProps(props)
41+
}, _getConstProps(props), null, 0, "u6_0");
42+
};
43+
44+
45+
Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"mappings\":\";;;;0CAG4B,CAAC;IAC1B,qBAAO,UAAC;wBAAQ;0BAAA;QAAO,KAAK,kBAAE;gBAAC,GAAM,KAAK;gBAAE;aAAY;;;wBAAM;sBAAA;AAC/D\"}")
46+
/*
47+
{
48+
"origin": "test.tsx",
49+
"name": "test_component_LUXeXe0DQrg",
50+
"entry": null,
51+
"displayName": "test.tsx_test_component",
52+
"hash": "LUXeXe0DQrg",
53+
"canonicalFilename": "test.tsx_test_component_LUXeXe0DQrg",
54+
"path": "",
55+
"extension": "js",
56+
"parent": null,
57+
"ctxKind": "function",
58+
"ctxName": "component$",
59+
"captures": false,
60+
"loc": [
61+
78,
62+
170
63+
],
64+
"paramNames": [
65+
"props"
66+
]
67+
}
68+
*/
69+
== DIAGNOSTICS ==
70+
71+
[]

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,15 +30,15 @@ import { _getVarProps } from "@qwik.dev/core";
3030
import { _jsxSplit } from "@qwik.dev/core";
3131
export const test_component_LUXeXe0DQrg = (props)=>{
3232
return /*#__PURE__*/ _jsxSplit("div", {
33-
..._getVarProps(props),
34-
..._getConstProps(props)
33+
..._getVarProps(props)
3534
}, {
35+
..._getConstProps(props),
3636
test: "test"
3737
}, null, 0, "u6_0");
3838
};
3939

4040

41-
Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"mappings\":\";;;0CAG4B,CAAC;IAC1B,qBACC,UAAC;wBAAQ;0BAAA;;QAAO,MAAK;;AAEvB\"}")
41+
Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"mappings\":\";;;0CAG4B,CAAC;IAC1B,qBACC,UAAC;wBAAQ;;0BAAA;QAAO,MAAK;;AAEvB\"}")
4242
/*
4343
{
4444
"origin": "test.tsx",

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,12 @@ import { _jsxSplit } from "@qwik.dev/core";
3131
export const test_component_LUXeXe0DQrg = (props)=>{
3232
return /*#__PURE__*/ _jsxSplit("div", {
3333
test: "test",
34-
..._getVarProps(props),
35-
..._getConstProps(props)
36-
}, null, null, 0, "u6_0");
34+
..._getVarProps(props)
35+
}, _getConstProps(props), null, 0, "u6_0");
3736
};
3837

3938

40-
Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"mappings\":\";;;0CAG4B,CAAC;IAC1B,qBACC,UAAC;QAAI,MAAK;wBAAW;0BAAA;;AAEvB\"}")
39+
Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"mappings\":\";;;0CAG4B,CAAC;IAC1B,qBACC,UAAC;QAAI,MAAK;wBAAW;sBAAA;AAEvB\"}")
4140
/*
4241
{
4342
"origin": "test.tsx",

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,15 +64,14 @@ const i_DGk9xLyRokA = ()=>import("./test.tsx_test_component_button_onClick_DGk9x
6464
export const test_component_LUXeXe0DQrg = (props)=>{
6565
return /*#__PURE__*/ _jsxSplit("button", {
6666
..._getVarProps(props),
67-
..._getConstProps(props),
6867
onClick$: /*#__PURE__*/ qrl(i_DGk9xLyRokA, "test_component_button_onClick_DGk9xLyRokA", [
6968
props
7069
])
71-
}, null, null, 0, "u6_0");
70+
}, _getConstProps(props), null, 0, "u6_0");
7271
};
7372

7473

75-
Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"mappings\":\";;;;;0CAG4B,CAAC;IAC1B,qBAAO,UAAC;wBAAW;0BAAA;QAAO,QAAQ;;;;AACnC\"}")
74+
Some("{\"version\":3,\"sources\":[\"/user/qwik/src/test.tsx\"],\"names\":[],\"mappings\":\";;;;;0CAG4B,CAAC;IAC1B,qBAAO,UAAC;wBAAW;QAAO,QAAQ;;;sBAAf;AACpB\"}")
7675
/*
7776
{
7877
"origin": "test.tsx",

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4612,6 +4612,23 @@ fn should_merge_attributes() {
46124612
});
46134613
}
46144614

4615+
#[test]
4616+
fn should_merge_attributes2() {
4617+
test_input!(TestInput {
4618+
code: r#"
4619+
import { component$ } from '@qwik.dev/core';
4620+
4621+
export default component$((props) => {
4622+
return <div {...props} class={[props.class, 'component']} {...props} />;
4623+
});
4624+
"#
4625+
.to_string(),
4626+
transpile_ts: true,
4627+
transpile_jsx: true,
4628+
..TestInput::default()
4629+
});
4630+
}
4631+
46154632
// TODO(misko): Make this test work by implementing strict serialization.
46164633
// #[test]
46174634
// fn example_of_synchronous_qrl_that_cant_be_serialized() {

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

Lines changed: 49 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1200,9 +1200,50 @@ impl<'a> QwikTransform<'a> {
12001200
.cloned()
12011201
.collect();
12021202

1203+
let props = object.props;
1204+
let last_spread_index = props
1205+
.iter()
1206+
.rposition(|p| matches!(p, ast::PropOrSpread::Spread(_)));
1207+
let has_var_prop_after_last_spread = if let Some(index) = last_spread_index {
1208+
props[index + 1..].iter().any(|prop| {
1209+
if let ast::PropOrSpread::Prop(box ast::Prop::Shorthand(node)) = prop {
1210+
if node.sym == *CHILDREN {
1211+
return false;
1212+
}
1213+
}
1214+
if let ast::PropOrSpread::Prop(box ast::Prop::KeyValue(ref node)) = prop {
1215+
let key_word = match node.key {
1216+
ast::PropName::Ident(ref ident) => Some(ident.sym.clone()),
1217+
ast::PropName::Str(ref s) => Some(s.value.clone()),
1218+
_ => None,
1219+
};
1220+
1221+
if let Some(key_word) = key_word {
1222+
if key_word == *CHILDREN {
1223+
return false;
1224+
}
1225+
}
1226+
}
1227+
if let ast::PropOrSpread::Spread(_) = prop {
1228+
return true;
1229+
}
1230+
if let ast::PropOrSpread::Prop(box ast::Prop::KeyValue(node)) = prop {
1231+
if is_const_expr(
1232+
&node.value,
1233+
&self.options.global_collect,
1234+
Some(&const_idents),
1235+
) {
1236+
return false;
1237+
}
1238+
}
1239+
true
1240+
})
1241+
} else {
1242+
false
1243+
};
1244+
12031245
// Do we have spread arguments?
1204-
let mut spread_props_count = object
1205-
.props
1246+
let mut spread_props_count = props
12061247
.iter()
12071248
.filter(|prop| !matches!(prop, ast::PropOrSpread::Prop(_)))
12081249
.count();
@@ -1211,9 +1252,8 @@ impl<'a> QwikTransform<'a> {
12111252
let should_runtime_sort = has_spread_props;
12121253
let mut static_listeners = !has_spread_props;
12131254
let mut static_subtree = !has_spread_props;
1214-
let props_count = object.props.len();
12151255

1216-
for prop in object.props {
1256+
for prop in props.into_iter() {
12171257
let mut name_token = false;
12181258
// If we have spread props, all the props that come before it are variable even if they're static
12191259
let maybe_const_props = if spread_props_count > 0 {
@@ -1510,9 +1550,6 @@ impl<'a> QwikTransform<'a> {
15101550
let (_, var_props_call, const_props_call, _, _) =
15111551
self.handle_jsx_props_obj_spread(&spread);
15121552

1513-
// If there are more spreads after it,
1514-
// both _getVarProps and _getConstProps should be spread into the same props object
1515-
15161553
let var_props_call_prop =
15171554
ast::PropOrSpread::Spread(ast::SpreadElement {
15181555
expr: var_props_call.expr,
@@ -1525,15 +1562,14 @@ impl<'a> QwikTransform<'a> {
15251562
});
15261563

15271564
var_props.push(var_props_call_prop);
1528-
if spread_props_count > 1 {
1565+
if spread_props_count > 1 || has_var_prop_after_last_spread {
15291566
// Add both spreads to var_props since they'll be combined
1567+
// or props after the last spread are var props
15301568
var_props.push(const_props_call_prop);
1531-
} else if spread_props_count == props_count {
1532-
// Last spread - keep the original separation
1533-
const_props.push(const_props_call_prop);
15341569
} else {
1535-
// Single, not last spread
1536-
var_props.push(const_props_call_prop);
1570+
// Single spread or last spread - keep the original separation
1571+
// Props after the last spread are const props
1572+
const_props.push(const_props_call_prop);
15371573
}
15381574
} else {
15391575
// If the spread is not an ident, we need to handle it like default spread

0 commit comments

Comments
 (0)