chore: optimize inline new class expressions via hoisting the class

pull/12011/head
Dominic Gannaway 5 months ago
parent 7272b650d6
commit 3a219db578

@ -0,0 +1,5 @@
---
"svelte": patch
---
chore: optimize inline new class expressions via hoisting the class

@ -12,7 +12,7 @@
## perf_avoid_inline_class
> Avoid 'new class' — instead, declare the class at the top level scope
> Avoid complex 'new class' expressions that extend other classes or contain arguemtns within their constructor — instead, declare the class at the top level scope or simplify the `new class` expression
## perf_avoid_nested_class

@ -32,6 +32,7 @@ import {
import { Scope, get_rune } from '../scope.js';
import { merge } from '../visitors.js';
import { a11y_validators } from './a11y.js';
import { can_hoist_inline_class_expression } from '../3-transform/utils.js';
/** @param {import('#compiler').Attribute} attribute */
function validate_attribute(attribute) {
@ -329,6 +330,102 @@ function validate_block_not_empty(node, context) {
}
}
/**
* @type {import('zimmerframe').Visitors<import('#compiler').SvelteNode, import('./types.js').AnalysisState>}
*/
export const validation_runes_js = {
ImportDeclaration(node) {
if (typeof node.source.value === 'string' && node.source.value.startsWith('svelte/internal')) {
e.import_svelte_internal_forbidden(node);
}
},
ExportSpecifier(node, { state }) {
validate_export(node, state.scope, node.local.name);
},
ExportNamedDeclaration(node, { state, next }) {
if (node.declaration?.type !== 'VariableDeclaration') return;
// visit children, so bindings are correctly initialised
next();
for (const declarator of node.declaration.declarations) {
for (const id of extract_identifiers(declarator.id)) {
validate_export(node, state.scope, id.name);
}
}
},
CallExpression(node, { state, path }) {
if (get_rune(node, state.scope) === '$host') {
e.host_invalid_placement(node);
}
validate_call_expression(node, state.scope, path);
},
VariableDeclarator(node, { state }) {
const init = node.init;
const rune = get_rune(init, state.scope);
if (rune === null) return;
const args = /** @type {import('estree').CallExpression} */ (init).arguments;
if ((rune === '$derived' || rune === '$derived.by') && args.length !== 1) {
e.rune_invalid_arguments_length(node, rune, 'exactly one argument');
} else if (rune === '$state' && args.length > 1) {
e.rune_invalid_arguments_length(node, rune, 'zero or one arguments');
} else if (rune === '$props') {
e.props_invalid_placement(node);
} else if (rune === '$bindable') {
e.bindable_invalid_location(node);
}
},
AssignmentExpression(node, { state }) {
validate_assignment(node, node.left, state);
},
UpdateExpression(node, { state }) {
validate_assignment(node, node.argument, state);
},
ClassBody(node, context) {
/** @type {string[]} */
const private_derived_state = [];
for (const definition of node.body) {
if (
definition.type === 'PropertyDefinition' &&
definition.key.type === 'PrivateIdentifier' &&
definition.value?.type === 'CallExpression'
) {
const rune = get_rune(definition.value, context.state.scope);
if (rune === '$derived' || rune === '$derived.by') {
private_derived_state.push(definition.key.name);
}
}
}
context.next({
...context.state,
private_derived_state
});
},
ClassDeclaration(node, context) {
// In modules, we allow top-level module scope only, in components, we allow the component scope,
// which is function_depth of 1. With the exception of `new class` which is also not allowed at
// component scope level either.
const allowed_depth = context.state.ast_type === 'module' ? 0 : 1;
if (context.state.scope.function_depth > allowed_depth) {
w.perf_avoid_nested_class(node);
}
},
NewExpression(node, context) {
if (
node.callee.type === 'ClassExpression' &&
!can_hoist_inline_class_expression(node, context.state.ast_type, context.state.scope)
) {
w.perf_avoid_inline_class(node);
}
}
};
/**
* @type {import('zimmerframe').Visitors<import('#compiler').SvelteNode, import('./types.js').AnalysisState>}
*/
@ -807,7 +904,8 @@ export const validation_legacy = merge(validation, a11y_validators, {
},
UpdateExpression(node, { state }) {
validate_assignment(node, node.argument, state);
}
},
NewExpression: validation_runes_js.NewExpression
});
/**
@ -933,99 +1031,6 @@ function ensure_no_module_import_conflict(node, state) {
}
}
/**
* @type {import('zimmerframe').Visitors<import('#compiler').SvelteNode, import('./types.js').AnalysisState>}
*/
export const validation_runes_js = {
ImportDeclaration(node) {
if (typeof node.source.value === 'string' && node.source.value.startsWith('svelte/internal')) {
e.import_svelte_internal_forbidden(node);
}
},
ExportSpecifier(node, { state }) {
validate_export(node, state.scope, node.local.name);
},
ExportNamedDeclaration(node, { state, next }) {
if (node.declaration?.type !== 'VariableDeclaration') return;
// visit children, so bindings are correctly initialised
next();
for (const declarator of node.declaration.declarations) {
for (const id of extract_identifiers(declarator.id)) {
validate_export(node, state.scope, id.name);
}
}
},
CallExpression(node, { state, path }) {
if (get_rune(node, state.scope) === '$host') {
e.host_invalid_placement(node);
}
validate_call_expression(node, state.scope, path);
},
VariableDeclarator(node, { state }) {
const init = node.init;
const rune = get_rune(init, state.scope);
if (rune === null) return;
const args = /** @type {import('estree').CallExpression} */ (init).arguments;
if ((rune === '$derived' || rune === '$derived.by') && args.length !== 1) {
e.rune_invalid_arguments_length(node, rune, 'exactly one argument');
} else if (rune === '$state' && args.length > 1) {
e.rune_invalid_arguments_length(node, rune, 'zero or one arguments');
} else if (rune === '$props') {
e.props_invalid_placement(node);
} else if (rune === '$bindable') {
e.bindable_invalid_location(node);
}
},
AssignmentExpression(node, { state }) {
validate_assignment(node, node.left, state);
},
UpdateExpression(node, { state }) {
validate_assignment(node, node.argument, state);
},
ClassBody(node, context) {
/** @type {string[]} */
const private_derived_state = [];
for (const definition of node.body) {
if (
definition.type === 'PropertyDefinition' &&
definition.key.type === 'PrivateIdentifier' &&
definition.value?.type === 'CallExpression'
) {
const rune = get_rune(definition.value, context.state.scope);
if (rune === '$derived' || rune === '$derived.by') {
private_derived_state.push(definition.key.name);
}
}
}
context.next({
...context.state,
private_derived_state
});
},
ClassDeclaration(node, context) {
// In modules, we allow top-level module scope only, in components, we allow the component scope,
// which is function_depth of 1. With the exception of `new class` which is also not allowed at
// component scope level either.
const allowed_depth = context.state.ast_type === 'module' ? 0 : 1;
if (context.state.scope.function_depth > allowed_depth) {
w.perf_avoid_nested_class(node);
}
},
NewExpression(node, context) {
if (node.callee.type === 'ClassExpression' && context.state.scope.function_depth > 0) {
w.perf_avoid_inline_class(node);
}
}
};
/**
* @param {import('../../errors.js').NodeLike} node
* @param {import('estree').Pattern | import('estree').Expression} argument

@ -44,6 +44,7 @@ export function client_component(source, analysis, options) {
const state = {
analysis,
options,
ast_type: 'template',
scope: analysis.module.scope,
scopes: analysis.template.scopes,
hoisted: [b.import_all('$', 'svelte/internal/client')],
@ -543,8 +544,10 @@ export function client_module(analysis, options) {
const state = {
analysis,
options,
ast_type: 'module',
scope: analysis.module.scope,
scopes: analysis.module.scopes,
hoisted: [b.import_all('$', 'svelte/internal/client')],
legacy_reactive_statements: new Map(),
public_state: new Map(),
private_state: new Map(),
@ -568,6 +571,6 @@ export function client_module(analysis, options) {
return {
type: 'Program',
sourceType: 'module',
body: [b.import_all('$', 'svelte/internal/client'), ...module.body]
body: [...state.hoisted, ...module.body]
};
}

@ -8,11 +8,11 @@ import type {
import type { Namespace, SvelteNode, ValidatedCompileOptions } from '#compiler';
import type { TransformState } from '../types.js';
import type { ComponentAnalysis } from '../../types.js';
import type { Location } from 'locate-character';
export interface ClientTransformState extends TransformState {
readonly private_state: Map<string, StateField>;
readonly public_state: Map<string, StateField>;
readonly hoisted: Array<Statement | ModuleDeclaration>;
/**
* `true` if the current lexical scope belongs to a class constructor. this allows
@ -22,6 +22,8 @@ export interface ClientTransformState extends TransformState {
/** The $: calls, which will be ordered in the end */
readonly legacy_reactive_statements: Map<LabeledStatement, Statement>;
readonly ast_type: 'module' | 'template';
}
export type SourceLocation =

@ -1,5 +1,9 @@
import { get_rune } from '../../../scope.js';
import { is_hoistable_function, transform_inspect_rune } from '../../utils.js';
import { get_rune, set_scope } from '../../../scope.js';
import {
can_hoist_inline_class_expression,
is_hoistable_function,
transform_inspect_rune
} from '../../utils.js';
import * as b from '../../../../utils/builders.js';
import * as assert from '../../../../utils/assert.js';
import {
@ -10,6 +14,8 @@ import {
} from '../utils.js';
import { extract_paths } from '../../../../utils/ast.js';
import { regex_invalid_identifier_chars } from '../../../patterns.js';
import { walk } from 'zimmerframe';
import is_reference from 'is-reference';
/** @type {import('../types.js').ComponentVisitors} */
export const javascript_visitors_runes = {
@ -490,6 +496,139 @@ export const javascript_visitors_runes = {
}
next();
},
NewExpression(node, context) {
const state = context.state;
if (
node.callee.type === 'ClassExpression' &&
can_hoist_inline_class_expression(node, state.ast_type, state.scope)
) {
const id = node.callee.id?.name || state.scope.generate('hoisted_inline_class');
const { _ } = set_scope(state.scopes);
const closure_identifiers = new Map();
let class_expression = /** @type {import('estree').ClassExpression} */ (
context.visit(node.callee)
);
class_expression = /** @type {import('estree').ClassExpression} */ (
walk(class_expression, state, {
_,
Identifier(node, context) {
const parent = /** @type {import('estree').Expression} */ (context.path.at(-1));
if (
is_reference(node, parent) &&
!get_rune(parent, state.scope) &&
parent.type !== 'ClassExpression'
) {
const binding = context.state.scope.get(node.name);
if (binding !== null && binding.scope.function_depth !== 0) {
let closure_identifier = closure_identifiers.get(node.name);
if (closure_identifier === undefined) {
// We define the kind as being something that is either reactive, in which case we leave the binding
// alone and simply pass the identiifer. If it's mutated, we box it, otherwise we keep it as standard.
closure_identifier = {
id: state.scope.generate('closure_' + node.name),
kind: binding.kind !== 'normal' ? 'ref' : binding.mutated ? 'boxed' : 'normal'
};
closure_identifiers.set(node.name, closure_identifier);
}
const member = b.member(b.this, b.private_id(closure_identifier.id));
return closure_identifier.kind === 'boxed' ? b.member(member, b.id('v')) : member;
}
}
context.next();
}
})
);
const class_body = class_expression.body.body;
/** @type {Array<import('estree').PropertyDefinition | import('estree').MethodDefinition>} */
const body_entries = [...closure_identifiers.values()].map(({ id }) =>
b.prop_def(b.private_id(id), null)
);
const class_definitions = [];
for (let i = 0; i < class_body.length; i++) {
const entry = class_body[i];
if (entry.type === 'PropertyDefinition') {
if (entry.value) {
class_definitions.push(entry);
}
if (entry.key.type === 'PrivateIdentifier') {
class_body.splice(i, 1, b.prop_def(entry.key, null));
} else {
class_body.splice(i, 1);
i--;
}
}
}
let constructor = /** @type {import('estree').MethodDefinition | undefined} */ (
class_body.find((e) => e.type === 'MethodDefinition' && e.kind === 'constructor')
);
if (!constructor) {
constructor = b.method('constructor', b.id('constructor'), [], []);
body_entries.push(constructor);
}
constructor.value.params.push(...[...closure_identifiers.values()].map(({ id }) => b.id(id)));
const constructor_body = constructor.value.body.body;
const assignments = [...closure_identifiers.values()].map(({ id }) =>
b.stmt(b.assignment('=', b.member(b.this, b.private_id(id)), b.id(id)))
);
for (const entry of class_definitions) {
assignments.push(
b.stmt(
b.assignment(
'=',
b.member(b.this, entry.key),
/** @type {import('estree').Expression} */ (entry.value)
)
)
);
}
constructor_body.unshift(...assignments);
class_body.unshift(...body_entries);
context.state.hoisted.push(b.class(b.id(id), class_expression));
return {
...node,
arguments: [...closure_identifiers.entries()].map(([name, { kind }]) => {
if (kind === 'ref') {
return b.id(name);
}
const visited = /** @type {import('estree').Identifier} */ (context.visit(b.id(name)));
if (kind === 'boxed') {
return b.object([
b.prop('get', b.id('v'), b.function(null, [], b.block([b.return(visited)]))),
b.prop(
'set',
b.id('v'),
b.function(
null,
[b.id('v')],
b.block([b.stmt(b.assignment('=', b.id(name), b.id('v')))])
)
)
]);
}
return visited;
}),
callee: b.id(id)
};
}
context.next();
}
};

@ -408,3 +408,58 @@ export function transform_inspect_rune(node, context) {
return b.call('$.inspect', as_fn ? b.thunk(b.array(arg)) : b.array(arg));
}
}
/**
* @param {import("estree").NewExpression} node
* @param {string} ast_type
* @param {import("../scope.js").Scope} scope
*/
export function can_hoist_inline_class_expression(node, ast_type, scope) {
const allowed_depth = ast_type === 'module' ? 0 : 1;
if (node.callee.type !== 'ClassExpression') {
return false;
}
// If the new expression has arguments, skip hoisting it.
if (node.arguments.length > 0) {
return false;
}
const super_class = node.callee.superClass;
if (super_class) {
// We don't support hoisting a class that extends another class
return false;
}
// If the class is top level, do not bother inlining it.
if (scope.function_depth < allowed_depth) {
return false;
}
const body = node.callee.body.body;
for (const body_entry of body) {
if (body_entry.type === 'MethodDefinition' && body_entry.kind === 'constructor') {
const func = body_entry.value;
// If the constructor has arguments we can't hoist the class as we need to rewrite the arguments.
if (func.params.length > 0) {
return false;
}
let has_arguments_reference = false;
// If the constructor contains a reference to `arguments` we also can't hoist.
walk(func, null, {
// @ts-ignore
Identifier(node, context) {
const parent = /** @type {import('estree').Expression} */ (context.path.at(-1));
if (is_reference(node, parent) && node.name === 'arguments') {
has_arguments_reference = true;
}
}
});
if (has_arguments_reference) {
return false;
}
}
}
return true;
}

@ -508,6 +508,19 @@ function var_builder(pattern, init) {
return declaration('var', pattern, init);
}
/**
* @returns {import('estree').ClassDeclaration}
* @param {import("estree").Identifier} id
* @param {import("estree").ClassExpression} entries
*/
function class_builder(id, entries) {
return {
...entries,
id,
type: 'ClassDeclaration'
};
}
/**
*
* @param {import('estree').VariableDeclaration | import('estree').Expression | null} init
@ -624,6 +637,7 @@ export {
let_builder as let,
const_builder as const,
var_builder as var,
class_builder as class,
true_instance as true,
false_instance as false,
for_builder as for,

Loading…
Cancel
Save