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.

Sign up for GitHub

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: inline default imports into template #13242

Closed
wants to merge 1 commit into from
Closed

perf: inline default imports into template #13242

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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/seven-shrimps-explode.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

perf: inline default imports into template
7 changes: 2 additions & 5 deletions packages/svelte/src/compiler/phases/3-transform/client/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,15 +314,12 @@ export function create_derived(state, arg) {

/**
* Whether a variable can be referenced directly from template string.
* @param {import('#compiler').Binding | undefined} binding
* @param {import('#compiler').Binding} binding
* @returns {boolean}
*/
export function can_inline_variable(binding) {
return (
!!binding &&
// in a `<script module>` block
!binding.scope.parent &&
// to prevent the need for escaping
binding.initial?.type === 'Literal'
!binding.scope.parent
);
}
10 changes: 7 additions & 3 deletions packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -631,7 +631,9 @@ function build_element_attribute_update_assignment(element, node_id, attribute,
} else {
if (inlinable_expression) {
context.state.template.push_quasi(` ${name}="`);
context.state.template.push_expression(value);
context.state.template.push_expression(
inlinable_expression.need_to_escape ? b.call('$.escape', value) : value
);
context.state.template.push_quasi('"');
} else {
state.init.push(update);
Expand All @@ -646,22 +648,24 @@ function build_element_attribute_update_assignment(element, node_id, attribute,
*/
function is_inlinable_expression(nodes, state) {
let has_expression_tag = false;
let need_to_escape = false;
for (let value of nodes) {
if (value.type === 'ExpressionTag') {
if (value.expression.type === 'Identifier') {
const binding = state.scope
.owner(value.expression.name)
?.declarations.get(value.expression.name);
if (!can_inline_variable(binding)) {
if (!binding || !can_inline_variable(binding)) {
return false;
}
need_to_escape = need_to_escape || binding.initial?.type !== 'Literal';
} else {
return false;
}
has_expression_tag = true;
}
}
return has_expression_tag;
return { need_to_escape };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really not a fan of a function returning false | { need_to_escape: boolean } — it's a messy and confusing type. If a function name begins with is_ then it ought to return a boolean; if it needs to return more granular information than that then something like analyze_expression would probably be better

}

/**
Expand Down
1 change: 1 addition & 0 deletions packages/svelte/src/compiler/utils/sanitize_template_string.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/**
* Escape special characters like ` $ { \
* @param {string} str
* @returns {string}
*/
Expand Down
18 changes: 13 additions & 5 deletions packages/svelte/tests/snapshot/samples/inline-module-vars/_expected/client/index.svelte.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,27 @@
import "svelte/internal/disclose-version";
import * as $ from "svelte/internal/client";
import __IMPORTED_ASSET_0__ from "./foo.svg";
import { counter } from "./some.js";

const __DECLARED_ASSET_0__ = "__VITE_ASSET__2AM7_y_a__ 1440w, __VITE_ASSET__2AM7_y_b__ 960w";
const __DECLARED_ASSET_1__ = "__VITE_ASSET__2AM7_y_c__ 1440w, __VITE_ASSET__2AM7_y_d__ 960w";
const __DECLARED_ASSET_2__ = "__VITE_ASSET__2AM7_y_e__ 1440w, __VITE_ASSET__2AM7_y_f__ 960w";
const __DECLARED_ASSET_3__ = "__VITE_ASSET__2AM7_y_g__";
var root = $.template(`<picture><source srcset="${__DECLARED_ASSET_0__}" type="image/avif"> <source srcset="${__DECLARED_ASSET_1__}" type="image/webp"> <source srcset="${__DECLARED_ASSET_2__}" type="image/png"> <img src="${__DECLARED_ASSET_3__}" alt="production test" width="1440" height="1440"></picture>`);
var root = $.template(`<div></div> <img src="${$.escape(__IMPORTED_ASSET_0__)}" alt="default imports are not live bindings so can be inlined"> <picture><source srcset="${__DECLARED_ASSET_0__}" type="image/avif"> <source srcset="${__DECLARED_ASSET_1__}" type="image/webp"> <source srcset="${__DECLARED_ASSET_2__}" type="image/png"> <img src="${__DECLARED_ASSET_3__}" alt="production test" width="1440" height="1440"></picture>`, 1);

export default function Inline_module_vars($$anchor) {
var picture = root();
var fragment = root();
var div = $.first_child(fragment);

div.textContent = `${counter ?? ""} named exports are live bindings so cannot be inlined`;

var img = $.sibling(div, 2);
var picture = $.sibling(img, 2);
var source = $.child(picture);
var source_1 = $.sibling(source, 2);
var source_2 = $.sibling(source_1, 2);
var img = $.sibling(source_2, 2);
var img_1 = $.sibling(source_2, 2);

$.reset(picture);
$.append($$anchor, picture);
}
$.append($$anchor, fragment);
}
4 changes: 3 additions & 1 deletion packages/svelte/tests/snapshot/samples/inline-module-vars/_expected/server/index.svelte.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import * as $ from "svelte/internal/server";
import __IMPORTED_ASSET_0__ from "./foo.svg";
import { counter } from "./some.js";

const __DECLARED_ASSET_0__ = "__VITE_ASSET__2AM7_y_a__ 1440w, __VITE_ASSET__2AM7_y_b__ 960w";
const __DECLARED_ASSET_1__ = "__VITE_ASSET__2AM7_y_c__ 1440w, __VITE_ASSET__2AM7_y_d__ 960w";
const __DECLARED_ASSET_2__ = "__VITE_ASSET__2AM7_y_e__ 1440w, __VITE_ASSET__2AM7_y_f__ 960w";
const __DECLARED_ASSET_3__ = "__VITE_ASSET__2AM7_y_g__";

export default function Inline_module_vars($$payload) {
$$payload.out += `<picture><source${$.attr("srcset", __DECLARED_ASSET_0__)} type="image/avif"> <source${$.attr("srcset", __DECLARED_ASSET_1__)} type="image/webp"> <source${$.attr("srcset", __DECLARED_ASSET_2__)} type="image/png"> <img${$.attr("src", __DECLARED_ASSET_3__)} alt="production test" width="1440" height="1440"></picture>`;
$$payload.out += `<div>${$.escape(counter)} named exports are live bindings so cannot be inlined</div> <img${$.attr("src", __IMPORTED_ASSET_0__)} alt="default imports are not live bindings so can be inlined"> <picture><source${$.attr("srcset", __DECLARED_ASSET_0__)} type="image/avif"> <source${$.attr("srcset", __DECLARED_ASSET_1__)} type="image/webp"> <source${$.attr("srcset", __DECLARED_ASSET_2__)} type="image/png"> <img${$.attr("src", __DECLARED_ASSET_3__)} alt="production test" width="1440" height="1440"></picture>`;
}
6 changes: 6 additions & 0 deletions packages/svelte/tests/snapshot/samples/inline-module-vars/index.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,18 @@
<svelte:options runes={true} />

<script module>
import __IMPORTED_ASSET_0__ from "./foo.svg";
import { counter } from "./some.js";
const __DECLARED_ASSET_0__ = "__VITE_ASSET__2AM7_y_a__ 1440w, __VITE_ASSET__2AM7_y_b__ 960w";
const __DECLARED_ASSET_1__ = "__VITE_ASSET__2AM7_y_c__ 1440w, __VITE_ASSET__2AM7_y_d__ 960w";
const __DECLARED_ASSET_2__ = "__VITE_ASSET__2AM7_y_e__ 1440w, __VITE_ASSET__2AM7_y_f__ 960w";
const __DECLARED_ASSET_3__ = "__VITE_ASSET__2AM7_y_g__";
</script>

<div>{counter} named exports are live bindings so cannot be inlined</div>

<img src={__IMPORTED_ASSET_0__} alt="default imports are not live bindings so can be inlined" />

<picture>
<source srcset={__DECLARED_ASSET_0__} type="image/avif" />
<source srcset={__DECLARED_ASSET_1__} type="image/webp" />
Expand Down