Skip to content

Commit

Permalink
feat: allow ignoring runtime warnings (#12608)
Browse files Browse the repository at this point in the history 
* feat: allow ignoring binding_property_non_reactive

* chore: add comments before `to_ignore`

* chore: fix warnings regeneration

* chore: include client warnings code in svelte ignore extract

* feat: allow ignoring state_snapshot_uncloneable

* chore: abstract ignore into function

* feat: allow skipping of `hydration_attribute_changed`

* feat: allow skip of `hydration_html_changed`

* feat: allow skipping `ownership_invalid_binding`

* chore: revert extracting codes and use hardcoded list

* chore: update changeset

* feat: allow skipping `ownership_invalid_mutation`

* is_to_ignore -> is_ignored

* make is_ignored type safe

* tweak

* tweak naming

* tweak

* remove extra args

* comment is redundant, code contains enough information

* remove more unwanted args

* lint

---------

Co-authored-by: Rich Harris <[email protected]>
  • Loading branch information
@paoloricciuti @Rich-Harris
paoloricciuti and Rich-Harris authored Jul 31, 2024
1 parent 4b1b886 commit 64d2a2e
Show file tree
Hide file tree
Showing 31 changed files with 419 additions and 87 deletions.
5 changes: 5 additions & 0 deletions .changeset/large-emus-cough.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

feat: allow ignoring runtime warnings
70 changes: 48 additions & 22 deletions packages/svelte/src/compiler/phases/3-transform/client/utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {
PROPS_IS_RUNES,
PROPS_IS_UPDATED
} from '../../../../constants.js';
import { dev } from '../../../state.js';
import { is_ignored, dev } from '../../../state.js';

/**
* @template {ClientTransformState} State
Expand Down Expand Up @@ -282,6 +282,22 @@ export function serialize_set_binding(node, context, fallback, prefix, options)
);
}

/**
* @param {any} serialized
* @returns
*/
function maybe_skip_ownership_validation(serialized) {
if (is_ignored(node, 'ownership_invalid_mutation')) {
return b.call('$.skip_ownership_validation', b.thunk(serialized));
}

return serialized;
}

if (binding.kind === 'derived') {
return maybe_skip_ownership_validation(fallback());
}

const is_store = binding.kind === 'store_sub';
const left_name = is_store ? left.name.slice(1) : left.name;

Expand Down Expand Up @@ -382,45 +398,55 @@ export function serialize_set_binding(node, context, fallback, prefix, options)
return /** @type {Expression} */ (visit(node));
}

return b.call(
'$.store_mutate',
serialize_get_binding(b.id(left_name), state),
b.assignment(node.operator, /** @type {Pattern}} */ (visit_node(node.left)), value),
b.call('$.untrack', b.id('$' + left_name))
return maybe_skip_ownership_validation(
b.call(
'$.store_mutate',
serialize_get_binding(b.id(left_name), state),
b.assignment(node.operator, /** @type {Pattern}} */ (visit_node(node.left)), value),
b.call('$.untrack', b.id('$' + left_name))
)
);
} else if (
!state.analysis.runes ||
// this condition can go away once legacy mode is gone; only necessary for interop with legacy parent bindings
(binding.mutated && binding.kind === 'bindable_prop')
) {
if (binding.kind === 'bindable_prop') {
return b.call(
left,
b.assignment(node.operator, /** @type {Pattern} */ (visit(node.left)), value),
b.true
return maybe_skip_ownership_validation(
b.call(
left,
b.assignment(node.operator, /** @type {Pattern} */ (visit(node.left)), value),
b.true
)
);
} else {
return b.call(
'$.mutate',
b.id(left_name),
b.assignment(node.operator, /** @type {Pattern} */ (visit(node.left)), value)
return maybe_skip_ownership_validation(
b.call(
'$.mutate',
b.id(left_name),
b.assignment(node.operator, /** @type {Pattern} */ (visit(node.left)), value)
)
);
}
} else if (
node.right.type === 'Literal' &&
prefix != null &&
(node.operator === '+=' || node.operator === '-=')
) {
return b.update(
node.operator === '+=' ? '++' : '--',
/** @type {Expression} */ (visit(node.left)),
prefix
return maybe_skip_ownership_validation(
b.update(
node.operator === '+=' ? '++' : '--',
/** @type {Expression} */ (visit(node.left)),
prefix
)
);
} else {
return b.assignment(
node.operator,
/** @type {Pattern} */ (visit(node.left)),
/** @type {Expression} */ (visit(node.right))
return maybe_skip_ownership_validation(
b.assignment(
node.operator,
/** @type {Pattern} */ (visit(node.left)),
/** @type {Expression} */ (visit(node.right))
)
);
}
}
Expand Down
16 changes: 13 additions & 3 deletions packages/svelte/src/compiler/phases/3-transform/client/visitors/global.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import is_reference from 'is-reference';
import { serialize_get_binding, serialize_set_binding } from '../utils.js';
import * as b from '../../../../utils/builders.js';
import { is_ignored } from '../../../../state.js';

/** @type {Visitors} */
export const global_visitors = {
Expand Down Expand Up @@ -115,6 +116,15 @@ export const global_visitors = {

return b.call(fn, ...args);
} else {
/** @param {any} serialized */
function maybe_skip_ownership_validation(serialized) {
if (is_ignored(node, 'ownership_invalid_mutation')) {
return b.call('$.skip_ownership_validation', b.thunk(serialized));
}

return serialized;
}

// turn it into an IIFEE assignment expression: i++ -> (() => { const $$value = i; i+=1; return $$value; })
const assignment = b.assignment(
node.operator === '++' ? '+=' : '-=',
Expand All @@ -130,9 +140,9 @@ export const global_visitors = {
const value = /** @type {Expression} */ (visit(argument));
if (serialized_assignment === assignment) {
// No change to output -> nothing to transform -> we can keep the original update expression
return next();
return maybe_skip_ownership_validation(next());
} else if (context.state.analysis.runes) {
return serialized_assignment;
return maybe_skip_ownership_validation(serialized_assignment);
} else {
/** @type {Statement[]} */
let statements;
Expand All @@ -146,7 +156,7 @@ export const global_visitors = {
b.return(b.id(tmp_id))
];
}
return b.call(b.thunk(b.block(statements)));
return maybe_skip_ownership_validation(b.call(b.thunk(b.block(statements))));
}
}
}
Expand Down
20 changes: 13 additions & 7 deletions packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js
View file
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
/** @import { CallExpression, Expression, Identifier, Literal, MethodDefinition, PrivateIdentifier, PropertyDefinition, VariableDeclarator } from 'estree' */
/** @import { Binding } from '#compiler' */
/** @import { ComponentVisitors, StateField } from '../types.js' */
import { dev, is_ignored } from '../../../../state.js';
import * as assert from '../../../../utils/assert.js';
import { extract_paths } from '../../../../utils/ast.js';
import * as b from '../../../../utils/builders.js';
import { regex_invalid_identifier_chars } from '../../../patterns.js';
import { get_rune } from '../../../scope.js';
import { is_hoistable_function, transform_inspect_rune } from '../../utils.js';
import * as b from '../../../../utils/builders.js';
import * as assert from '../../../../utils/assert.js';
import {
get_prop_source,
is_prop_source,
is_state_source,
serialize_proxy_reassignment,
should_proxy_or_freeze
} from '../utils.js';
import { extract_paths } from '../../../../utils/ast.js';
import { regex_invalid_identifier_chars } from '../../../patterns.js';
import { dev } from '../../../../state.js';

/** @type {ComponentVisitors} */
export const javascript_visitors_runes = {
Expand Down Expand Up @@ -197,7 +197,9 @@ export const javascript_visitors_runes = {
b.call(
'$.add_owner',
b.call('$.get', b.member(b.this, b.private_id(name))),
b.id('owner')
b.id('owner'),
b.literal(false),
is_ignored(node, 'ownership_invalid_binding') && b.true
)
)
),
Expand Down Expand Up @@ -446,7 +448,11 @@ export const javascript_visitors_runes = {
}

if (rune === '$state.snapshot') {
return b.call('$.snapshot', /** @type {Expression} */ (context.visit(node.arguments[0])));
return b.call(
'$.snapshot',
/** @type {Expression} */ (context.visit(node.arguments[0])),
is_ignored(node, 'state_snapshot_uncloneable') && b.true
);
}

if (rune === '$state.is') {
Expand Down
99 changes: 65 additions & 34 deletions packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,26 @@
/** @import { SourceLocation } from '#shared' */
/** @import { Scope } from '../../../scope.js' */
/** @import { ComponentClientTransformState, ComponentContext, ComponentVisitors } from '../types.js' */
import is_reference from 'is-reference';
import { walk } from 'zimmerframe';
import {
AttributeAliases,
DOMBooleanAttributes,
EACH_INDEX_REACTIVE,
EACH_IS_ANIMATED,
EACH_IS_CONTROLLED,
EACH_IS_STRICT_EQUALS,
EACH_ITEM_REACTIVE,
EACH_KEYED,
is_capture_event,
TEMPLATE_FRAGMENT,
TEMPLATE_USE_IMPORT_NODE,
TRANSITION_GLOBAL,
TRANSITION_IN,
TRANSITION_OUT
} from '../../../../../constants.js';
import { escape_html } from '../../../../../escaping.js';
import { dev, is_ignored, locator } from '../../../../state.js';
import {
extract_identifiers,
extract_paths,
Expand All @@ -13,48 +33,28 @@ import {
object,
unwrap_optional
} from '../../../../utils/ast.js';
import * as b from '../../../../utils/builders.js';
import { sanitize_template_string } from '../../../../utils/sanitize_template_string.js';
import { binding_properties } from '../../../bindings.js';
import { clean_nodes, determine_namespace_for_children, infer_namespace } from '../../utils.js';
import {
DOMProperties,
LoadErrorElements,
PassiveEvents,
VoidElements
} from '../../../constants.js';
import { is_custom_element_node, is_element_node } from '../../../nodes.js';
import * as b from '../../../../utils/builders.js';
import { regex_is_valid_identifier } from '../../../patterns.js';
import { clean_nodes, determine_namespace_for_children, infer_namespace } from '../../utils.js';
import {
with_loc,
create_derived,
create_derived_block_argument,
function_visitor,
get_assignment_value,
serialize_get_binding,
serialize_set_binding,
create_derived,
create_derived_block_argument
with_loc
} from '../utils.js';
import {
AttributeAliases,
DOMBooleanAttributes,
EACH_INDEX_REACTIVE,
EACH_IS_ANIMATED,
EACH_IS_CONTROLLED,
EACH_IS_STRICT_EQUALS,
EACH_ITEM_REACTIVE,
EACH_KEYED,
is_capture_event,
TEMPLATE_FRAGMENT,
TEMPLATE_USE_IMPORT_NODE,
TRANSITION_GLOBAL,
TRANSITION_IN,
TRANSITION_OUT
} from '../../../../../constants.js';
import { escape_html } from '../../../../../escaping.js';
import { regex_is_valid_identifier } from '../../../patterns.js';
import { javascript_visitors_runes } from './javascript-runes.js';
import { sanitize_template_string } from '../../../../utils/sanitize_template_string.js';
import { walk } from 'zimmerframe';
import { dev, locator } from '../../../../state.js';
import is_reference from 'is-reference';

/**
* @param {RegularElement | SvelteElement} element
Expand Down Expand Up @@ -324,7 +324,8 @@ function serialize_element_spread_attributes(
b.id(id),
b.object(values),
lowercase_attributes,
b.literal(context.state.analysis.css.hash)
b.literal(context.state.analysis.css.hash),
is_ignored(element, 'hydration_attribute_changed') && b.true
)
)
);
Expand Down Expand Up @@ -489,7 +490,15 @@ function serialize_element_attribute_update_assignment(element, node_id, attribu

// The foreign namespace doesn't have any special handling, everything goes through the attr function
if (context.state.metadata.namespace === 'foreign') {
const statement = b.stmt(b.call('$.set_attribute', node_id, b.literal(name), value));
const statement = b.stmt(
b.call(
'$.set_attribute',
node_id,
b.literal(name),
value,
is_ignored(element, 'hydration_attribute_changed') && b.true
)
);

if (attribute.metadata.expression.has_state) {
const id = state.scope.generate(`${node_id.name}_${name}`);
Expand Down Expand Up @@ -525,7 +534,15 @@ function serialize_element_attribute_update_assignment(element, node_id, attribu
update = b.stmt(b.assignment('=', b.member(node_id, b.id(name)), value));
} else {
const callee = name.startsWith('xlink') ? '$.set_xlink_attribute' : '$.set_attribute';
update = b.stmt(b.call(callee, node_id, b.literal(name), value));
update = b.stmt(
b.call(
callee,
node_id,
b.literal(name),
value,
is_ignored(element, 'hydration_attribute_changed') && b.true
)
);
}

if (attribute.metadata.expression.has_state) {
Expand Down Expand Up @@ -780,7 +797,12 @@ function serialize_inline_component(node, component_name, context, anchor = cont
} else if (attribute.type === 'BindDirective') {
const expression = /** @type {Expression} */ (context.visit(attribute.expression));

if (dev && expression.type === 'MemberExpression' && context.state.analysis.runes) {
if (
dev &&
expression.type === 'MemberExpression' &&
context.state.analysis.runes &&
!is_ignored(node, 'binding_property_non_reactive')
) {
context.state.init.push(serialize_validate_binding(context.state, attribute, expression));
}

Expand All @@ -789,7 +811,14 @@ function serialize_inline_component(node, component_name, context, anchor = cont
} else {
if (dev) {
binding_initializers.push(
b.stmt(b.call(b.id('$.add_owner_effect'), b.thunk(expression), b.id(component_name)))
b.stmt(
b.call(
b.id('$.add_owner_effect'),
b.thunk(expression),
b.id(component_name),
is_ignored(node, 'ownership_invalid_binding') && b.true
)
)
);
}

Expand Down Expand Up @@ -1811,7 +1840,8 @@ export const template_visitors = {
context.state.node,
b.thunk(/** @type {Expression} */ (context.visit(node.expression))),
b.literal(context.state.metadata.namespace === 'svg'),
b.literal(context.state.metadata.namespace === 'mathml')
b.literal(context.state.metadata.namespace === 'mathml'),
is_ignored(node, 'hydration_html_changed') && b.true
)
)
);
Expand Down Expand Up @@ -2903,7 +2933,8 @@ export const template_visitors = {
type === 'KeyBlock'
)) &&
dev &&
context.state.analysis.runes
context.state.analysis.runes &&
!is_ignored(node, 'binding_property_non_reactive')
) {
context.state.init.push(
serialize_validate_binding(
Expand Down
Loading

0 comments on commit 64d2a2e

Please sign in to comment.