Skip to content

chore: remove event hoisting #17030

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

Merged
merged 5 commits into from
Oct 24, 2025
Merged

chore: remove event hoisting #17030

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
945a188
chore: get rid of hoisted event handlers
Rich-Harris Oct 24, 2025
7ce367e
remove unused stuff
Rich-Harris Oct 24, 2025
a1219a6
simplify
Rich-Harris Oct 24, 2025
c267597
wow we can delete so much more code. this makes me so happy
Rich-Harris Oct 24, 2025
279796a
even more!
Rich-Harris Oct 24, 2025
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
188 changes: 5 additions & 183 deletions packages/svelte/src/compiler/phases/2-analyze/visitors/Attribute.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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';

/**
Expand Down Expand Up @@ -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;
}
7 changes: 0 additions & 7 deletions packages/svelte/src/compiler/phases/2-analyze/visitors/shared/function.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
126 changes: 2 additions & 124 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
@@ -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';
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
11 changes: 0 additions & 11 deletions packages/svelte/src/compiler/phases/3-transform/client/visitors/FunctionDeclaration.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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);
}
Loading
Loading