chore: remove event hoisting (#17030)

* chore: get rid of hoisted event handlers

* remove unused stuff

* simplify

* wow we can delete so much more code. this makes me so happy

* even more!
pull/17029/head
Rich Harris 4 weeks ago committed by GitHub
parent c08ecba1b7
commit 4eb432e941
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -1,12 +1,7 @@
/** @import { ArrowFunctionExpression, Expression, FunctionDeclaration, FunctionExpression } from 'estree' */
/** @import { AST, DelegatedEvent } from '#compiler' */
/** @import { AST } from '#compiler' */
/** @import { Context } from '../types' */
import { cannot_be_set_statically, is_capture_event, is_delegated } from '../../../../utils.js';
import {
get_attribute_chunks,
get_attribute_expression,
is_event_attribute
} from '../../../utils/ast.js';
import { cannot_be_set_statically, can_delegate_event } from '../../../../utils.js';
import { get_attribute_chunks, is_event_attribute } from '../../../utils/ast.js';
import { mark_subtree_dynamic } from './shared/fragment.js';
/**
@ -64,181 +59,8 @@ export function Attribute(node, context) {
context.state.analysis.uses_event_attributes = true;
}
const expression = get_attribute_expression(node);
const delegated_event = get_delegated_event(node.name.slice(2), expression, context);
if (delegated_event !== null) {
if (delegated_event.hoisted) {
delegated_event.function.metadata.hoisted = true;
}
node.metadata.delegated = delegated_event;
}
}
}
}
/** @type {DelegatedEvent} */
const unhoisted = { hoisted: false };
/**
* Checks if given event attribute can be delegated/hoisted and returns the corresponding info if so
* @param {string} event_name
* @param {Expression | null} handler
* @param {Context} context
* @returns {null | DelegatedEvent}
*/
function get_delegated_event(event_name, handler, context) {
// Handle delegated event handlers. Bail out if not a delegated event.
if (!handler || !is_delegated(event_name)) {
return null;
}
// If we are not working with a RegularElement, then bail out.
const element = context.path.at(-1);
if (element?.type !== 'RegularElement') {
return null;
}
/** @type {FunctionExpression | FunctionDeclaration | ArrowFunctionExpression | null} */
let target_function = null;
let binding = null;
if (element.metadata.has_spread) {
// event attribute becomes part of the dynamic spread array
return unhoisted;
}
if (handler.type === 'ArrowFunctionExpression' || handler.type === 'FunctionExpression') {
target_function = handler;
} else if (handler.type === 'Identifier') {
binding = context.state.scope.get(handler.name);
if (context.state.analysis.module.scope.references.has(handler.name)) {
// If a binding with the same name is referenced in the module scope (even if not declared there), bail out
return unhoisted;
}
if (binding != null) {
for (const { path } of binding.references) {
const parent = path.at(-1);
if (parent === undefined) return unhoisted;
const grandparent = path.at(-2);
/** @type {AST.RegularElement | null} */
let element = null;
/** @type {string | null} */
let event_name = null;
if (parent.type === 'OnDirective') {
element = /** @type {AST.RegularElement} */ (grandparent);
event_name = parent.name;
} else if (
parent.type === 'ExpressionTag' &&
grandparent?.type === 'Attribute' &&
is_event_attribute(grandparent)
) {
element = /** @type {AST.RegularElement} */ (path.at(-3));
const attribute = /** @type {AST.Attribute} */ (grandparent);
event_name = get_attribute_event_name(attribute.name);
}
if (element && event_name) {
if (
element.type !== 'RegularElement' ||
element.metadata.has_spread ||
!is_delegated(event_name)
) {
return unhoisted;
}
} else if (parent.type !== 'FunctionDeclaration' && parent.type !== 'VariableDeclarator') {
return unhoisted;
}
}
node.metadata.delegated =
parent?.type === 'RegularElement' && can_delegate_event(node.name.slice(2));
}
// If the binding is exported, bail out
if (context.state.analysis.exports.find((node) => node.name === handler.name)) {
return unhoisted;
}
if (binding?.is_function()) {
target_function = binding.initial;
}
}
// If we can't find a function, or the function has multiple parameters, bail out
if (target_function == null || target_function.params.length > 1) {
return unhoisted;
}
const visited_references = new Set();
const scope = target_function.metadata.scope;
for (const [reference] of scope.references) {
// Bail out if the arguments keyword is used or $host is referenced
if (reference === 'arguments' || reference === '$host') return unhoisted;
// Bail out if references a store subscription
if (scope.get(`$${reference}`)?.kind === 'store_sub') return unhoisted;
const binding = scope.get(reference);
const local_binding = context.state.scope.get(reference);
// if the function access a snippet that can't be hoisted we bail out
if (
local_binding !== null &&
local_binding.initial?.type === 'SnippetBlock' &&
!local_binding.initial.metadata.can_hoist
) {
return unhoisted;
}
// If we are referencing a binding that is shadowed in another scope then bail out (unless it's declared within the function).
if (
local_binding !== null &&
binding !== null &&
local_binding.node !== binding.node &&
scope.declarations.get(reference) !== binding
) {
return unhoisted;
}
// If we have multiple references to the same store using $ prefix, bail out.
if (
binding !== null &&
binding.kind === 'store_sub' &&
visited_references.has(reference.slice(1))
) {
return unhoisted;
}
// If we reference the index within an each block, then bail out.
if (binding !== null && binding.initial?.type === 'EachBlock') return unhoisted;
if (
binding !== null &&
// Bail out if the binding is a rest param
(binding.declaration_kind === 'rest_param' ||
// Bail out if we reference anything from the EachBlock (for now) that mutates in non-runes mode,
(((!context.state.analysis.runes && binding.kind === 'each') ||
// or any normal not reactive bindings that are mutated.
binding.kind === 'normal') &&
binding.updated))
) {
return unhoisted;
}
visited_references.add(reference);
}
return { hoisted: true, function: target_function };
}
/**
* @param {string} event_name
*/
function get_attribute_event_name(event_name) {
event_name = event_name.slice(2);
if (is_capture_event(event_name)) {
event_name = event_name.slice(0, -7);
}
return event_name;
}

@ -6,13 +6,6 @@
* @param {Context} context
*/
export function visit_function(node, context) {
// TODO retire this in favour of a more general solution based on bindings
node.metadata = {
hoisted: false,
hoisted_params: [],
scope: context.state.scope
};
if (context.state.expression) {
for (const [name] of context.state.scope.references) {
const binding = context.state.scope.get(name);

@ -1,6 +1,6 @@
/** @import { ArrowFunctionExpression, AssignmentExpression, BlockStatement, Expression, FunctionDeclaration, FunctionExpression, Identifier, Node, Pattern, UpdateExpression } from 'estree' */
/** @import { BlockStatement, Expression, Identifier } from 'estree' */
/** @import { Binding } from '#compiler' */
/** @import { ClientTransformState, ComponentClientTransformState, ComponentContext } from './types.js' */
/** @import { ClientTransformState, ComponentClientTransformState } from './types.js' */
/** @import { Analysis } from '../../types.js' */
/** @import { Scope } from '../../scope.js' */
import * as b from '#compiler/builders';
@ -12,9 +12,6 @@ import {
PROPS_IS_UPDATED,
PROPS_IS_BINDABLE
} from '../../../../constants.js';
import { dev } from '../../../state.js';
import { walk } from 'zimmerframe';
import { validate_mutation } from './visitors/shared/utils.js';
/**
* @param {Binding} binding
@ -46,125 +43,6 @@ export function build_getter(node, state) {
return node;
}
/**
* @param {FunctionDeclaration | FunctionExpression | ArrowFunctionExpression} node
* @param {ComponentContext} context
* @returns {Pattern[]}
*/
function get_hoisted_params(node, context) {
const scope = context.state.scope;
/** @type {Identifier[]} */
const params = [];
/**
* We only want to push if it's not already present to avoid name clashing
* @param {Identifier} id
*/
function push_unique(id) {
if (!params.find((param) => param.name === id.name)) {
params.push(id);
}
}
for (const [reference] of scope.references) {
let binding = scope.get(reference);
if (binding !== null && !scope.declarations.has(reference) && binding.initial !== node) {
if (binding.kind === 'store_sub') {
// We need both the subscription for getting the value and the store for updating
push_unique(b.id(binding.node.name));
binding = /** @type {Binding} */ (scope.get(binding.node.name.slice(1)));
}
let expression = context.state.transform[reference]?.read(b.id(binding.node.name));
if (
// If it's a destructured derived binding, then we can extract the derived signal reference and use that.
// TODO this code is bad, we need to kill it
expression != null &&
typeof expression !== 'function' &&
expression.type === 'MemberExpression' &&
expression.object.type === 'CallExpression' &&
expression.object.callee.type === 'Identifier' &&
expression.object.callee.name === '$.get' &&
expression.object.arguments[0].type === 'Identifier'
) {
push_unique(b.id(expression.object.arguments[0].name));
} else if (
// If we are referencing a simple $$props value, then we need to reference the object property instead
(binding.kind === 'prop' || binding.kind === 'bindable_prop') &&
!is_prop_source(binding, context.state)
) {
push_unique(b.id('$$props'));
} else if (
// imports don't need to be hoisted
binding.declaration_kind !== 'import'
) {
// create a copy to remove start/end tags which would mess up source maps
push_unique(b.id(binding.node.name));
// rest props are often accessed through the $$props object for optimization reasons,
// but we can't know if the delegated event handler will use it, so we need to add both as params
if (binding.kind === 'rest_prop' && context.state.analysis.runes) {
push_unique(b.id('$$props'));
}
}
}
}
if (dev) {
// this is a little hacky, but necessary for ownership validation
// to work inside hoisted event handlers
/**
* @param {AssignmentExpression | UpdateExpression} node
* @param {{ next: () => void, stop: () => void }} context
*/
function visit(node, { next, stop }) {
if (validate_mutation(node, /** @type {any} */ (context), node) !== node) {
params.push(b.id('$$ownership_validator'));
stop();
} else {
next();
}
}
walk(/** @type {Node} */ (node), null, {
AssignmentExpression: visit,
UpdateExpression: visit
});
}
return params;
}
/**
* @param {FunctionDeclaration | FunctionExpression | ArrowFunctionExpression} node
* @param {ComponentContext} context
* @returns {Pattern[]}
*/
export function build_hoisted_params(node, context) {
const hoisted_params = get_hoisted_params(node, context);
node.metadata.hoisted_params = hoisted_params;
/** @type {Pattern[]} */
const params = [];
if (node.params.length === 0) {
if (hoisted_params.length > 0) {
// For the event object
params.push(b.id(context.state.scope.generate('_')));
}
} else {
for (const param of node.params) {
params.push(/** @type {Pattern} */ (context.visit(param)));
}
}
params.push(...hoisted_params);
return params;
}
/**
* @param {Binding} binding
* @param {ComponentClientTransformState} state

@ -1,7 +1,5 @@
/** @import { FunctionDeclaration } from 'estree' */
/** @import { ComponentContext } from '../types' */
import { build_hoisted_params } from '../utils.js';
import * as b from '#compiler/builders';
/**
* @param {FunctionDeclaration} node
@ -10,14 +8,5 @@ import * as b from '#compiler/builders';
export function FunctionDeclaration(node, context) {
const state = { ...context.state, in_constructor: false, in_derived: false };
if (node.metadata?.hoisted === true) {
const params = build_hoisted_params(node, context);
const body = context.visit(node.body, state);
context.state.hoisted.push(/** @type {FunctionDeclaration} */ ({ ...node, params, body }));
return b.empty;
}
context.next(state);
}

@ -7,7 +7,6 @@ import * as b from '#compiler/builders';
import * as assert from '../../../../utils/assert.js';
import { get_rune } from '../../../scope.js';
import { get_prop_source, is_prop_source, is_state_source, should_proxy } from '../utils.js';
import { is_hoisted_function } from '../../utils.js';
import { get_value } from './shared/declarations.js';
/**
@ -32,13 +31,6 @@ export function VariableDeclaration(node, context) {
rune === '$state.snapshot' ||
rune === '$host'
) {
if (init != null && is_hoisted_function(init)) {
context.state.hoisted.push(
b.const(declarator.id, /** @type {Expression} */ (context.visit(init)))
);
continue;
}
declarations.push(/** @type {VariableDeclarator} */ (context.visit(declarator)));
continue;
}
@ -295,16 +287,6 @@ export function VariableDeclaration(node, context) {
const has_props = bindings.some((binding) => binding.kind === 'bindable_prop');
if (!has_state && !has_props) {
const init = declarator.init;
if (init != null && is_hoisted_function(init)) {
context.state.hoisted.push(
b.const(declarator.id, /** @type {Expression} */ (context.visit(init)))
);
continue;
}
declarations.push(/** @type {VariableDeclarator} */ (context.visit(declarator)));
continue;
}

@ -26,40 +26,12 @@ export function visit_event_attribute(node, context) {
let handler = build_event_handler(tag.expression, tag.metadata.expression, context);
if (node.metadata.delegated) {
let delegated_assignment;
if (!context.state.events.has(event_name)) {
context.state.events.add(event_name);
}
// Hoist function if we can, otherwise we leave the function as is
if (node.metadata.delegated.hoisted) {
if (node.metadata.delegated.function === tag.expression) {
const func_name = context.state.scope.root.unique('on_' + event_name);
context.state.hoisted.push(b.var(func_name, handler));
handler = func_name;
}
const hoisted_params = /** @type {Expression[]} */ (
node.metadata.delegated.function.metadata.hoisted_params
);
// When we hoist a function we assign an array with the function and all
// hoisted closure params.
if (hoisted_params) {
const args = [handler, ...hoisted_params];
delegated_assignment = b.array(args);
} else {
delegated_assignment = handler;
}
} else {
delegated_assignment = handler;
}
context.state.init.push(
b.stmt(
b.assignment('=', b.member(context.state.node, '__' + event_name), delegated_assignment)
)
b.stmt(b.assignment('=', b.member(context.state.node, '__' + event_name), handler))
);
} else {
const statement = b.stmt(

@ -1,14 +1,11 @@
/** @import { ArrowFunctionExpression, FunctionExpression, Node } from 'estree' */
/** @import { ComponentContext } from '../../types' */
import { build_hoisted_params } from '../../utils.js';
/**
* @param {ArrowFunctionExpression | FunctionExpression} node
* @param {ComponentContext} context
*/
export const visit_function = (node, context) => {
const metadata = node.metadata;
let state = { ...context.state, in_constructor: false, in_derived: false };
if (node.type === 'FunctionExpression') {
@ -16,15 +13,5 @@ export const visit_function = (node, context) => {
state.in_constructor = parent.type === 'MethodDefinition' && parent.kind === 'constructor';
}
if (metadata?.hoisted === true) {
const params = build_hoisted_params(node, context);
return /** @type {FunctionExpression} */ ({
...node,
params,
body: context.visit(node.body, state)
});
}
context.next(state);
};

@ -1,36 +1,17 @@
/** @import { Context } from 'zimmerframe' */
/** @import { TransformState } from './types.js' */
/** @import { AST, Binding, Namespace, ValidatedCompileOptions } from '#compiler' */
/** @import { Node, Expression, CallExpression, MemberExpression } from 'estree' */
import {
regex_ends_with_whitespaces,
regex_not_whitespace,
regex_starts_with_newline,
regex_starts_with_whitespaces
} from '../patterns.js';
import * as b from '#compiler/builders';
import * as e from '../../errors.js';
import { walk } from 'zimmerframe';
import { extract_identifiers } from '../../utils/ast.js';
import check_graph_for_cycles from '../2-analyze/utils/check_graph_for_cycles.js';
import is_reference from 'is-reference';
import { set_scope } from '../scope.js';
import { dev } from '../../state.js';
/**
* @param {Node} node
* @returns {boolean}
*/
export function is_hoisted_function(node) {
if (
node.type === 'ArrowFunctionExpression' ||
node.type === 'FunctionExpression' ||
node.type === 'FunctionDeclaration'
) {
return node.metadata?.hoisted === true;
}
return false;
}
/**
* Match Svelte 4 behaviour by sorting ConstTag nodes in topological order

@ -59,7 +59,7 @@ export function create_attribute(name, start, end, value) {
name,
value,
metadata: {
delegated: null,
delegated: false,
needs_clsx: false
}
};

@ -109,29 +109,3 @@ export interface ComponentAnalysis extends Analysis {
*/
snippets: Set<AST.SnippetBlock>;
}
declare module 'estree' {
interface ArrowFunctionExpression {
metadata: {
hoisted: boolean;
hoisted_params: Pattern[];
scope: Scope;
};
}
interface FunctionExpression {
metadata: {
hoisted: boolean;
hoisted_params: Pattern[];
scope: Scope;
};
}
interface FunctionDeclaration {
metadata: {
hoisted: boolean;
hoisted_params: Pattern[];
scope: Scope;
};
}
}

@ -5,8 +5,6 @@ import type {
VariableDeclaration,
VariableDeclarator,
Expression,
FunctionDeclaration,
FunctionExpression,
Identifier,
MemberExpression,
Node,
@ -27,13 +25,6 @@ import type { _CSS } from './css';
*/
export type Namespace = 'html' | 'svg' | 'mathml';
export type DelegatedEvent =
| {
hoisted: true;
function: ArrowFunctionExpression | FunctionExpression | FunctionDeclaration;
}
| { hoisted: false };
export namespace AST {
export interface BaseNode {
type: string;
@ -531,7 +522,7 @@ export namespace AST {
/** @internal */
metadata: {
/** May be set if this is an event attribute */
delegated: null | DelegatedEvent;
delegated: boolean;
/** May be `true` if this is a `class` attribute that needs `clsx` */
needs_clsx: boolean;
};

@ -42,8 +42,7 @@ export function arrow(params, body, async = false) {
body,
expression: body.type !== 'BlockStatement',
generator: false,
async,
metadata: /** @type {any} */ (null) // should not be used by codegen
async
};
}
@ -237,8 +236,7 @@ export function function_declaration(id, params, body, async = false) {
params,
body,
generator: false,
async,
metadata: /** @type {any} */ (null) // should not be used by codegen
async
};
}
@ -595,8 +593,7 @@ function function_builder(id, params, body, async = false) {
params,
body,
generator: false,
async,
metadata: /** @type {any} */ (null) // should not be used by codegen
async
};
}

@ -7,7 +7,7 @@ import { add_form_reset_listener, autofocus } from './misc.js';
import * as w from '../../warnings.js';
import { LOADING_ATTR_SYMBOL } from '#client/constants';
import { queue_micro_task } from '../task.js';
import { is_capture_event, is_delegated, normalize_attribute } from '../../../../utils.js';
import { is_capture_event, can_delegate_event, normalize_attribute } from '../../../../utils.js';
import {
active_effect,
active_reaction,
@ -378,7 +378,7 @@ function set_attributes(
const opts = {};
const event_handle_key = '$$' + key;
let event_name = key.slice(2);
var delegated = is_delegated(event_name);
var delegated = can_delegate_event(event_name);
if (is_capture_event(event_name)) {
event_name = event_name.slice(0, -7);

@ -1,5 +1,5 @@
import { teardown } from '../../reactivity/effects.js';
import { define_property, is_array } from '../../../shared/utils.js';
import { define_property } from '../../../shared/utils.js';
import { hydrating } from '../hydration.js';
import { queue_micro_task } from '../task.js';
import { FILENAME } from '../../../../constants.js';
@ -258,12 +258,7 @@ export function handle_event_propagation(event) {
// -> the target could not have been disabled because it emits the event in the first place
event.target === current_target)
) {
if (is_array(delegated)) {
var [fn, ...data] = delegated;
fn.apply(current_target, [event, ...data]);
} else {
delegated.call(current_target, event);
}
delegated.call(current_target, event);
}
} catch (error) {
if (throw_error) {

@ -137,7 +137,7 @@ const DELEGATED_EVENTS = [
* Returns `true` if `event_name` is a delegated event
* @param {string} event_name
*/
export function is_delegated(event_name) {
export function can_delegate_event(event_name) {
return DELEGATED_EVENTS.includes(event_name);
}

@ -15,9 +15,9 @@ export default test({
{},
[],
{ x: 'hello' },
'at HTMLButtonElement.on_click',
'at HTMLButtonElement.Main.button.__click',
['hello'],
'at HTMLButtonElement.on_click'
'at HTMLButtonElement.Main.button.__click'
]);
}
});

@ -15,9 +15,9 @@ export default test({
assert.deepEqual(normalise_inspect_logs(logs), [
[],
[{}],
'at HTMLButtonElement.on_click',
'at HTMLButtonElement.Main.button.__click',
[{}, {}],
'at HTMLButtonElement.on_click'
'at HTMLButtonElement.Main.button.__click'
]);
}
});

@ -1,19 +1,20 @@
import 'svelte/internal/disclose-version';
import * as $ from 'svelte/internal/client';
function increment(_, counter) {
counter.count += 1;
}
var root = $.from_html(`<button> </button> <!> `, 1);
export default function Await_block_scope($$anchor) {
let counter = $.proxy({ count: 0 });
const promise = $.derived(() => Promise.resolve(counter));
function increment() {
counter.count += 1;
}
var fragment = root();
var button = $.first_child(fragment);
button.__click = [increment, counter];
button.__click = increment;
var text = $.child(button);

@ -2,12 +2,6 @@ import 'svelte/internal/disclose-version';
import 'svelte/internal/flags/legacy';
import * as $ from 'svelte/internal/client';
var on_click = (e) => {
const index = Number(e.currentTarget.dataset.index);
console.log(index);
};
var root_1 = $.from_html(`<button type="button">B</button>`);
export default function Delegated_locally_declared_shadowed($$anchor) {
@ -18,7 +12,13 @@ export default function Delegated_locally_declared_shadowed($$anchor) {
var button = root_1();
$.set_attribute(button, 'data-index', index);
button.__click = [on_click];
button.__click = (e) => {
const index = Number(e.currentTarget.dataset.index);
console.log(index);
};
$.append($$anchor, button);
});

@ -1,7 +1,6 @@
import 'svelte/internal/disclose-version';
import * as $ from 'svelte/internal/client';
var on_click = (_, count) => $.update(count);
var root = $.from_html(`<h1></h1> <b></b> <button> </button> <h1></h1>`, 1);
export default function Nullish_coallescence_omittance($$anchor) {
@ -18,7 +17,7 @@ export default function Nullish_coallescence_omittance($$anchor) {
var button = $.sibling(b, 2);
button.__click = [on_click, count];
button.__click = () => $.update(count);
var text = $.child(button);

@ -1,18 +1,19 @@
import 'svelte/internal/disclose-version';
import * as $ from 'svelte/internal/client';
function reset(_, str, tpl) {
$.set(str, '');
$.set(str, ``);
$.set(tpl, '');
$.set(tpl, ``);
}
var root = $.from_html(`<input/> <input/> <button>reset</button>`, 1);
export default function State_proxy_literal($$anchor) {
let str = $.state('');
let tpl = $.state(``);
function reset() {
$.set(str, '');
$.set(str, ``);
$.set(tpl, '');
$.set(tpl, ``);
}
var fragment = root();
var input = $.first_child(fragment);
@ -24,7 +25,7 @@ export default function State_proxy_literal($$anchor) {
var button = $.sibling(input_1, 2);
button.__click = [reset, str, tpl];
button.__click = reset;
$.bind_value(input, () => $.get(str), ($$value) => $.set(str, $$value));
$.bind_value(input_1, () => $.get(tpl), ($$value) => $.set(tpl, $$value));
$.append($$anchor, fragment);

Loading…
Cancel
Save