From b43c22b114ece786ebdb35353673e66b4a0fd789 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Thu, 25 Jul 2024 19:00:03 +0200 Subject: [PATCH 01/21] feat: allow ignoring binding_property_non_reactive --- .changeset/large-emus-cough.md | 5 +++++ .../3-transform/client/visitors/template.js | 16 +++++++++++++--- packages/svelte/src/compiler/warnings.js | 3 ++- .../_config.js | 11 +++++++++++ .../main.svelte | 6 ++++++ .../_config.js | 11 +++++++++++ .../main.svelte | 6 ++++++ 7 files changed, 54 insertions(+), 4 deletions(-) create mode 100644 .changeset/large-emus-cough.md create mode 100644 packages/svelte/tests/runtime-runes/samples/binding-property-non-reactive-ignored-2/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/binding-property-non-reactive-ignored-2/main.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/binding-property-non-reactive-ignored/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/binding-property-non-reactive-ignored/main.svelte diff --git a/.changeset/large-emus-cough.md b/.changeset/large-emus-cough.md new file mode 100644 index 000000000000..81e6d9b71443 --- /dev/null +++ b/.changeset/large-emus-cough.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +feat: allow ignoring binding_property_non_reactive diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js index 3e8fff654f6d..a6b4b0f72e9a 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js @@ -53,7 +53,7 @@ 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 { locator } from '../../../../state.js'; +import { ignore_map, locator } from '../../../../state.js'; import is_reference from 'is-reference'; /** @@ -781,10 +781,15 @@ function serialize_inline_component(node, component_name, context, anchor = cont } else if (attribute.type === 'BindDirective') { const expression = /** @type {Expression} */ (context.visit(attribute.expression)); + const to_ignore = ignore_map + .get(node) + ?.some((code) => code.has('binding_property_non_reactive')); + if ( expression.type === 'MemberExpression' && context.state.options.dev && - context.state.analysis.runes + context.state.analysis.runes && + !to_ignore ) { context.state.init.push(serialize_validate_binding(context.state, attribute, expression)); } @@ -2872,6 +2877,10 @@ export const template_visitors = { const expression = node.expression; const property = binding_properties[node.name]; + const to_ignore = ignore_map + .get(node) + ?.some((code) => code.has('binding_property_non_reactive')); + if ( expression.type === 'MemberExpression' && (node.name !== 'this' || @@ -2883,7 +2892,8 @@ export const template_visitors = { type === 'KeyBlock' )) && context.state.options.dev && - context.state.analysis.runes + context.state.analysis.runes && + !to_ignore ) { context.state.init.push( serialize_validate_binding( diff --git a/packages/svelte/src/compiler/warnings.js b/packages/svelte/src/compiler/warnings.js index d9c197cf7522..2735193a6d5c 100644 --- a/packages/svelte/src/compiler/warnings.js +++ b/packages/svelte/src/compiler/warnings.js @@ -115,7 +115,8 @@ export const codes = [ "element_invalid_self_closing_tag", "event_directive_deprecated", "slot_element_deprecated", - "svelte_element_invalid_this" + "svelte_element_invalid_this", + "binding_property_non_reactive" ]; /** diff --git a/packages/svelte/tests/runtime-runes/samples/binding-property-non-reactive-ignored-2/_config.js b/packages/svelte/tests/runtime-runes/samples/binding-property-non-reactive-ignored-2/_config.js new file mode 100644 index 000000000000..e93067eb9d34 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/binding-property-non-reactive-ignored-2/_config.js @@ -0,0 +1,11 @@ +import { test } from '../../test'; + +export default test({ + mode: ['client'], + compileOptions: { + dev: true + }, + async test({ warnings, assert }) { + assert.deepEqual(warnings, []); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/binding-property-non-reactive-ignored-2/main.svelte b/packages/svelte/tests/runtime-runes/samples/binding-property-non-reactive-ignored-2/main.svelte new file mode 100644 index 000000000000..218cfe7506cd --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/binding-property-non-reactive-ignored-2/main.svelte @@ -0,0 +1,6 @@ + + + + diff --git a/packages/svelte/tests/runtime-runes/samples/binding-property-non-reactive-ignored/_config.js b/packages/svelte/tests/runtime-runes/samples/binding-property-non-reactive-ignored/_config.js new file mode 100644 index 000000000000..e93067eb9d34 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/binding-property-non-reactive-ignored/_config.js @@ -0,0 +1,11 @@ +import { test } from '../../test'; + +export default test({ + mode: ['client'], + compileOptions: { + dev: true + }, + async test({ warnings, assert }) { + assert.deepEqual(warnings, []); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/binding-property-non-reactive-ignored/main.svelte b/packages/svelte/tests/runtime-runes/samples/binding-property-non-reactive-ignored/main.svelte new file mode 100644 index 000000000000..4cbd69d06bf1 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/binding-property-non-reactive-ignored/main.svelte @@ -0,0 +1,6 @@ + + + + From cedc6d41e302a6c6090d236bd05262e57f77c7a2 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Thu, 25 Jul 2024 19:05:47 +0200 Subject: [PATCH 02/21] chore: add comments before `to_ignore` --- .../compiler/phases/3-transform/client/visitors/template.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js index a6b4b0f72e9a..7a1a7c791cf2 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js @@ -781,6 +781,9 @@ function serialize_inline_component(node, component_name, context, anchor = cont } else if (attribute.type === 'BindDirective') { const expression = /** @type {Expression} */ (context.visit(attribute.expression)); + // serialize_validate_binding will add a function that specifically throw + // `binding_property_non_reactive` error. If there's a svelte ignore + // before we avoid adding this validation to avoid throwing the runtime warning const to_ignore = ignore_map .get(node) ?.some((code) => code.has('binding_property_non_reactive')); @@ -2877,6 +2880,9 @@ export const template_visitors = { const expression = node.expression; const property = binding_properties[node.name]; + // serialize_validate_binding will add a function that specifically throw + // `binding_property_non_reactive` error. If there's a svelte ignore + // before we avoid adding this validation to avoid throwing the runtime warning const to_ignore = ignore_map .get(node) ?.some((code) => code.has('binding_property_non_reactive')); From 3e0f6c18c547e440efdb8af7f97d9f21ad43da19 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Thu, 25 Jul 2024 21:13:05 +0200 Subject: [PATCH 03/21] chore: fix warnings regeneration --- packages/svelte/src/compiler/warnings.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/svelte/src/compiler/warnings.js b/packages/svelte/src/compiler/warnings.js index 2735193a6d5c..d9c197cf7522 100644 --- a/packages/svelte/src/compiler/warnings.js +++ b/packages/svelte/src/compiler/warnings.js @@ -115,8 +115,7 @@ export const codes = [ "element_invalid_self_closing_tag", "event_directive_deprecated", "slot_element_deprecated", - "svelte_element_invalid_this", - "binding_property_non_reactive" + "svelte_element_invalid_this" ]; /** From 4eaf118efec1ed86e5bc504b9d96d588dfffe8e9 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Mon, 29 Jul 2024 22:27:48 +0200 Subject: [PATCH 04/21] chore: include client warnings code in svelte ignore extract --- .../process-messages/templates/client-warnings.js | 2 ++ .../src/compiler/utils/extract_svelte_ignore.js | 11 ++++++----- packages/svelte/src/internal/client/warnings.js | 12 ++++++++++++ 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/packages/svelte/scripts/process-messages/templates/client-warnings.js b/packages/svelte/scripts/process-messages/templates/client-warnings.js index d40a59ec0c8d..3d78466435fb 100644 --- a/packages/svelte/scripts/process-messages/templates/client-warnings.js +++ b/packages/svelte/scripts/process-messages/templates/client-warnings.js @@ -15,3 +15,5 @@ export function CODE(PARAMETER) { console.warn('CODE'); } } + +export const codes = CODES; diff --git a/packages/svelte/src/compiler/utils/extract_svelte_ignore.js b/packages/svelte/src/compiler/utils/extract_svelte_ignore.js index ff440d4c4125..1c6f73f1e8c0 100644 --- a/packages/svelte/src/compiler/utils/extract_svelte_ignore.js +++ b/packages/svelte/src/compiler/utils/extract_svelte_ignore.js @@ -1,5 +1,6 @@ import fuzzymatch from '../phases/1-parse/utils/fuzzymatch.js'; import * as w from '../warnings.js'; +import { codes as client_codes } from '../../internal/client/warnings.js'; const regex_svelte_ignore = /^\s*svelte-ignore\s/; @@ -37,7 +38,7 @@ export function extract_svelte_ignore(offset, text, runes) { for (const match of text.slice(length).matchAll(/([\w$-]+)(,)?/gm)) { const code = match[1]; - if (w.codes.includes(code)) { + if (w.codes.includes(code) || client_codes.includes(code)) { ignores.push(code); } else { const replacement = replacements[code] ?? code.replace(/-/g, '_'); @@ -46,10 +47,10 @@ export function extract_svelte_ignore(offset, text, runes) { const start = offset + /** @type {number} */ (match.index); const end = start + code.length; - if (w.codes.includes(replacement)) { + if (w.codes.includes(replacement) || client_codes.includes(replacement)) { w.legacy_code({ start, end }, code, replacement); } else { - const suggestion = fuzzymatch(code, w.codes); + const suggestion = fuzzymatch(code, w.codes.concat(client_codes)); w.unknown_code({ start, end }, code, suggestion); } } @@ -65,10 +66,10 @@ export function extract_svelte_ignore(offset, text, runes) { ignores.push(code); - if (!w.codes.includes(code)) { + if (!w.codes.includes(code) || client_codes.includes(code)) { const replacement = replacements[code] ?? code.replace(/-/g, '_'); - if (w.codes.includes(replacement)) { + if (w.codes.includes(replacement) || client_codes.includes(replacement)) { ignores.push(replacement); } } diff --git a/packages/svelte/src/internal/client/warnings.js b/packages/svelte/src/internal/client/warnings.js index 23c00f971dbb..1cb604284492 100644 --- a/packages/svelte/src/internal/client/warnings.js +++ b/packages/svelte/src/internal/client/warnings.js @@ -5,6 +5,18 @@ import { DEV } from 'esm-env'; var bold = 'font-weight: bold'; var normal = 'font-weight: normal'; +export const codes = [ + "binding_property_non_reactive", + "hydration_attribute_changed", + "hydration_html_changed", + "hydration_mismatch", + "invalid_raw_snippet_render", + "lifecycle_double_unmount", + "ownership_invalid_binding", + "ownership_invalid_mutation", + "state_proxy_equality_mismatch" +]; + /** * `%binding%` (%location%) is binding to a non-reactive property * @param {string} binding From dc536546d369a5e6cc7e334420fdbf46905e1925 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Mon, 29 Jul 2024 22:45:55 +0200 Subject: [PATCH 05/21] feat: allow ignoring state_snapshot_uncloneable --- .../process-messages/templates/shared-warnings.js | 2 ++ .../3-transform/client/visitors/javascript-runes.js | 11 ++++++++++- .../3-transform/server/visitors/CallExpression.js | 9 ++++++++- .../src/compiler/utils/extract_svelte_ignore.js | 13 ++++++++----- packages/svelte/src/internal/shared/clone.js | 7 ++++--- packages/svelte/src/internal/shared/warnings.js | 5 +++++ .../state-snapshot-uncloneable-ignored/_config.js | 11 +++++++++++ .../state-snapshot-uncloneable-ignored/main.svelte | 11 +++++++++++ 8 files changed, 59 insertions(+), 10 deletions(-) create mode 100644 packages/svelte/tests/runtime-runes/samples/state-snapshot-uncloneable-ignored/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/state-snapshot-uncloneable-ignored/main.svelte diff --git a/packages/svelte/scripts/process-messages/templates/shared-warnings.js b/packages/svelte/scripts/process-messages/templates/shared-warnings.js index d40a59ec0c8d..3d78466435fb 100644 --- a/packages/svelte/scripts/process-messages/templates/shared-warnings.js +++ b/packages/svelte/scripts/process-messages/templates/shared-warnings.js @@ -15,3 +15,5 @@ export function CODE(PARAMETER) { console.warn('CODE'); } } + +export const codes = CODES; diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js index f24514a8f9c3..162657a39505 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js @@ -14,6 +14,7 @@ import { } from '../utils.js'; import { extract_paths } from '../../../../utils/ast.js'; import { regex_invalid_identifier_chars } from '../../../patterns.js'; +import { ignore_map } from '../../../../state.js'; /** @type {ComponentVisitors} */ export const javascript_visitors_runes = { @@ -449,7 +450,15 @@ export const javascript_visitors_runes = { } if (rune === '$state.snapshot') { - return b.call('$.snapshot', /** @type {Expression} */ (context.visit(node.arguments[0]))); + const to_ignore = ignore_map + .get(node) + ?.some((code) => code.has('state_snapshot_uncloneable')); + + return b.call( + '$.snapshot', + /** @type {Expression} */ (context.visit(node.arguments[0])), + b.literal(!!to_ignore) + ); } if (rune === '$state.is') { diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/CallExpression.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/CallExpression.js index 91ff0f67719f..1845a59f959a 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/CallExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/CallExpression.js @@ -3,6 +3,7 @@ import { get_rune } from '../../../scope.js'; import * as b from '../../../../utils/builders.js'; import { transform_inspect_rune } from '../../utils.js'; +import { ignore_map } from '../../../../state.js'; /** * @param {CallExpression} node @@ -25,7 +26,13 @@ export function CallExpression(node, context) { } if (rune === '$state.snapshot') { - return b.call('$.snapshot', /** @type {Expression} */ (context.visit(node.arguments[0]))); + const to_ignore = ignore_map.get(node)?.some((code) => code.has('state_snapshot_uncloneable')); + + return b.call( + '$.snapshot', + /** @type {Expression} */ (context.visit(node.arguments[0])), + b.literal(!!to_ignore) + ); } if (rune === '$state.is') { diff --git a/packages/svelte/src/compiler/utils/extract_svelte_ignore.js b/packages/svelte/src/compiler/utils/extract_svelte_ignore.js index 1c6f73f1e8c0..4939e269e412 100644 --- a/packages/svelte/src/compiler/utils/extract_svelte_ignore.js +++ b/packages/svelte/src/compiler/utils/extract_svelte_ignore.js @@ -1,6 +1,7 @@ import fuzzymatch from '../phases/1-parse/utils/fuzzymatch.js'; import * as w from '../warnings.js'; import { codes as client_codes } from '../../internal/client/warnings.js'; +import { codes as shared_codes } from '../../internal/shared/warnings.js'; const regex_svelte_ignore = /^\s*svelte-ignore\s/; @@ -17,6 +18,8 @@ const replacements = { 'unused-export-let': 'export_let_unused' }; +const codes = w.codes.concat(client_codes).concat(shared_codes); + /** * @param {number} offset * @param {string} text @@ -38,7 +41,7 @@ export function extract_svelte_ignore(offset, text, runes) { for (const match of text.slice(length).matchAll(/([\w$-]+)(,)?/gm)) { const code = match[1]; - if (w.codes.includes(code) || client_codes.includes(code)) { + if (codes.includes(code)) { ignores.push(code); } else { const replacement = replacements[code] ?? code.replace(/-/g, '_'); @@ -47,10 +50,10 @@ export function extract_svelte_ignore(offset, text, runes) { const start = offset + /** @type {number} */ (match.index); const end = start + code.length; - if (w.codes.includes(replacement) || client_codes.includes(replacement)) { + if (codes.includes(replacement)) { w.legacy_code({ start, end }, code, replacement); } else { - const suggestion = fuzzymatch(code, w.codes.concat(client_codes)); + const suggestion = fuzzymatch(code, codes); w.unknown_code({ start, end }, code, suggestion); } } @@ -66,10 +69,10 @@ export function extract_svelte_ignore(offset, text, runes) { ignores.push(code); - if (!w.codes.includes(code) || client_codes.includes(code)) { + if (!codes.includes(code)) { const replacement = replacements[code] ?? code.replace(/-/g, '_'); - if (w.codes.includes(replacement) || client_codes.includes(replacement)) { + if (codes.includes(replacement)) { ignores.push(replacement); } } diff --git a/packages/svelte/src/internal/shared/clone.js b/packages/svelte/src/internal/shared/clone.js index 9983ef619a16..9027c15c4f12 100644 --- a/packages/svelte/src/internal/shared/clone.js +++ b/packages/svelte/src/internal/shared/clone.js @@ -14,18 +14,19 @@ const empty = []; /** * @template T * @param {T} value + * @param {boolean} [skip_warning] * @returns {Snapshot} */ -export function snapshot(value) { +export function snapshot(value, skip_warning = false) { if (DEV) { /** @type {string[]} */ const paths = []; const copy = clone(value, new Map(), '', paths); - if (paths.length === 1 && paths[0] === '') { + if (paths.length === 1 && paths[0] === '' && !skip_warning) { // value could not be cloned w.state_snapshot_uncloneable(); - } else if (paths.length > 0) { + } else if (paths.length > 0 && !skip_warning) { // some properties could not be cloned const slice = paths.length > 10 ? paths.slice(0, 7) : paths.slice(0, 10); const excess = paths.length - slice.length; diff --git a/packages/svelte/src/internal/shared/warnings.js b/packages/svelte/src/internal/shared/warnings.js index 37269a674eb5..7fd4a525f271 100644 --- a/packages/svelte/src/internal/shared/warnings.js +++ b/packages/svelte/src/internal/shared/warnings.js @@ -5,6 +5,11 @@ import { DEV } from 'esm-env'; var bold = 'font-weight: bold'; var normal = 'font-weight: normal'; +export const codes = [ + "dynamic_void_element_content", + "state_snapshot_uncloneable" +]; + /** * `` is a void element — it cannot have content * @param {string} tag diff --git a/packages/svelte/tests/runtime-runes/samples/state-snapshot-uncloneable-ignored/_config.js b/packages/svelte/tests/runtime-runes/samples/state-snapshot-uncloneable-ignored/_config.js new file mode 100644 index 000000000000..e93067eb9d34 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/state-snapshot-uncloneable-ignored/_config.js @@ -0,0 +1,11 @@ +import { test } from '../../test'; + +export default test({ + mode: ['client'], + compileOptions: { + dev: true + }, + async test({ warnings, assert }) { + assert.deepEqual(warnings, []); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/state-snapshot-uncloneable-ignored/main.svelte b/packages/svelte/tests/runtime-runes/samples/state-snapshot-uncloneable-ignored/main.svelte new file mode 100644 index 000000000000..4bc1c1d1a09b --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/state-snapshot-uncloneable-ignored/main.svelte @@ -0,0 +1,11 @@ + + + +
a
From ca7aa51bc7b3132c113a745256bce1791d73e4d8 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Mon, 29 Jul 2024 22:53:39 +0200 Subject: [PATCH 06/21] chore: abstract ignore into function --- .../client/visitors/javascript-runes.js | 16 ++-- .../3-transform/client/visitors/template.js | 78 +++++++++---------- .../server/visitors/CallExpression.js | 8 +- packages/svelte/src/compiler/state.js | 10 +++ 4 files changed, 54 insertions(+), 58 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js index 162657a39505..3ddc1dc6da71 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js @@ -1,10 +1,13 @@ /** @import { CallExpression, Expression, Identifier, Literal, MethodDefinition, PrivateIdentifier, PropertyDefinition, VariableDeclarator } from 'estree' */ /** @import { Binding } from '#compiler' */ /** @import { ComponentVisitors, StateField } from '../types.js' */ +import { is_to_ignore } 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, @@ -12,9 +15,6 @@ import { 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 { ignore_map } from '../../../../state.js'; /** @type {ComponentVisitors} */ export const javascript_visitors_runes = { @@ -450,14 +450,10 @@ export const javascript_visitors_runes = { } if (rune === '$state.snapshot') { - const to_ignore = ignore_map - .get(node) - ?.some((code) => code.has('state_snapshot_uncloneable')); - return b.call( '$.snapshot', /** @type {Expression} */ (context.visit(node.arguments[0])), - b.literal(!!to_ignore) + b.literal(is_to_ignore(node, 'state_snapshot_uncloneable')) ); } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js index 8d6aa14cfd55..a6537a4d4f4a 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js @@ -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 { is_to_ignore, locator } from '../../../../state.js'; import { extract_identifiers, extract_paths, @@ -13,8 +33,9 @@ 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, @@ -22,39 +43,18 @@ import { 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 { ignore_map, locator } from '../../../../state.js'; -import is_reference from 'is-reference'; /** * @param {RegularElement | SvelteElement} element @@ -781,18 +781,14 @@ function serialize_inline_component(node, component_name, context, anchor = cont } else if (attribute.type === 'BindDirective') { const expression = /** @type {Expression} */ (context.visit(attribute.expression)); - // serialize_validate_binding will add a function that specifically throw - // `binding_property_non_reactive` error. If there's a svelte ignore - // before we avoid adding this validation to avoid throwing the runtime warning - const to_ignore = ignore_map - .get(node) - ?.some((code) => code.has('binding_property_non_reactive')); - if ( expression.type === 'MemberExpression' && context.state.options.dev && context.state.analysis.runes && - !to_ignore + // serialize_validate_binding will add a function that specifically throw + // `binding_property_non_reactive` error. If there's a svelte ignore + // before we avoid adding this validation to avoid throwing the runtime warning + !is_to_ignore(node, 'binding_property_non_reactive') ) { context.state.init.push(serialize_validate_binding(context.state, attribute, expression)); } @@ -2894,13 +2890,6 @@ export const template_visitors = { const expression = node.expression; const property = binding_properties[node.name]; - // serialize_validate_binding will add a function that specifically throw - // `binding_property_non_reactive` error. If there's a svelte ignore - // before we avoid adding this validation to avoid throwing the runtime warning - const to_ignore = ignore_map - .get(node) - ?.some((code) => code.has('binding_property_non_reactive')); - if ( expression.type === 'MemberExpression' && (node.name !== 'this' || @@ -2913,7 +2902,10 @@ export const template_visitors = { )) && context.state.options.dev && context.state.analysis.runes && - !to_ignore + // serialize_validate_binding will add a function that specifically throw + // `binding_property_non_reactive` error. If there's a svelte ignore + // before we avoid adding this validation to avoid throwing the runtime warning + !is_to_ignore(node, 'binding_property_non_reactive') ) { context.state.init.push( serialize_validate_binding( diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/CallExpression.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/CallExpression.js index 1845a59f959a..760a980a06e2 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/CallExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/CallExpression.js @@ -1,9 +1,9 @@ /** @import { CallExpression, Expression } from 'estree' */ /** @import { Context } from '../types.js' */ -import { get_rune } from '../../../scope.js'; +import { is_to_ignore } from '../../../../state.js'; import * as b from '../../../../utils/builders.js'; +import { get_rune } from '../../../scope.js'; import { transform_inspect_rune } from '../../utils.js'; -import { ignore_map } from '../../../../state.js'; /** * @param {CallExpression} node @@ -26,12 +26,10 @@ export function CallExpression(node, context) { } if (rune === '$state.snapshot') { - const to_ignore = ignore_map.get(node)?.some((code) => code.has('state_snapshot_uncloneable')); - return b.call( '$.snapshot', /** @type {Expression} */ (context.visit(node.arguments[0])), - b.literal(!!to_ignore) + b.literal(is_to_ignore(node, 'state_snapshot_uncloneable')) ); } diff --git a/packages/svelte/src/compiler/state.js b/packages/svelte/src/compiler/state.js index 459c9a56c323..37f82c744ce9 100644 --- a/packages/svelte/src/compiler/state.js +++ b/packages/svelte/src/compiler/state.js @@ -59,6 +59,16 @@ export function reset_warning_filter(fn = () => true) { warning_filter = fn; } +/** + * + * @param {SvelteNode | NodeLike} node + * @param {string} code + * @returns + */ +export function is_to_ignore(node, code) { + return !!ignore_map.get(node)?.some((code_set) => code_set.has(code)); +} + /** * @param {string} _source * @param {{ filename?: string, rootDir?: string }} options From e1595c0b30e1bed84b0dca458c30a404ec85eaec Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Mon, 29 Jul 2024 23:06:11 +0200 Subject: [PATCH 07/21] feat: allow skipping of `hydration_attribute_changed` --- .../3-transform/client/visitors/template.js | 23 ++++++++++++++++--- .../client/dom/elements/attributes.js | 12 ++++++---- .../_config.js | 16 +++++++++++++ .../main.svelte | 6 +++++ 4 files changed, 50 insertions(+), 7 deletions(-) create mode 100644 packages/svelte/tests/runtime-runes/samples/hydration-attribute-changed-ignored/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/hydration-attribute-changed-ignored/main.svelte diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js index a6537a4d4f4a..7fbdcab08145 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js @@ -325,7 +325,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), + b.literal(is_to_ignore(element, 'hydration_attribute_changed')) ) ) ); @@ -490,7 +491,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, + b.literal(is_to_ignore(element, 'hydration_attribute_changed')) + ) + ); if (attribute.metadata.dynamic) { const id = state.scope.generate(`${node_id.name}_${name}`); @@ -526,7 +535,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, + b.literal(is_to_ignore(element, 'hydration_attribute_changed')) + ) + ); } if (attribute.metadata.dynamic) { diff --git a/packages/svelte/src/internal/client/dom/elements/attributes.js b/packages/svelte/src/internal/client/dom/elements/attributes.js index 7c8c0d8107e5..3e6b5c312caf 100644 --- a/packages/svelte/src/internal/client/dom/elements/attributes.js +++ b/packages/svelte/src/internal/client/dom/elements/attributes.js @@ -82,8 +82,9 @@ export function set_checked(element, checked) { * @param {Element} element * @param {string} attribute * @param {string | null} value + * @param {boolean} [skip_warning] */ -export function set_attribute(element, attribute, value) { +export function set_attribute(element, attribute, value, skip_warning) { value = value == null ? null : value + ''; // @ts-expect-error @@ -93,7 +94,9 @@ export function set_attribute(element, attribute, value) { attributes[attribute] = element.getAttribute(attribute); if (attribute === 'src' || attribute === 'href' || attribute === 'srcset') { - check_src_in_dev_hydration(element, attribute, value); + if (!skip_warning) { + check_src_in_dev_hydration(element, attribute, value); + } // If we reset these attributes, they would result in another network request, which we want to avoid. // We assume they are the same between client and server as checking if they are equal is expensive @@ -150,9 +153,10 @@ export function set_custom_element_data(node, prop, value) { * @param {Record} next New attributes - this function mutates this object * @param {boolean} lowercase_attributes * @param {string} css_hash + * @param {boolean} [skip_warning] * @returns {Record} */ -export function set_attributes(element, prev, next, lowercase_attributes, css_hash) { +export function set_attributes(element, prev, next, lowercase_attributes, css_hash, skip_warning) { var has_hash = css_hash.length !== 0; var current = prev || {}; var is_option_element = element.tagName === 'OPTION'; @@ -274,7 +278,7 @@ export function set_attributes(element, prev, next, lowercase_attributes, css_ha if (setters.includes(name)) { if (hydrating && (name === 'src' || name === 'href' || name === 'srcset')) { - check_src_in_dev_hydration(element, name, value); + if (!skip_warning) check_src_in_dev_hydration(element, name, value); } else { // @ts-ignore element[name] = value; diff --git a/packages/svelte/tests/runtime-runes/samples/hydration-attribute-changed-ignored/_config.js b/packages/svelte/tests/runtime-runes/samples/hydration-attribute-changed-ignored/_config.js new file mode 100644 index 000000000000..6bbad1bf6469 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/hydration-attribute-changed-ignored/_config.js @@ -0,0 +1,16 @@ +import { test } from '../../test'; + +export default test({ + server_props: { + browser: false + }, + props: { + browser: true + }, + compileOptions: { + dev: true + }, + async test({ warnings, assert }) { + assert.deepEqual(warnings, []); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/hydration-attribute-changed-ignored/main.svelte b/packages/svelte/tests/runtime-runes/samples/hydration-attribute-changed-ignored/main.svelte new file mode 100644 index 000000000000..c06bf953412e --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/hydration-attribute-changed-ignored/main.svelte @@ -0,0 +1,6 @@ + + + + From d9dadd5981ac211ff79eae3697a7ab7f2c9b81f4 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Mon, 29 Jul 2024 23:10:21 +0200 Subject: [PATCH 08/21] feat: allow skip of `hydration_html_changed` --- .../3-transform/client/visitors/template.js | 3 ++- .../src/internal/client/dom/blocks/html.js | 5 +++-- .../hydration-html-changed-ignored/_config.js | 16 ++++++++++++++++ .../hydration-html-changed-ignored/main.svelte | 6 ++++++ 4 files changed, 27 insertions(+), 3 deletions(-) create mode 100644 packages/svelte/tests/runtime-runes/samples/hydration-html-changed-ignored/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/hydration-html-changed-ignored/main.svelte diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js index 7fbdcab08145..d1065f69655f 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js @@ -1826,7 +1826,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'), + b.literal(is_to_ignore(node, 'hydration_html_changed')) ) ) ); diff --git a/packages/svelte/src/internal/client/dom/blocks/html.js b/packages/svelte/src/internal/client/dom/blocks/html.js index 3c4051261e20..63ca704c0f59 100644 --- a/packages/svelte/src/internal/client/dom/blocks/html.js +++ b/packages/svelte/src/internal/client/dom/blocks/html.js @@ -37,9 +37,10 @@ function check_hash(element, server_hash, value) { * @param {() => string} get_value * @param {boolean} svg * @param {boolean} mathml + * @param {boolean} [skip_warning] * @returns {void} */ -export function html(node, get_value, svg, mathml) { +export function html(node, get_value, svg, mathml, skip_warning) { var anchor = node; var value = ''; @@ -78,7 +79,7 @@ export function html(node, get_value, svg, mathml) { throw HYDRATION_ERROR; } - if (DEV) { + if (DEV && !skip_warning) { check_hash(/** @type {Element} */ (next.parentNode), hash, value); } diff --git a/packages/svelte/tests/runtime-runes/samples/hydration-html-changed-ignored/_config.js b/packages/svelte/tests/runtime-runes/samples/hydration-html-changed-ignored/_config.js new file mode 100644 index 000000000000..6bbad1bf6469 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/hydration-html-changed-ignored/_config.js @@ -0,0 +1,16 @@ +import { test } from '../../test'; + +export default test({ + server_props: { + browser: false + }, + props: { + browser: true + }, + compileOptions: { + dev: true + }, + async test({ warnings, assert }) { + assert.deepEqual(warnings, []); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/hydration-html-changed-ignored/main.svelte b/packages/svelte/tests/runtime-runes/samples/hydration-html-changed-ignored/main.svelte new file mode 100644 index 000000000000..fc1e4f4e50ea --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/hydration-html-changed-ignored/main.svelte @@ -0,0 +1,6 @@ + + + +{@html browser ? 'a' : 'b'} From 6f92da43876854e48dae030b7501840e109ec99d Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Mon, 29 Jul 2024 23:26:51 +0200 Subject: [PATCH 09/21] feat: allow skipping `ownership_invalid_binding` --- .../3-transform/client/visitors/javascript-runes.js | 4 +++- .../phases/3-transform/client/visitors/template.js | 9 ++++++++- packages/svelte/src/internal/client/dev/ownership.js | 10 ++++++---- .../ownership-invalid-binding-ignored/Child.svelte | 5 +++++ .../ownership-invalid-binding-ignored/Parent.svelte | 8 ++++++++ .../ownership-invalid-binding-ignored/_config.js | 11 +++++++++++ .../ownership-invalid-binding-ignored/main.svelte | 6 ++++++ .../_expected/client/main.svelte.js | 8 ++++---- 8 files changed, 51 insertions(+), 10 deletions(-) create mode 100644 packages/svelte/tests/runtime-runes/samples/ownership-invalid-binding-ignored/Child.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/ownership-invalid-binding-ignored/Parent.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/ownership-invalid-binding-ignored/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/ownership-invalid-binding-ignored/main.svelte diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js index 3ddc1dc6da71..164c749e3cd5 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js @@ -201,7 +201,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), + b.literal(is_to_ignore(node, 'ownership_invalid_binding')) ) ) ), diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js index d1065f69655f..6bba5e4a4225 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js @@ -815,7 +815,14 @@ function serialize_inline_component(node, component_name, context, anchor = cont } else { if (context.state.options.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), + b.literal(is_to_ignore(node, 'ownership_invalid_binding')) + ) + ) ); } diff --git a/packages/svelte/src/internal/client/dev/ownership.js b/packages/svelte/src/internal/client/dev/ownership.js index 7d10118788ed..55497d22433e 100644 --- a/packages/svelte/src/internal/client/dev/ownership.js +++ b/packages/svelte/src/internal/client/dev/ownership.js @@ -108,15 +108,16 @@ export function mark_module_end(component) { * @param {any} object * @param {any} owner * @param {boolean} [global] + * @param {boolean} [skip_warning] */ -export function add_owner(object, owner, global = false) { +export function add_owner(object, owner, global = false, skip_warning = false) { if (object && !global) { const component = dev_current_component_function; const metadata = object[STATE_SYMBOL]; if (metadata && !has_owner(metadata, component)) { let original = get_owner(metadata); - if (owner[FILENAME] !== component[FILENAME]) { + if (owner[FILENAME] !== component[FILENAME] && !skip_warning) { w.ownership_invalid_binding(component[FILENAME], owner[FILENAME], original[FILENAME]); } } @@ -128,10 +129,11 @@ export function add_owner(object, owner, global = false) { /** * @param {() => unknown} get_object * @param {any} Component + * @param {boolean} [skip_warning] */ -export function add_owner_effect(get_object, Component) { +export function add_owner_effect(get_object, Component, skip_warning = false) { user_pre_effect(() => { - add_owner(get_object(), Component); + add_owner(get_object(), Component, false, skip_warning); }); } diff --git a/packages/svelte/tests/runtime-runes/samples/ownership-invalid-binding-ignored/Child.svelte b/packages/svelte/tests/runtime-runes/samples/ownership-invalid-binding-ignored/Child.svelte new file mode 100644 index 000000000000..78b82caed9ed --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/ownership-invalid-binding-ignored/Child.svelte @@ -0,0 +1,5 @@ + + +{test} diff --git a/packages/svelte/tests/runtime-runes/samples/ownership-invalid-binding-ignored/Parent.svelte b/packages/svelte/tests/runtime-runes/samples/ownership-invalid-binding-ignored/Parent.svelte new file mode 100644 index 000000000000..1a724a3832df --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/ownership-invalid-binding-ignored/Parent.svelte @@ -0,0 +1,8 @@ + + + + diff --git a/packages/svelte/tests/runtime-runes/samples/ownership-invalid-binding-ignored/_config.js b/packages/svelte/tests/runtime-runes/samples/ownership-invalid-binding-ignored/_config.js new file mode 100644 index 000000000000..e93067eb9d34 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/ownership-invalid-binding-ignored/_config.js @@ -0,0 +1,11 @@ +import { test } from '../../test'; + +export default test({ + mode: ['client'], + compileOptions: { + dev: true + }, + async test({ warnings, assert }) { + assert.deepEqual(warnings, []); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/ownership-invalid-binding-ignored/main.svelte b/packages/svelte/tests/runtime-runes/samples/ownership-invalid-binding-ignored/main.svelte new file mode 100644 index 000000000000..e0da1dfcabaa --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/ownership-invalid-binding-ignored/main.svelte @@ -0,0 +1,6 @@ + + + diff --git a/packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/_expected/client/main.svelte.js b/packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/_expected/client/main.svelte.js index aa4d22d99a6d..e2951a7439f4 100644 --- a/packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/_expected/client/main.svelte.js +++ b/packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/_expected/client/main.svelte.js @@ -13,19 +13,19 @@ export default function Main($$anchor) { var custom_element = $.sibling($.sibling(svg, true)); var div_1 = $.sibling($.sibling(custom_element, true)); - $.template_effect(() => $.set_attribute(div_1, "foobar", y())); + $.template_effect(() => $.set_attribute(div_1, "foobar", y(), false)); var svg_1 = $.sibling($.sibling(div_1, true)); - $.template_effect(() => $.set_attribute(svg_1, "viewBox", y())); + $.template_effect(() => $.set_attribute(svg_1, "viewBox", y(), false)); var custom_element_1 = $.sibling($.sibling(svg_1, true)); $.template_effect(() => $.set_custom_element_data(custom_element_1, "fooBar", y())); $.template_effect(() => { - $.set_attribute(div, "foobar", x); - $.set_attribute(svg, "viewBox", x); + $.set_attribute(div, "foobar", x, false); + $.set_attribute(svg, "viewBox", x, false); $.set_custom_element_data(custom_element, "fooBar", x); }); From 3b7f27944af5462a2ed482b51b82846738f1b65b Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Mon, 29 Jul 2024 23:43:30 +0200 Subject: [PATCH 10/21] chore: revert extracting codes and use hardcoded list --- .../process-messages/templates/client-warnings.js | 2 -- .../process-messages/templates/shared-warnings.js | 2 -- .../src/compiler/utils/extract_svelte_ignore.js | 15 ++++++++++++--- packages/svelte/src/internal/client/warnings.js | 12 ------------ packages/svelte/src/internal/shared/warnings.js | 5 ----- 5 files changed, 12 insertions(+), 24 deletions(-) diff --git a/packages/svelte/scripts/process-messages/templates/client-warnings.js b/packages/svelte/scripts/process-messages/templates/client-warnings.js index 3d78466435fb..d40a59ec0c8d 100644 --- a/packages/svelte/scripts/process-messages/templates/client-warnings.js +++ b/packages/svelte/scripts/process-messages/templates/client-warnings.js @@ -15,5 +15,3 @@ export function CODE(PARAMETER) { console.warn('CODE'); } } - -export const codes = CODES; diff --git a/packages/svelte/scripts/process-messages/templates/shared-warnings.js b/packages/svelte/scripts/process-messages/templates/shared-warnings.js index 3d78466435fb..d40a59ec0c8d 100644 --- a/packages/svelte/scripts/process-messages/templates/shared-warnings.js +++ b/packages/svelte/scripts/process-messages/templates/shared-warnings.js @@ -15,5 +15,3 @@ export function CODE(PARAMETER) { console.warn('CODE'); } } - -export const codes = CODES; diff --git a/packages/svelte/src/compiler/utils/extract_svelte_ignore.js b/packages/svelte/src/compiler/utils/extract_svelte_ignore.js index 4939e269e412..df99cc067a6f 100644 --- a/packages/svelte/src/compiler/utils/extract_svelte_ignore.js +++ b/packages/svelte/src/compiler/utils/extract_svelte_ignore.js @@ -1,7 +1,5 @@ import fuzzymatch from '../phases/1-parse/utils/fuzzymatch.js'; import * as w from '../warnings.js'; -import { codes as client_codes } from '../../internal/client/warnings.js'; -import { codes as shared_codes } from '../../internal/shared/warnings.js'; const regex_svelte_ignore = /^\s*svelte-ignore\s/; @@ -18,7 +16,18 @@ const replacements = { 'unused-export-let': 'export_let_unused' }; -const codes = w.codes.concat(client_codes).concat(shared_codes); +// we use a list of ignorable runtime warnings because not every runtime warning +// can be ignored and we want to keep the validation for svelte-ignore in place +const ignorable_runtime_warnings = [ + 'state_snapshot_uncloneable', + 'binding_property_non_reactive', + 'hydration_attribute_changed', + 'hydration_html_changed', + 'ownership_invalid_binding', + 'ownership_invalid_mutation' +]; + +const codes = w.codes.concat(ignorable_runtime_warnings); /** * @param {number} offset diff --git a/packages/svelte/src/internal/client/warnings.js b/packages/svelte/src/internal/client/warnings.js index 1cb604284492..23c00f971dbb 100644 --- a/packages/svelte/src/internal/client/warnings.js +++ b/packages/svelte/src/internal/client/warnings.js @@ -5,18 +5,6 @@ import { DEV } from 'esm-env'; var bold = 'font-weight: bold'; var normal = 'font-weight: normal'; -export const codes = [ - "binding_property_non_reactive", - "hydration_attribute_changed", - "hydration_html_changed", - "hydration_mismatch", - "invalid_raw_snippet_render", - "lifecycle_double_unmount", - "ownership_invalid_binding", - "ownership_invalid_mutation", - "state_proxy_equality_mismatch" -]; - /** * `%binding%` (%location%) is binding to a non-reactive property * @param {string} binding diff --git a/packages/svelte/src/internal/shared/warnings.js b/packages/svelte/src/internal/shared/warnings.js index 7fd4a525f271..37269a674eb5 100644 --- a/packages/svelte/src/internal/shared/warnings.js +++ b/packages/svelte/src/internal/shared/warnings.js @@ -5,11 +5,6 @@ import { DEV } from 'esm-env'; var bold = 'font-weight: bold'; var normal = 'font-weight: normal'; -export const codes = [ - "dynamic_void_element_content", - "state_snapshot_uncloneable" -]; - /** * `` is a void element — it cannot have content * @param {string} tag From b3ec018fabce8ca187aa440dc35105d8d9689876 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Tue, 30 Jul 2024 00:03:32 +0200 Subject: [PATCH 11/21] chore: update changeset --- .changeset/large-emus-cough.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/large-emus-cough.md b/.changeset/large-emus-cough.md index 81e6d9b71443..b282b5faecaa 100644 --- a/.changeset/large-emus-cough.md +++ b/.changeset/large-emus-cough.md @@ -2,4 +2,4 @@ 'svelte': patch --- -feat: allow ignoring binding_property_non_reactive +feat: allow ignoring runtime warnings From ec3ae41e0305df0e0d98dda2cddea83d437bdfdd Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Tue, 30 Jul 2024 22:38:49 +0200 Subject: [PATCH 12/21] feat: allow skipping `ownership_invalid_mutation` --- .../phases/3-transform/client/utils.js | 70 +++++++++++++------ .../3-transform/client/visitors/global.js | 20 +++++- .../src/internal/client/dev/ownership.js | 12 ++++ packages/svelte/src/internal/client/index.js | 3 +- .../Child.svelte | 45 ++++++++++++ .../_config.js | 19 +++++ .../main.svelte | 9 +++ 7 files changed, 153 insertions(+), 25 deletions(-) create mode 100644 packages/svelte/tests/runtime-runes/samples/ownership-invalid-mutation-ignored/Child.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/ownership-invalid-mutation-ignored/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/ownership-invalid-mutation-ignored/main.svelte diff --git a/packages/svelte/src/compiler/phases/3-transform/client/utils.js b/packages/svelte/src/compiler/phases/3-transform/client/utils.js index 529318d4fc4d..c3e5eb0e9969 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/utils.js @@ -16,6 +16,7 @@ import { PROPS_IS_RUNES, PROPS_IS_UPDATED } from '../../../../constants.js'; +import { is_to_ignore } from '../../../state.js'; /** * @template {ClientTransformState} State @@ -281,6 +282,23 @@ export function serialize_set_binding(node, context, fallback, prefix, options) ); } + const ignore_invalid_mutation = + is_to_ignore(node, 'ownership_invalid_mutation') && context.state.options.dev; + + /** + * + * @param {any} serialized + * @returns + */ + function maybe_wrap_skip_ownership(serialized) { + if (!ignore_invalid_mutation) return serialized; + return b.call('$.skip_ownership_invalid_mutation', b.thunk(serialized)); + } + + if (binding.kind === 'derived') { + return maybe_wrap_skip_ownership(fallback()); + } + const is_store = binding.kind === 'store_sub'; const left_name = is_store ? left.name.slice(1) : left.name; @@ -381,11 +399,13 @@ 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_wrap_skip_ownership( + 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 || @@ -393,16 +413,20 @@ export function serialize_set_binding(node, context, fallback, prefix, options) (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_wrap_skip_ownership( + 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_wrap_skip_ownership( + b.call( + '$.mutate', + b.id(left_name), + b.assignment(node.operator, /** @type {Pattern} */ (visit(node.left)), value) + ) ); } } else if ( @@ -410,16 +434,20 @@ export function serialize_set_binding(node, context, fallback, prefix, options) prefix != null && (node.operator === '+=' || node.operator === '-=') ) { - return b.update( - node.operator === '+=' ? '++' : '--', - /** @type {Expression} */ (visit(node.left)), - prefix + return maybe_wrap_skip_ownership( + 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_wrap_skip_ownership( + b.assignment( + node.operator, + /** @type {Pattern} */ (visit(node.left)), + /** @type {Expression} */ (visit(node.right)) + ) ); } } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/global.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/global.js index 8d4698c6632d..3f819a7bc96c 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/global.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/global.js @@ -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_to_ignore } from '../../../../state.js'; /** @type {Visitors} */ export const global_visitors = { @@ -115,6 +116,19 @@ export const global_visitors = { return b.call(fn, ...args); } else { + const ignore_invalid_mutation = + is_to_ignore(node, 'ownership_invalid_mutation') && context.state.options.dev; + + /** + * + * @param {any} serialized + * @returns + */ + function maybe_wrap_skip_ownership(serialized) { + if (!ignore_invalid_mutation) return serialized; + return b.call('$.skip_ownership_invalid_mutation', b.thunk(serialized)); + } + // turn it into an IIFEE assignment expression: i++ -> (() => { const $$value = i; i+=1; return $$value; }) const assignment = b.assignment( node.operator === '++' ? '+=' : '-=', @@ -130,9 +144,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_wrap_skip_ownership(next()); } else if (context.state.analysis.runes) { - return serialized_assignment; + return maybe_wrap_skip_ownership(serialized_assignment); } else { /** @type {Statement[]} */ let statements; @@ -146,7 +160,7 @@ export const global_visitors = { b.return(b.id(tmp_id)) ]; } - return b.call(b.thunk(b.block(statements))); + return maybe_wrap_skip_ownership(b.call(b.thunk(b.block(statements)))); } } } diff --git a/packages/svelte/src/internal/client/dev/ownership.js b/packages/svelte/src/internal/client/dev/ownership.js index 55497d22433e..8867407cb09c 100644 --- a/packages/svelte/src/internal/client/dev/ownership.js +++ b/packages/svelte/src/internal/client/dev/ownership.js @@ -229,10 +229,22 @@ function get_owner(metadata) { ); } +let should_skip_ownership_invalid_mutation = false; + +/** + * @param {()=>any} fn + */ +export function skip_ownership_invalid_mutation(fn) { + should_skip_ownership_invalid_mutation = true; + fn(); + should_skip_ownership_invalid_mutation = false; +} + /** * @param {ProxyMetadata} metadata */ export function check_ownership(metadata) { + if (should_skip_ownership_invalid_mutation) return; const component = get_component(); if (component && !has_owner(metadata, component)) { diff --git a/packages/svelte/src/internal/client/index.js b/packages/svelte/src/internal/client/index.js index c70cb6674b65..8aa69c7e4dfa 100644 --- a/packages/svelte/src/internal/client/index.js +++ b/packages/svelte/src/internal/client/index.js @@ -6,7 +6,8 @@ export { add_owner, mark_module_start, mark_module_end, - add_owner_effect + add_owner_effect, + skip_ownership_invalid_mutation } from './dev/ownership.js'; export { check_target, legacy_api } from './dev/legacy.js'; export { inspect } from './dev/inspect.js'; diff --git a/packages/svelte/tests/runtime-runes/samples/ownership-invalid-mutation-ignored/Child.svelte b/packages/svelte/tests/runtime-runes/samples/ownership-invalid-mutation-ignored/Child.svelte new file mode 100644 index 000000000000..32bcef525056 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/ownership-invalid-mutation-ignored/Child.svelte @@ -0,0 +1,45 @@ + + + + + + + + + diff --git a/packages/svelte/tests/runtime-runes/samples/ownership-invalid-mutation-ignored/_config.js b/packages/svelte/tests/runtime-runes/samples/ownership-invalid-mutation-ignored/_config.js new file mode 100644 index 000000000000..44ac85aa2836 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/ownership-invalid-mutation-ignored/_config.js @@ -0,0 +1,19 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + mode: ['client'], + compileOptions: { + dev: true + }, + async test({ warnings, assert, target }) { + const [btn1, btn2, btn3, btn4] = target.querySelectorAll('button'); + flushSync(() => { + btn1.click(); + btn2.click(); + btn3.click(); + btn4.click(); + }); + assert.deepEqual(warnings, []); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/ownership-invalid-mutation-ignored/main.svelte b/packages/svelte/tests/runtime-runes/samples/ownership-invalid-mutation-ignored/main.svelte new file mode 100644 index 000000000000..b642dd32cfa1 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/ownership-invalid-mutation-ignored/main.svelte @@ -0,0 +1,9 @@ + + + From e21c4ccf60bfaf8657f90dbb194252399a390632 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 30 Jul 2024 18:58:38 -0400 Subject: [PATCH 13/21] is_to_ignore -> is_ignored --- .../compiler/phases/3-transform/client/utils.js | 4 ++-- .../phases/3-transform/client/visitors/global.js | 4 ++-- .../client/visitors/javascript-runes.js | 6 +++--- .../3-transform/client/visitors/template.js | 16 ++++++++-------- .../server/visitors/CallExpression.js | 4 ++-- packages/svelte/src/compiler/state.js | 2 +- 6 files changed, 18 insertions(+), 18 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/utils.js b/packages/svelte/src/compiler/phases/3-transform/client/utils.js index c3e5eb0e9969..5a49f69d4d08 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/utils.js @@ -16,7 +16,7 @@ import { PROPS_IS_RUNES, PROPS_IS_UPDATED } from '../../../../constants.js'; -import { is_to_ignore } from '../../../state.js'; +import { is_ignored } from '../../../state.js'; /** * @template {ClientTransformState} State @@ -283,7 +283,7 @@ export function serialize_set_binding(node, context, fallback, prefix, options) } const ignore_invalid_mutation = - is_to_ignore(node, 'ownership_invalid_mutation') && context.state.options.dev; + is_ignored(node, 'ownership_invalid_mutation') && context.state.options.dev; /** * diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/global.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/global.js index 3f819a7bc96c..cd12f71dff39 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/global.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/global.js @@ -3,7 +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_to_ignore } from '../../../../state.js'; +import { is_ignored } from '../../../../state.js'; /** @type {Visitors} */ export const global_visitors = { @@ -117,7 +117,7 @@ export const global_visitors = { return b.call(fn, ...args); } else { const ignore_invalid_mutation = - is_to_ignore(node, 'ownership_invalid_mutation') && context.state.options.dev; + is_ignored(node, 'ownership_invalid_mutation') && context.state.options.dev; /** * diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js index 164c749e3cd5..d8dfa7392146 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js @@ -1,7 +1,7 @@ /** @import { CallExpression, Expression, Identifier, Literal, MethodDefinition, PrivateIdentifier, PropertyDefinition, VariableDeclarator } from 'estree' */ /** @import { Binding } from '#compiler' */ /** @import { ComponentVisitors, StateField } from '../types.js' */ -import { is_to_ignore } from '../../../../state.js'; +import { 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'; @@ -203,7 +203,7 @@ export const javascript_visitors_runes = { b.call('$.get', b.member(b.this, b.private_id(name))), b.id('owner'), b.literal(false), - b.literal(is_to_ignore(node, 'ownership_invalid_binding')) + b.literal(is_ignored(node, 'ownership_invalid_binding')) ) ) ), @@ -455,7 +455,7 @@ export const javascript_visitors_runes = { return b.call( '$.snapshot', /** @type {Expression} */ (context.visit(node.arguments[0])), - b.literal(is_to_ignore(node, 'state_snapshot_uncloneable')) + b.literal(is_ignored(node, 'state_snapshot_uncloneable')) ); } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js index 324012f12f18..ab9f570ef05f 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js @@ -22,7 +22,7 @@ import { TRANSITION_OUT } from '../../../../../constants.js'; import { escape_html } from '../../../../../escaping.js'; -import { is_to_ignore, locator } from '../../../../state.js'; +import { is_ignored, locator } from '../../../../state.js'; import { extract_identifiers, extract_paths, @@ -325,7 +325,7 @@ function serialize_element_spread_attributes( b.object(values), lowercase_attributes, b.literal(context.state.analysis.css.hash), - b.literal(is_to_ignore(element, 'hydration_attribute_changed')) + b.literal(is_ignored(element, 'hydration_attribute_changed')) ) ) ); @@ -496,7 +496,7 @@ function serialize_element_attribute_update_assignment(element, node_id, attribu node_id, b.literal(name), value, - b.literal(is_to_ignore(element, 'hydration_attribute_changed')) + b.literal(is_ignored(element, 'hydration_attribute_changed')) ) ); @@ -540,7 +540,7 @@ function serialize_element_attribute_update_assignment(element, node_id, attribu node_id, b.literal(name), value, - b.literal(is_to_ignore(element, 'hydration_attribute_changed')) + b.literal(is_ignored(element, 'hydration_attribute_changed')) ) ); } @@ -804,7 +804,7 @@ function serialize_inline_component(node, component_name, context, anchor = cont // serialize_validate_binding will add a function that specifically throw // `binding_property_non_reactive` error. If there's a svelte ignore // before we avoid adding this validation to avoid throwing the runtime warning - !is_to_ignore(node, 'binding_property_non_reactive') + !is_ignored(node, 'binding_property_non_reactive') ) { context.state.init.push(serialize_validate_binding(context.state, attribute, expression)); } @@ -819,7 +819,7 @@ function serialize_inline_component(node, component_name, context, anchor = cont b.id('$.add_owner_effect'), b.thunk(expression), b.id(component_name), - b.literal(is_to_ignore(node, 'ownership_invalid_binding')) + b.literal(is_ignored(node, 'ownership_invalid_binding')) ) ) ); @@ -1846,7 +1846,7 @@ export const template_visitors = { b.thunk(/** @type {Expression} */ (context.visit(node.expression))), b.literal(context.state.metadata.namespace === 'svg'), b.literal(context.state.metadata.namespace === 'mathml'), - b.literal(is_to_ignore(node, 'hydration_html_changed')) + b.literal(is_ignored(node, 'hydration_html_changed')) ) ) ); @@ -2942,7 +2942,7 @@ export const template_visitors = { // serialize_validate_binding will add a function that specifically throw // `binding_property_non_reactive` error. If there's a svelte ignore // before we avoid adding this validation to avoid throwing the runtime warning - !is_to_ignore(node, 'binding_property_non_reactive') + !is_ignored(node, 'binding_property_non_reactive') ) { context.state.init.push( serialize_validate_binding( diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/CallExpression.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/CallExpression.js index 760a980a06e2..3abb1debb7be 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/CallExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/CallExpression.js @@ -1,6 +1,6 @@ /** @import { CallExpression, Expression } from 'estree' */ /** @import { Context } from '../types.js' */ -import { is_to_ignore } from '../../../../state.js'; +import { is_ignored } from '../../../../state.js'; import * as b from '../../../../utils/builders.js'; import { get_rune } from '../../../scope.js'; import { transform_inspect_rune } from '../../utils.js'; @@ -29,7 +29,7 @@ export function CallExpression(node, context) { return b.call( '$.snapshot', /** @type {Expression} */ (context.visit(node.arguments[0])), - b.literal(is_to_ignore(node, 'state_snapshot_uncloneable')) + b.literal(is_ignored(node, 'state_snapshot_uncloneable')) ); } diff --git a/packages/svelte/src/compiler/state.js b/packages/svelte/src/compiler/state.js index 37f82c744ce9..b71417da0f2d 100644 --- a/packages/svelte/src/compiler/state.js +++ b/packages/svelte/src/compiler/state.js @@ -65,7 +65,7 @@ export function reset_warning_filter(fn = () => true) { * @param {string} code * @returns */ -export function is_to_ignore(node, code) { +export function is_ignored(node, code) { return !!ignore_map.get(node)?.some((code_set) => code_set.has(code)); } From 7fda79a95848d259605eda56612536bcbb160633 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 30 Jul 2024 19:05:18 -0400 Subject: [PATCH 14/21] make is_ignored type safe --- packages/svelte/src/compiler/state.js | 4 ++-- .../src/compiler/utils/extract_svelte_ignore.js | 14 ++------------ packages/svelte/src/constants.js | 11 +++++++++++ 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/packages/svelte/src/compiler/state.js b/packages/svelte/src/compiler/state.js index b71417da0f2d..1da25549803a 100644 --- a/packages/svelte/src/compiler/state.js +++ b/packages/svelte/src/compiler/state.js @@ -62,11 +62,11 @@ export function reset_warning_filter(fn = () => true) { /** * * @param {SvelteNode | NodeLike} node - * @param {string} code + * @param {import('../constants.js').IGNORABLE_RUNTIME_WARNINGS[number]} code * @returns */ export function is_ignored(node, code) { - return !!ignore_map.get(node)?.some((code_set) => code_set.has(code)); + return !!ignore_map.get(node)?.some((codes) => codes.has(code)); } /** diff --git a/packages/svelte/src/compiler/utils/extract_svelte_ignore.js b/packages/svelte/src/compiler/utils/extract_svelte_ignore.js index df99cc067a6f..2f0d3873074c 100644 --- a/packages/svelte/src/compiler/utils/extract_svelte_ignore.js +++ b/packages/svelte/src/compiler/utils/extract_svelte_ignore.js @@ -1,3 +1,4 @@ +import { IGNORABLE_RUNTIME_WARNINGS } from '../../constants.js'; import fuzzymatch from '../phases/1-parse/utils/fuzzymatch.js'; import * as w from '../warnings.js'; @@ -16,18 +17,7 @@ const replacements = { 'unused-export-let': 'export_let_unused' }; -// we use a list of ignorable runtime warnings because not every runtime warning -// can be ignored and we want to keep the validation for svelte-ignore in place -const ignorable_runtime_warnings = [ - 'state_snapshot_uncloneable', - 'binding_property_non_reactive', - 'hydration_attribute_changed', - 'hydration_html_changed', - 'ownership_invalid_binding', - 'ownership_invalid_mutation' -]; - -const codes = w.codes.concat(ignorable_runtime_warnings); +const codes = w.codes.concat(IGNORABLE_RUNTIME_WARNINGS); /** * @param {number} offset diff --git a/packages/svelte/src/constants.js b/packages/svelte/src/constants.js index 0cc2102925e2..12b17ecb301f 100644 --- a/packages/svelte/src/constants.js +++ b/packages/svelte/src/constants.js @@ -199,6 +199,17 @@ const void_element_names = [ 'wbr' ]; +// we use a list of ignorable runtime warnings because not every runtime warning +// can be ignored and we want to keep the validation for svelte-ignore in place +export const IGNORABLE_RUNTIME_WARNINGS = /** @type {const} */ ([ + 'state_snapshot_uncloneable', + 'binding_property_non_reactive', + 'hydration_attribute_changed', + 'hydration_html_changed', + 'ownership_invalid_binding', + 'ownership_invalid_mutation' +]); + /** @param {string} name */ export function is_void(name) { return void_element_names.includes(name) || name.toLowerCase() === '!doctype'; From aa62e2adba1352b8d812c4d3640376e1a9cd54a0 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 30 Jul 2024 19:07:20 -0400 Subject: [PATCH 15/21] tweak --- .../src/compiler/phases/3-transform/client/utils.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/utils.js b/packages/svelte/src/compiler/phases/3-transform/client/utils.js index 5a49f69d4d08..493e9a6f152f 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/utils.js @@ -282,17 +282,16 @@ export function serialize_set_binding(node, context, fallback, prefix, options) ); } - const ignore_invalid_mutation = - is_ignored(node, 'ownership_invalid_mutation') && context.state.options.dev; - /** - * * @param {any} serialized * @returns */ function maybe_wrap_skip_ownership(serialized) { - if (!ignore_invalid_mutation) return serialized; - return b.call('$.skip_ownership_invalid_mutation', b.thunk(serialized)); + if (context.state.options.dev && is_ignored(node, 'ownership_invalid_mutation')) { + return b.call('$.skip_ownership_invalid_mutation', b.thunk(serialized)); + } + + return serialized; } if (binding.kind === 'derived') { From 02115ecf012eb0b058308770549b7fecd2478611 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 30 Jul 2024 19:14:43 -0400 Subject: [PATCH 16/21] tweak naming --- .../compiler/phases/3-transform/client/utils.js | 16 ++++++++-------- .../phases/3-transform/client/visitors/global.js | 2 +- .../svelte/src/internal/client/dev/ownership.js | 13 +++++++------ packages/svelte/src/internal/client/index.js | 2 +- 4 files changed, 17 insertions(+), 16 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/utils.js b/packages/svelte/src/compiler/phases/3-transform/client/utils.js index 493e9a6f152f..e0d974d23a6f 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/utils.js @@ -286,16 +286,16 @@ export function serialize_set_binding(node, context, fallback, prefix, options) * @param {any} serialized * @returns */ - function maybe_wrap_skip_ownership(serialized) { + function maybe_skip_ownership_validation(serialized) { if (context.state.options.dev && is_ignored(node, 'ownership_invalid_mutation')) { - return b.call('$.skip_ownership_invalid_mutation', b.thunk(serialized)); + return b.call('$.skip_ownership_validation', b.thunk(serialized)); } return serialized; } if (binding.kind === 'derived') { - return maybe_wrap_skip_ownership(fallback()); + return maybe_skip_ownership_validation(fallback()); } const is_store = binding.kind === 'store_sub'; @@ -398,7 +398,7 @@ export function serialize_set_binding(node, context, fallback, prefix, options) return /** @type {Expression} */ (visit(node)); } - return maybe_wrap_skip_ownership( + return maybe_skip_ownership_validation( b.call( '$.store_mutate', serialize_get_binding(b.id(left_name), state), @@ -412,7 +412,7 @@ export function serialize_set_binding(node, context, fallback, prefix, options) (binding.mutated && binding.kind === 'bindable_prop') ) { if (binding.kind === 'bindable_prop') { - return maybe_wrap_skip_ownership( + return maybe_skip_ownership_validation( b.call( left, b.assignment(node.operator, /** @type {Pattern} */ (visit(node.left)), value), @@ -420,7 +420,7 @@ export function serialize_set_binding(node, context, fallback, prefix, options) ) ); } else { - return maybe_wrap_skip_ownership( + return maybe_skip_ownership_validation( b.call( '$.mutate', b.id(left_name), @@ -433,7 +433,7 @@ export function serialize_set_binding(node, context, fallback, prefix, options) prefix != null && (node.operator === '+=' || node.operator === '-=') ) { - return maybe_wrap_skip_ownership( + return maybe_skip_ownership_validation( b.update( node.operator === '+=' ? '++' : '--', /** @type {Expression} */ (visit(node.left)), @@ -441,7 +441,7 @@ export function serialize_set_binding(node, context, fallback, prefix, options) ) ); } else { - return maybe_wrap_skip_ownership( + return maybe_skip_ownership_validation( b.assignment( node.operator, /** @type {Pattern} */ (visit(node.left)), diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/global.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/global.js index cd12f71dff39..bfc6b292c1b4 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/global.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/global.js @@ -126,7 +126,7 @@ export const global_visitors = { */ function maybe_wrap_skip_ownership(serialized) { if (!ignore_invalid_mutation) return serialized; - return b.call('$.skip_ownership_invalid_mutation', b.thunk(serialized)); + return b.call('$.skip_ownership_validation', b.thunk(serialized)); } // turn it into an IIFEE assignment expression: i++ -> (() => { const $$value = i; i+=1; return $$value; }) diff --git a/packages/svelte/src/internal/client/dev/ownership.js b/packages/svelte/src/internal/client/dev/ownership.js index 8867407cb09c..62d4a43e9411 100644 --- a/packages/svelte/src/internal/client/dev/ownership.js +++ b/packages/svelte/src/internal/client/dev/ownership.js @@ -229,22 +229,23 @@ function get_owner(metadata) { ); } -let should_skip_ownership_invalid_mutation = false; +let skip = false; /** - * @param {()=>any} fn + * @param {() => any} fn */ -export function skip_ownership_invalid_mutation(fn) { - should_skip_ownership_invalid_mutation = true; +export function skip_ownership_validation(fn) { + skip = true; fn(); - should_skip_ownership_invalid_mutation = false; + skip = false; } /** * @param {ProxyMetadata} metadata */ export function check_ownership(metadata) { - if (should_skip_ownership_invalid_mutation) return; + if (skip) return; + const component = get_component(); if (component && !has_owner(metadata, component)) { diff --git a/packages/svelte/src/internal/client/index.js b/packages/svelte/src/internal/client/index.js index 8aa69c7e4dfa..edd50bff32d4 100644 --- a/packages/svelte/src/internal/client/index.js +++ b/packages/svelte/src/internal/client/index.js @@ -7,7 +7,7 @@ export { mark_module_start, mark_module_end, add_owner_effect, - skip_ownership_invalid_mutation + skip_ownership_validation } from './dev/ownership.js'; export { check_target, legacy_api } from './dev/legacy.js'; export { inspect } from './dev/inspect.js'; From 4e20b24a9247339dc8ebcadbaa4b0a7de1fb450c Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 30 Jul 2024 19:17:52 -0400 Subject: [PATCH 17/21] tweak --- .../3-transform/client/visitors/global.js | 22 ++++++++----------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/global.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/global.js index bfc6b292c1b4..3b677a1ce97f 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/global.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/global.js @@ -116,17 +116,13 @@ export const global_visitors = { return b.call(fn, ...args); } else { - const ignore_invalid_mutation = - is_ignored(node, 'ownership_invalid_mutation') && context.state.options.dev; + /** @param {any} serialized */ + function maybe_skip_ownership_validation(serialized) { + if (context.state.options.dev && is_ignored(node, 'ownership_invalid_mutation')) { + return b.call('$.skip_ownership_validation', b.thunk(serialized)); + } - /** - * - * @param {any} serialized - * @returns - */ - function maybe_wrap_skip_ownership(serialized) { - if (!ignore_invalid_mutation) return serialized; - 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; }) @@ -144,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 maybe_wrap_skip_ownership(next()); + return maybe_skip_ownership_validation(next()); } else if (context.state.analysis.runes) { - return maybe_wrap_skip_ownership(serialized_assignment); + return maybe_skip_ownership_validation(serialized_assignment); } else { /** @type {Statement[]} */ let statements; @@ -160,7 +156,7 @@ export const global_visitors = { b.return(b.id(tmp_id)) ]; } - return maybe_wrap_skip_ownership(b.call(b.thunk(b.block(statements)))); + return maybe_skip_ownership_validation(b.call(b.thunk(b.block(statements)))); } } } From c12fe445d0babe94cc5f973ff6c4f05e81b2ca43 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 30 Jul 2024 19:25:23 -0400 Subject: [PATCH 18/21] remove extra args --- .../phases/3-transform/client/visitors/javascript-runes.js | 4 ++-- .../compiler/phases/3-transform/client/visitors/template.js | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js index d8dfa7392146..8e5155e74a07 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js @@ -203,7 +203,7 @@ export const javascript_visitors_runes = { b.call('$.get', b.member(b.this, b.private_id(name))), b.id('owner'), b.literal(false), - b.literal(is_ignored(node, 'ownership_invalid_binding')) + is_ignored(node, 'ownership_invalid_binding') && b.true ) ) ), @@ -455,7 +455,7 @@ export const javascript_visitors_runes = { return b.call( '$.snapshot', /** @type {Expression} */ (context.visit(node.arguments[0])), - b.literal(is_ignored(node, 'state_snapshot_uncloneable')) + is_ignored(node, 'state_snapshot_uncloneable') && b.true ); } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js index ab9f570ef05f..9587ae8b3dbb 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js @@ -325,7 +325,7 @@ function serialize_element_spread_attributes( b.object(values), lowercase_attributes, b.literal(context.state.analysis.css.hash), - b.literal(is_ignored(element, 'hydration_attribute_changed')) + is_ignored(element, 'hydration_attribute_changed') && b.true ) ) ); @@ -496,7 +496,7 @@ function serialize_element_attribute_update_assignment(element, node_id, attribu node_id, b.literal(name), value, - b.literal(is_ignored(element, 'hydration_attribute_changed')) + is_ignored(element, 'hydration_attribute_changed') && b.true ) ); @@ -540,7 +540,7 @@ function serialize_element_attribute_update_assignment(element, node_id, attribu node_id, b.literal(name), value, - b.literal(is_ignored(element, 'hydration_attribute_changed')) + is_ignored(element, 'hydration_attribute_changed') && b.true ) ); } From 19000d11c09d245e8158994cd23bf896ad55dd06 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 30 Jul 2024 19:25:48 -0400 Subject: [PATCH 19/21] comment is redundant, code contains enough information --- .../compiler/phases/3-transform/client/visitors/template.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js index 9587ae8b3dbb..cf7d5d19e98a 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js @@ -801,9 +801,6 @@ function serialize_inline_component(node, component_name, context, anchor = cont expression.type === 'MemberExpression' && context.state.options.dev && context.state.analysis.runes && - // serialize_validate_binding will add a function that specifically throw - // `binding_property_non_reactive` error. If there's a svelte ignore - // before we avoid adding this validation to avoid throwing the runtime warning !is_ignored(node, 'binding_property_non_reactive') ) { context.state.init.push(serialize_validate_binding(context.state, attribute, expression)); From f85d49108002832149ae32bfa0f028383ba1673c Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 30 Jul 2024 19:28:38 -0400 Subject: [PATCH 20/21] remove more unwanted args --- .../phases/3-transform/client/visitors/template.js | 7 ++----- .../phases/3-transform/server/visitors/CallExpression.js | 2 +- .../_expected/client/main.svelte.js | 8 ++++---- 3 files changed, 7 insertions(+), 10 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js index cf7d5d19e98a..a91f483e1429 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js @@ -816,7 +816,7 @@ function serialize_inline_component(node, component_name, context, anchor = cont b.id('$.add_owner_effect'), b.thunk(expression), b.id(component_name), - b.literal(is_ignored(node, 'ownership_invalid_binding')) + is_ignored(node, 'ownership_invalid_binding') && b.true ) ) ); @@ -1843,7 +1843,7 @@ export const template_visitors = { b.thunk(/** @type {Expression} */ (context.visit(node.expression))), b.literal(context.state.metadata.namespace === 'svg'), b.literal(context.state.metadata.namespace === 'mathml'), - b.literal(is_ignored(node, 'hydration_html_changed')) + is_ignored(node, 'hydration_html_changed') && b.true ) ) ); @@ -2936,9 +2936,6 @@ export const template_visitors = { )) && context.state.options.dev && context.state.analysis.runes && - // serialize_validate_binding will add a function that specifically throw - // `binding_property_non_reactive` error. If there's a svelte ignore - // before we avoid adding this validation to avoid throwing the runtime warning !is_ignored(node, 'binding_property_non_reactive') ) { context.state.init.push( diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/CallExpression.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/CallExpression.js index 3abb1debb7be..4b15a772c9e8 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/CallExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/CallExpression.js @@ -29,7 +29,7 @@ export function CallExpression(node, context) { return b.call( '$.snapshot', /** @type {Expression} */ (context.visit(node.arguments[0])), - b.literal(is_ignored(node, 'state_snapshot_uncloneable')) + is_ignored(node, 'state_snapshot_uncloneable') && b.true ); } diff --git a/packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/_expected/client/main.svelte.js b/packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/_expected/client/main.svelte.js index e2951a7439f4..aa4d22d99a6d 100644 --- a/packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/_expected/client/main.svelte.js +++ b/packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/_expected/client/main.svelte.js @@ -13,19 +13,19 @@ export default function Main($$anchor) { var custom_element = $.sibling($.sibling(svg, true)); var div_1 = $.sibling($.sibling(custom_element, true)); - $.template_effect(() => $.set_attribute(div_1, "foobar", y(), false)); + $.template_effect(() => $.set_attribute(div_1, "foobar", y())); var svg_1 = $.sibling($.sibling(div_1, true)); - $.template_effect(() => $.set_attribute(svg_1, "viewBox", y(), false)); + $.template_effect(() => $.set_attribute(svg_1, "viewBox", y())); var custom_element_1 = $.sibling($.sibling(svg_1, true)); $.template_effect(() => $.set_custom_element_data(custom_element_1, "fooBar", y())); $.template_effect(() => { - $.set_attribute(div, "foobar", x, false); - $.set_attribute(svg, "viewBox", x, false); + $.set_attribute(div, "foobar", x); + $.set_attribute(svg, "viewBox", x); $.set_custom_element_data(custom_element, "fooBar", x); }); From 2aad26540e3a187e2934ca65e436d6bc2ddbaa1c Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 30 Jul 2024 22:41:04 -0400 Subject: [PATCH 21/21] lint --- .../phases/3-transform/client/visitors/javascript-runes.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js index 7688ed3dd8e1..77c889ac8e7d 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js @@ -1,7 +1,7 @@ /** @import { CallExpression, Expression, Identifier, Literal, MethodDefinition, PrivateIdentifier, PropertyDefinition, VariableDeclarator } from 'estree' */ /** @import { Binding } from '#compiler' */ /** @import { ComponentVisitors, StateField } from '../types.js' */ -import { is_ignored } from '../../../../state.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'; @@ -15,7 +15,6 @@ import { serialize_proxy_reassignment, should_proxy_or_freeze } from '../utils.js'; -import { dev } from '../../../../state.js'; /** @type {ComponentVisitors} */ export const javascript_visitors_runes = {