Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: use unchanged primitives directly in template attributes #10015

Closed
wants to merge 31 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
e0af98b
perf: hoist variables which are not mutated or reassigned
benmccann Dec 27, 2023
57f1977
inline into template attributes
benmccann Dec 28, 2023
a01d4f8
cleanup
benmccann Dec 28, 2023
ba87d70
shorter changeset
benmccann Dec 28, 2023
dfe7df0
update get template
benmccann Dec 28, 2023
0bd0c4e
fix
benmccann Dec 28, 2023
b090038
fixes
benmccann Dec 28, 2023
f5151b8
remove extra blank line
benmccann Dec 28, 2023
0cddd53
remove accidental code
benmccann Dec 28, 2023
c53ae0c
remove unused imports
benmccann Dec 28, 2023
2169608
fix
benmccann Dec 29, 2023
eec1110
add todo
benmccann Dec 29, 2023
fcb076b
inline variables in module scope
benmccann Dec 29, 2023
098462d
expand test to set stage for inlining text nodes
benmccann Dec 29, 2023
0c3043c
remove extra character
benmccann Dec 30, 2023
d36f460
test case and fix
benmccann Dec 31, 2023
6ffb21b
mostly implement inlining for template text nodes
benmccann Jan 2, 2024
3e1f4c7
Merge branch 'main' into hoist-unmodified-var
benmccann Jan 2, 2024
02789df
regnerate types
benmccann Jan 2, 2024
8e17fa7
merge main
benmccann Jan 3, 2024
f43965e
add details
benmccann Jan 5, 2024
d224156
clarify test
benmccann Jan 6, 2024
9429dc8
implement inlining for template text nodes
benmccann Jan 6, 2024
c0c653b
custom elements
benmccann Jan 6, 2024
acb4455
cleanup
benmccann Jan 6, 2024
bc62554
notes
benmccann Jan 6, 2024
27cfdde
handle context=module variables
benmccann Jan 7, 2024
0398a0d
format
benmccann Jan 7, 2024
623c7c7
tweak comment
benmccann Jan 7, 2024
8cc7422
merge main
benmccann Jan 8, 2024
e7e1454
format
benmccann Jan 8, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/twelve-peaches-jam.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

perf: use unchanged primitives directly in template attributes
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,18 @@ export function client_component(source, analysis, options) {
return a;
},
get template() {
/** @type {any[]} */
const a = [];
a.push = () =>
error(null, 'INTERNAL', 'template.push should not be called outside create_block');
const a = {
quasi: [],
expressions: []
};
a.quasi.push = () =>
error(null, 'INTERNAL', 'template.quasi.push should not be called outside create_block');
a.expressions.push = () =>
error(
null,
'INTERNAL',
'template.expressions.push should not be called outside create_block'
);
return a;
},
legacy_reactive_statements: new Map(),
Expand Down Expand Up @@ -336,8 +344,8 @@ export function client_component(source, analysis, options) {
}

const body = [
...state.hoisted,
...module.body,
...state.hoisted,
b.export_default(
b.function_declaration(
b.id(analysis.name),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ import type {
Statement,
LabeledStatement,
Identifier,
PrivateIdentifier
PrivateIdentifier,
Expression
} from 'estree';
import type { Namespace, SvelteNode, ValidatedCompileOptions } from '#compiler';
import type { TransformState } from '../types.js';
Expand Down Expand Up @@ -45,7 +46,10 @@ export interface ComponentClientTransformState extends ClientTransformState {
/** Stuff that happens after the render effect (bindings, actions) */
readonly after_update: Statement[];
/** The HTML template string */
readonly template: string[];
readonly template: {
quasi: string[];
expressions: Expression[];
};
readonly metadata: {
namespace: Namespace;
/** `true` if the HTML template needs to be instantiated with `importNode` */
Expand Down
37 changes: 37 additions & 0 deletions packages/svelte/src/compiler/phases/3-transform/client/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
PROPS_IS_RUNES,
PROPS_IS_UPDATED
} from '../../../../constants.js';
import { GlobalBindings } from '../../constants.js';

/**
* @template {import('./types').ClientTransformState} State
Expand Down Expand Up @@ -518,3 +519,39 @@ export function should_proxy_or_freeze(node) {

return true;
}

/**
* Whether a variable can be referenced directly from template string.
* @param {import('#compiler').Binding | undefined} binding
* @param {string} name
* @returns {boolean}
*/
export function can_inline_variable(binding, name) {
return can_hoist_declaration(binding, name) || (!!binding && !binding.scope.has_parent());
}

/**
* @param {import('#compiler').Binding | undefined} binding
* @param {string} name
* @returns {boolean}
*/
export function can_hoist_declaration(binding, name) {
// We cannot hoist functions or constructors because they could be non-deterministic
// (i.e. `Math.random()`) or have side-effects (e.g. `increment()`). We also cannot hoist
// anything that references a non-hoistable variable.
return (
!!binding &&
binding.kind === 'normal' &&
binding.scope.is_top_level &&
binding.scope.has_parent() && // i.e. not when context="module"
// For now we just allow primitives for simplicity
binding.initial?.type === 'Literal' &&
// For now, we just check that it's not mutated or reassigned for simplicity
// E.g. if you have `let x = 0; x++` you could hoist both statements
!binding.mutated &&
!binding.reassigned &&
// Avoid conflicts. It would be nice to rename the variable, but keeping it simple for now
!binding.scope.declared_in_outer_scope(name) &&
!GlobalBindings.has(name)
);
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { is_hoistable_function } from '../../utils.js';
import * as b from '../../../../utils/builders.js';
import { extract_paths } from '../../../../utils/ast.js';
import { get_prop_source, serialize_get_binding } from '../utils.js';
import { can_hoist_declaration, get_prop_source, serialize_get_binding } from '../utils.js';

/**
* Creates the output for a state declaration.
Expand Down Expand Up @@ -44,6 +44,18 @@ export const javascript_visitors_legacy = {

if (!has_state && !has_props) {
const init = declarator.init;

if (init != null && declarator.id.type === 'Identifier') {
const binding = state.scope
.owner(declarator.id.name)
?.declarations.get(declarator.id.name);

if (can_hoist_declaration(binding, declarator.id.name)) {
state.hoisted.push(b.declaration('const', declarator.id, init));
continue;
}
}

if (init != null && is_hoistable_function(init)) {
const hoistable_function = visit(init);
state.hoisted.push(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,12 @@ import { get_rune } from '../../../scope.js';
import { is_hoistable_function, transform_inspect_rune } from '../../utils.js';
import * as b from '../../../../utils/builders.js';
import * as assert from '../../../../utils/assert.js';
import { get_prop_source, is_state_source, should_proxy_or_freeze } from '../utils.js';
import {
can_hoist_declaration,
get_prop_source,
is_state_source,
should_proxy_or_freeze
} from '../utils.js';
import { extract_paths, unwrap_ts_expression } from '../../../../utils/ast.js';

/** @type {import('../types.js').ComponentVisitors} */
Expand Down Expand Up @@ -156,6 +161,15 @@ export const javascript_visitors_runes = {

for (const declarator of node.declarations) {
const init = unwrap_ts_expression(declarator.init);

if (init != null && declarator.id.type === 'Identifier') {
const binding = state.scope.owner(declarator.id.name)?.declarations.get(declarator.id.name);
if (can_hoist_declaration(binding, declarator.id.name)) {
state.hoisted.push(b.declaration('const', declarator.id, init));
continue;
}
}

const rune = get_rune(init, state.scope);
if (!rune || rune === '$effect.active' || rune === '$effect.root' || rune === '$inspect') {
if (init != null && is_hoistable_function(init)) {
Expand Down
Loading