Skip to content

feat: more efficient code generation when referencing globals #12712

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

Merged
merged 2 commits into from
Aug 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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/red-kings-draw.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

feat: more efficient code generation when referencing globals
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import { get_rune } from '../../scope.js';
import * as e from '../../../errors.js';
import { get_parent, unwrap_optional } from '../../../utils/ast.js';
import { is_known_safe_call, is_safe_identifier } from './shared/utils.js';
import { is_pure, is_safe_identifier } from './shared/utils.js';

/**
* @param {CallExpression} node
Expand Down Expand Up @@ -150,7 +150,7 @@ export function CallExpression(node, context) {
break;
}

if (context.state.expression && !is_known_safe_call(node.callee, context)) {
if (context.state.expression && !is_pure(node.callee, context)) {
context.state.expression.has_call = true;
context.state.expression.has_state = true;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/** @import { MemberExpression } from 'estree' */
/** @import { Context } from '../types' */
import * as e from '../../../errors.js';
import { is_safe_identifier } from './shared/utils.js';
import { is_pure, is_safe_identifier } from './shared/utils.js';

/**
* @param {MemberExpression} node
Expand All @@ -15,7 +15,7 @@ export function MemberExpression(node, context) {
}
}

if (context.state.expression) {
if (context.state.expression && !is_pure(node, context)) {
context.state.expression.has_state = true;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
/** @import { TaggedTemplateExpression, VariableDeclarator } from 'estree' */
/** @import { Context } from '../types' */
import { is_known_safe_call } from './shared/utils.js';
import { is_pure } from './shared/utils.js';

/**
* @param {TaggedTemplateExpression} node
* @param {Context} context
*/
export function TaggedTemplateExpression(node, context) {
if (context.state.expression && !is_known_safe_call(node.tag, context)) {
if (context.state.expression && !is_pure(node.tag, context)) {
context.state.expression.has_call = true;
context.state.expression.has_state = true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
/** @import { Scope } from '../../../scope' */
/** @import { NodeLike } from '../../../../errors.js' */
import * as e from '../../../../errors.js';
import { extract_identifiers } from '../../../../utils/ast.js';
import { extract_identifiers, object } from '../../../../utils/ast.js';
import * as w from '../../../../warnings.js';

/**
Expand Down Expand Up @@ -167,24 +167,23 @@ export function is_safe_identifier(expression, scope) {
}

/**
* @param {Expression | Super} callee
* @param {Expression | Super} node
* @param {Context} context
* @returns {boolean}
*/
export function is_known_safe_call(callee, context) {
// String / Number / BigInt / Boolean casting calls
if (callee.type === 'Identifier') {
const name = callee.name;
const binding = context.state.scope.get(name);
if (
binding === null &&
(name === 'BigInt' || name === 'String' || name === 'Number' || name === 'Boolean')
) {
return true;
}
export function is_pure(node, context) {
if (node.type !== 'Identifier' && node.type !== 'MemberExpression') {
return false;
}

// TODO add more cases
const left = object(node);
if (!left) return false;

if (left.type === 'Identifier') {
const binding = context.state.scope.get(left.name);
if (binding === null) return true; // globals are assumed to be safe
}

// TODO add more cases (safe Svelte imports, etc)
return false;
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<script>
import {untrack} from 'svelte';
import { untrack } from 'svelte';

const foo = $effect.tracking();
let bar = $state(false);
Expand All @@ -10,5 +10,5 @@

<p>{foo}</p>
<p>{bar}</p>
<p>{$effect.tracking()}</p>
<p>{untrack(() => $effect.tracking())}</p>
<p>{(bar, $effect.tracking())}</p>
<p>{untrack(() => (bar, $effect.tracking()))}</p>
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import "svelte/internal/disclose-version";
import * as $ from "svelte/internal/client";

var root = $.template(`<p> </p> <p> </p> <!>`, 1);

export default function Purity($$anchor) {
let min = 0;
let max = 100;
let number = 50;
let value = 'hello';
var fragment = root();
var p = $.first_child(fragment);
var text = $.child(p);

text.nodeValue = Math.max(min, Math.min(max, number));
$.reset(p);

var p_1 = $.sibling($.sibling(p, true));
var text_1 = $.child(p_1);

text_1.nodeValue = location.href;
$.reset(p_1);

var node = $.sibling($.sibling(p_1, true));

Child(node, {
prop: encodeURIComponent(value),
$$legacy: true
});

$.append($$anchor, fragment);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import * as $ from "svelte/internal/server";

export default function Purity($$payload) {
let min = 0;
let max = 100;
let number = 50;
let value = 'hello';

$$payload.out += `<p>${$.escape(Math.max(min, Math.min(max, number)))}</p> <p>${$.escape(location.href)}</p> `;
Child($$payload, { prop: encodeURIComponent(value) });
$$payload.out += `<!---->`;
}
12 changes: 12 additions & 0 deletions packages/svelte/tests/snapshot/samples/purity/index.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<script>
let min = 0;
let max = 100;
let number = 50;

let value = 'hello';
</script>

<p>{Math.max(min, Math.min(max, number))}</p>
<p>{location.href}</p>

<Child prop={encodeURIComponent(value)} />
Loading