Skip to content

Commit f1986da

Browse files
authored
feat: only inject push/init/pop when necessary (#11319)
* feat: only inject push/init/pop when necessary - closes #11297 * regenerate * differentiate between safe/unsafe * only inject $$props when necessary * more * fix * simplify * handle store subscriptions
1 parent 8e43e9a commit f1986da

File tree

14 files changed

+93
-48
lines changed

14 files changed

+93
-48
lines changed

.changeset/nervous-berries-boil.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'svelte': patch
3+
---
4+
5+
feat: only inject push/init/pop when necessary

packages/svelte/src/compiler/phases/2-analyze/index.js

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,8 @@ export function analyze_component(root, source, options) {
370370
uses_slots: false,
371371
uses_component_bindings: false,
372372
uses_render_tags: false,
373+
needs_context: false,
374+
needs_props: false,
373375
custom_element: options.customElementOptions ?? options.customElement,
374376
inject_styles: options.css === 'injected' || options.customElement,
375377
accessors: options.customElement
@@ -803,6 +805,8 @@ const legacy_scope_tweaker = {
803805
return next();
804806
}
805807

808+
state.analysis.needs_props = true;
809+
806810
if (!node.declaration) {
807811
for (const specifier of node.specifiers) {
808812
const binding = /** @type {import('#compiler').Binding} */ (
@@ -931,6 +935,8 @@ const runes_scope_tweaker = {
931935
}
932936

933937
if (rune === '$props') {
938+
state.analysis.needs_props = true;
939+
934940
for (const property of /** @type {import('estree').ObjectPattern} */ (node.id).properties) {
935941
if (property.type !== 'Property') continue;
936942

@@ -1038,6 +1044,33 @@ const function_visitor = (node, context) => {
10381044
});
10391045
};
10401046

1047+
/**
1048+
* A 'safe' identifier means that the `foo` in `foo.bar` or `foo()` will not
1049+
* call functions that require component context to exist
1050+
* @param {import('estree').Expression | import('estree').Super} expression
1051+
* @param {Scope} scope
1052+
*/
1053+
function is_safe_identifier(expression, scope) {
1054+
let node = expression;
1055+
while (node.type === 'MemberExpression') node = node.object;
1056+
1057+
if (node.type !== 'Identifier') return false;
1058+
1059+
const binding = scope.get(node.name);
1060+
if (!binding) return true;
1061+
1062+
if (binding.kind === 'store_sub') {
1063+
return is_safe_identifier({ name: node.name.slice(1), type: 'Identifier' }, scope);
1064+
}
1065+
1066+
return (
1067+
binding.declaration_kind !== 'import' &&
1068+
binding.kind !== 'prop' &&
1069+
binding.kind !== 'bindable_prop' &&
1070+
binding.kind !== 'rest_prop'
1071+
);
1072+
}
1073+
10411074
/** @type {import('./types').Visitors} */
10421075
const common_visitors = {
10431076
_(node, context) {
@@ -1209,14 +1242,16 @@ const common_visitors = {
12091242
}
12101243

12111244
const callee = node.callee;
1245+
const rune = get_rune(node, context.state.scope);
1246+
12121247
if (callee.type === 'Identifier') {
12131248
const binding = context.state.scope.get(callee.name);
12141249

12151250
if (binding !== null) {
12161251
binding.is_called = true;
12171252
}
12181253

1219-
if (get_rune(node, context.state.scope) === '$derived') {
1254+
if (rune === '$derived') {
12201255
// special case — `$derived(foo)` is treated as `$derived(() => foo)`
12211256
// for the purposes of identifying static state references
12221257
context.next({
@@ -1228,13 +1263,27 @@ const common_visitors = {
12281263
}
12291264
}
12301265

1266+
if (rune === '$effect' || rune === '$effect.pre') {
1267+
// `$effect` needs context because Svelte needs to know whether it should re-run
1268+
// effects that invalidate themselves, and that's determined by whether we're in runes mode
1269+
context.state.analysis.needs_context = true;
1270+
} else if (rune === null) {
1271+
if (!is_safe_identifier(callee, context.state.scope)) {
1272+
context.state.analysis.needs_context = true;
1273+
}
1274+
}
1275+
12311276
context.next();
12321277
},
12331278
MemberExpression(node, context) {
12341279
if (context.state.expression) {
12351280
context.state.expression.metadata.dynamic = true;
12361281
}
12371282

1283+
if (!is_safe_identifier(node, context.state.scope)) {
1284+
context.state.analysis.needs_context = true;
1285+
}
1286+
12381287
context.next();
12391288
},
12401289
BindDirective(node, context) {

packages/svelte/src/compiler/phases/3-transform/client/transform-client.js

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -351,12 +351,11 @@ export function client_component(source, analysis, options) {
351351
if (options.dev) push_args.push(b.id(analysis.name));
352352

353353
const component_block = b.block([
354-
b.stmt(b.call('$.push', ...push_args)),
355354
...store_setup,
356355
...legacy_reactive_declarations,
357356
...group_binding_declarations,
358357
.../** @type {import('estree').Statement[]} */ (instance.body),
359-
analysis.runes ? b.empty : b.stmt(b.call('$.init')),
358+
analysis.runes || !analysis.needs_context ? b.empty : b.stmt(b.call('$.init')),
360359
.../** @type {import('estree').Statement[]} */ (template.body)
361360
]);
362361

@@ -392,11 +391,22 @@ export function client_component(source, analysis, options) {
392391
: () => {};
393392

394393
append_styles();
395-
component_block.body.push(
396-
component_returned_object.length > 0
397-
? b.return(b.call('$.pop', b.object(component_returned_object)))
398-
: b.stmt(b.call('$.pop'))
399-
);
394+
395+
const should_inject_context =
396+
analysis.needs_context ||
397+
analysis.reactive_statements.size > 0 ||
398+
component_returned_object.length > 0 ||
399+
options.dev;
400+
401+
if (should_inject_context) {
402+
component_block.body.unshift(b.stmt(b.call('$.push', ...push_args)));
403+
404+
component_block.body.push(
405+
component_returned_object.length > 0
406+
? b.return(b.call('$.pop', b.object(component_returned_object)))
407+
: b.stmt(b.call('$.pop'))
408+
);
409+
}
400410

401411
if (analysis.uses_rest_props) {
402412
const named_props = analysis.exports.map(({ name, alias }) => alias ?? name);
@@ -430,11 +440,19 @@ export function client_component(source, analysis, options) {
430440
component_block.body.unshift(b.const('$$slots', b.call('$.sanitize_slots', b.id('$$props'))));
431441
}
432442

443+
let should_inject_props =
444+
should_inject_context ||
445+
analysis.needs_props ||
446+
analysis.uses_props ||
447+
analysis.uses_rest_props ||
448+
analysis.uses_slots ||
449+
analysis.slot_names.size > 0;
450+
433451
const body = [...state.hoisted, ...module.body];
434452

435453
const component = b.function_declaration(
436454
b.id(analysis.name),
437-
[b.id('$$anchor'), b.id('$$props')],
455+
should_inject_props ? [b.id('$$anchor'), b.id('$$props')] : [b.id('$$anchor')],
438456
component_block
439457
);
440458

packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1228,6 +1228,8 @@ function serialize_event_handler(node, { state, visit }) {
12281228

12291229
return handler;
12301230
} else {
1231+
state.analysis.needs_props = true;
1232+
12311233
// Function + .call to preserve "this" context as much as possible
12321234
return b.function(
12331235
null,

packages/svelte/src/compiler/phases/types.d.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,8 @@ export interface ComponentAnalysis extends Analysis {
5757
uses_slots: boolean;
5858
uses_component_bindings: boolean;
5959
uses_render_tags: boolean;
60+
needs_context: boolean;
61+
needs_props: boolean;
6062
custom_element: boolean | SvelteOptions['customElement'];
6163
/** If `true`, should append styles through JavaScript */
6264
inject_styles: boolean;

packages/svelte/tests/snapshot/samples/bind-component-snippet/_expected/client/index.svelte.js

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,7 @@ import TextInput from './Child.svelte';
55
var root_1 = $.template(`Something`, 1);
66
var root = $.template(`<!> `, 1);
77

8-
export default function Bind_component_snippet($$anchor, $$props) {
9-
$.push($$props, true);
10-
8+
export default function Bind_component_snippet($$anchor) {
119
let value = $.source('');
1210
const _snippet = snippet;
1311
var fragment_1 = root();
@@ -33,5 +31,4 @@ export default function Bind_component_snippet($$anchor, $$props) {
3331

3432
$.render_effect(() => $.set_text(text, ` value: ${$.stringify($.get(value))}`));
3533
$.append($$anchor, fragment_1);
36-
$.pop();
3734
}
Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,10 @@
11
import "svelte/internal/disclose-version";
22
import * as $ from "svelte/internal/client";
33

4-
export default function Bind_this($$anchor, $$props) {
5-
$.push($$props, false);
6-
$.init();
7-
4+
export default function Bind_this($$anchor) {
85
var fragment = $.comment();
96
var node = $.first_child(fragment);
107

118
$.bind_this(Foo(node, {}), ($$value) => foo = $$value, () => foo);
129
$.append($$anchor, fragment);
13-
$.pop();
1410
}

packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/_expected/client/main.svelte.js

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,7 @@ import * as $ from "svelte/internal/client";
33

44
var root = $.template(`<div></div> <svg></svg> <custom-element></custom-element> <div></div> <svg></svg> <custom-element></custom-element>`, 3);
55

6-
export default function Main($$anchor, $$props) {
7-
$.push($$props, true);
8-
6+
export default function Main($$anchor) {
97
// needs to be a snapshot test because jsdom does auto-correct the attribute casing
108
let x = 'test';
119
let y = () => 'test';
@@ -32,5 +30,4 @@ export default function Main($$anchor, $$props) {
3230
});
3331

3432
$.append($$anchor, fragment);
35-
$.pop();
3633
}
Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
11
import "svelte/internal/disclose-version";
22
import * as $ from "svelte/internal/client";
33

4-
export default function Each_string_template($$anchor, $$props) {
5-
$.push($$props, false);
6-
$.init();
7-
4+
export default function Each_string_template($$anchor) {
85
var fragment = $.comment();
96
var node = $.first_child(fragment);
107

@@ -16,5 +13,4 @@ export default function Each_string_template($$anchor, $$props) {
1613
});
1714

1815
$.append($$anchor, fragment);
19-
$.pop();
2016
}

packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/client/index.svelte.js

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
11
import "svelte/internal/disclose-version";
22
import * as $ from "svelte/internal/client";
33

4-
export default function Function_prop_no_getter($$anchor, $$props) {
5-
$.push($$props, true);
6-
4+
export default function Function_prop_no_getter($$anchor) {
75
let count = $.source(0);
86

97
function onmouseup() {
@@ -27,5 +25,4 @@ export default function Function_prop_no_getter($$anchor, $$props) {
2725
});
2826

2927
$.append($$anchor, fragment);
30-
$.pop();
3128
}

packages/svelte/tests/snapshot/samples/hello-world/_expected/client/index.svelte.js

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,8 @@ import * as $ from "svelte/internal/client";
33

44
var root = $.template(`<h1>hello world</h1>`);
55

6-
export default function Hello_world($$anchor, $$props) {
7-
$.push($$props, false);
8-
$.init();
9-
6+
export default function Hello_world($$anchor) {
107
var h1 = root();
118

129
$.append($$anchor, h1);
13-
$.pop();
1410
}

packages/svelte/tests/snapshot/samples/hmr/_expected/client/index.svelte.js

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,10 @@ import * as $ from "svelte/internal/client";
33

44
var root = $.template(`<h1>hello world</h1>`);
55

6-
function Hmr($$anchor, $$props) {
7-
$.push($$props, false);
8-
$.init();
9-
6+
function Hmr($$anchor) {
107
var h1 = root();
118

129
$.append($$anchor, h1);
13-
$.pop();
1410
}
1511

1612
if (import.meta.hot) {

packages/svelte/tests/snapshot/samples/state-proxy-literal/_expected/client/index.svelte.js

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,7 @@ function reset(_, str, tpl) {
1010

1111
var root = $.template(`<input> <input> <button>reset</button>`, 1);
1212

13-
export default function State_proxy_literal($$anchor, $$props) {
14-
$.push($$props, true);
15-
13+
export default function State_proxy_literal($$anchor) {
1614
let str = $.source('');
1715
let tpl = $.source(``);
1816
var fragment = root();
@@ -30,7 +28,6 @@ export default function State_proxy_literal($$anchor, $$props) {
3028
$.bind_value(input, () => $.get(str), ($$value) => $.set(str, $$value));
3129
$.bind_value(input_1, () => $.get(tpl), ($$value) => $.set(tpl, $$value));
3230
$.append($$anchor, fragment);
33-
$.pop();
3431
}
3532

3633
$.delegate(["click"]);

packages/svelte/tests/snapshot/samples/svelte-element/_expected/client/index.svelte.js

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,10 @@ import "svelte/internal/disclose-version";
22
import * as $ from "svelte/internal/client";
33

44
export default function Svelte_element($$anchor, $$props) {
5-
$.push($$props, true);
6-
75
let tag = $.prop($$props, "tag", 3, 'hr');
86
var fragment = $.comment();
97
var node = $.first_child(fragment);
108

119
$.element(node, tag, false);
1210
$.append($$anchor, fragment);
13-
$.pop();
1411
}

0 commit comments

Comments
 (0)