From 7d5bed27b59b2f84e2b4443c682431159169723c Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 20 Sep 2024 10:12:40 -0400 Subject: [PATCH 01/15] WIP --- .../client/visitors/RegularElement.js | 90 ++++++++++++------- 1 file changed, 56 insertions(+), 34 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js index 3b3888cf5b7e..09fb3a5b3d37 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js @@ -83,17 +83,13 @@ export function RegularElement(node, context) { const lets = []; const is_custom_element = is_custom_element_node(node); - let needs_input_reset = false; let needs_content_reset = false; /** @type {AST.BindDirective | null} */ let value_binding = null; - /** If true, needs `__value` for inputs */ - let needs_special_value_handling = node.name === 'option' || node.name === 'select'; let is_content_editable = false; let has_content_editable_binding = false; - let img_might_be_lazy = false; let might_need_event_replaying = false; let has_direction_attribute = false; let has_style_attribute = false; @@ -113,12 +109,16 @@ export function RegularElement(node, context) { } } + /** @type {Set} */ + const names = new Set(); + + /** @type {Set} */ + const bindings = new Set(); + for (const attribute of node.attributes) { if (attribute.type === 'Attribute') { - attributes.push(attribute); - if (node.name === 'img' && attribute.name === 'loading') { - img_might_be_lazy = true; - } + names.add(attribute.name); + if (attribute.name === 'dir') { has_direction_attribute = true; } @@ -129,7 +129,6 @@ export function RegularElement(node, context) { (attribute.name === 'value' || attribute.name === 'checked') && !is_text_attribute(attribute) ) { - needs_input_reset = true; needs_content_reset = true; } else if ( attribute.name === 'contenteditable' && @@ -139,12 +138,35 @@ export function RegularElement(node, context) { is_content_editable = true; } } else if (attribute.type === 'SpreadAttribute') { - attributes.push(attribute); - needs_input_reset = true; + names.add('*'); needs_content_reset = true; if (is_load_error_element(node.name)) { might_need_event_replaying = true; } + } + if (attribute.type === 'BindDirective') { + bindings.add(attribute.name); + + if (attribute.name === 'value') { + value_binding = attribute; + needs_content_reset = true; + } else if ( + attribute.name === 'innerHTML' || + attribute.name === 'innerText' || + attribute.name === 'textContent' + ) { + has_content_editable_binding = true; + } + } else if (attribute.type === 'UseDirective' && is_load_error_element(node.name)) { + might_need_event_replaying = true; + } + } + + for (const attribute of node.attributes) { + if (attribute.type === 'Attribute') { + attributes.push(attribute); + } else if (attribute.type === 'SpreadAttribute') { + attributes.push(attribute); } else if (attribute.type === 'ClassDirective') { class_directives.push(attribute); } else if (attribute.type === 'StyleDirective') { @@ -157,24 +179,6 @@ export function RegularElement(node, context) { b.stmt(has_action_directive ? b.call('$.effect', b.thunk(handler)) : handler) ); } else if (attribute.type !== 'LetDirective') { - if (attribute.type === 'BindDirective') { - if (attribute.name === 'group' || attribute.name === 'checked') { - needs_special_value_handling = true; - needs_input_reset = true; - } else if (attribute.name === 'value') { - value_binding = attribute; - needs_content_reset = true; - needs_input_reset = true; - } else if ( - attribute.name === 'innerHTML' || - attribute.name === 'innerText' || - attribute.name === 'textContent' - ) { - has_content_editable_binding = true; - } - } else if (attribute.type === 'UseDirective' && is_load_error_element(node.name)) { - might_need_event_replaying = true; - } context.visit(attribute); } } @@ -183,7 +187,21 @@ export function RegularElement(node, context) { child_metadata.bound_contenteditable = true; } - if (needs_input_reset && node.name === 'input') { + if ( + node.name === 'input' && + node.attributes.some((attribute) => { + return ( + attribute.type === 'SpreadAttribute' || + (attribute.type === 'Attribute' && + (attribute.name === 'value' || attribute.name === 'checked') && + !is_text_attribute(attribute)) || + (attribute.type === 'BindDirective' && + (attribute.name === 'group' || + attribute.name === 'checked' || + attribute.name === 'value')) + ); + }) + ) { context.state.init.push(b.stmt(b.call('$.remove_input_defaults', context.state.node))); } @@ -203,9 +221,6 @@ export function RegularElement(node, context) { // Then do attributes let is_attributes_reactive = false; if (node.metadata.has_spread) { - if (node.name === 'img') { - img_might_be_lazy = true; - } build_element_spread_attributes( attributes, context, @@ -216,6 +231,13 @@ export function RegularElement(node, context) { ); is_attributes_reactive = true; } else { + /** If true, needs `__value` for inputs */ + const needs_special_value_handling = + node.name === 'option' || + node.name === 'select' || + bindings.has('group') || + bindings.has('checked'); + for (const attribute of /** @type {AST.Attribute[]} */ (attributes)) { if (is_event_attribute(attribute)) { if ( @@ -261,7 +283,7 @@ export function RegularElement(node, context) { } // Apply the src and loading attributes for elements after the element is appended to the document - if (img_might_be_lazy) { + if (node.name === 'img' && (names.has('*') || names.has('loading'))) { context.state.after_update.push(b.stmt(b.call('$.handle_lazy_img', node_id))); } From 9f38ae593e856449152a5515f8e7c06589a9d98a Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 20 Sep 2024 10:34:03 -0400 Subject: [PATCH 02/15] tidy --- .../client/visitors/RegularElement.js | 33 ++++++------------- 1 file changed, 10 insertions(+), 23 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js index 09fb3a5b3d37..186c180f7949 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js @@ -89,8 +89,6 @@ export function RegularElement(node, context) { let value_binding = null; let is_content_editable = false; - let has_content_editable_binding = false; - let might_need_event_replaying = false; let has_direction_attribute = false; let has_style_attribute = false; @@ -140,9 +138,6 @@ export function RegularElement(node, context) { } else if (attribute.type === 'SpreadAttribute') { names.add('*'); needs_content_reset = true; - if (is_load_error_element(node.name)) { - might_need_event_replaying = true; - } } if (attribute.type === 'BindDirective') { bindings.add(attribute.name); @@ -150,15 +145,7 @@ export function RegularElement(node, context) { if (attribute.name === 'value') { value_binding = attribute; needs_content_reset = true; - } else if ( - attribute.name === 'innerHTML' || - attribute.name === 'innerText' || - attribute.name === 'textContent' - ) { - has_content_editable_binding = true; } - } else if (attribute.type === 'UseDirective' && is_load_error_element(node.name)) { - might_need_event_replaying = true; } } @@ -183,9 +170,9 @@ export function RegularElement(node, context) { } } - if (is_content_editable && has_content_editable_binding) { - child_metadata.bound_contenteditable = true; - } + child_metadata.bound_contenteditable = + is_content_editable && + (bindings.has('innerHTML') || bindings.has('innerText') || bindings.has('textContent')); if ( node.name === 'input' && @@ -240,12 +227,6 @@ export function RegularElement(node, context) { for (const attribute of /** @type {AST.Attribute[]} */ (attributes)) { if (is_event_attribute(attribute)) { - if ( - (attribute.name === 'onload' || attribute.name === 'onerror') && - is_load_error_element(node.name) - ) { - might_need_event_replaying = true; - } visit_event_attribute(attribute, context); continue; } @@ -297,7 +278,13 @@ export function RegularElement(node, context) { has_style_attribute || node.metadata.has_spread ); - if (might_need_event_replaying) { + if ( + is_load_error_element(node.name) && + (names.has('*') || + names.has('onload') || + names.has('onerror') || + node.attributes.some((attribute) => attribute.type === 'UseDirective')) + ) { context.state.after_update.push(b.stmt(b.call('$.replay_events', node_id))); } From 21bea466b0bf2f54a7259066c3bb9d2de6982763 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 20 Sep 2024 10:35:22 -0400 Subject: [PATCH 03/15] simplify --- .../3-transform/client/visitors/RegularElement.js | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js index 186c180f7949..ddeba33f2846 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js @@ -89,8 +89,6 @@ export function RegularElement(node, context) { let value_binding = null; let is_content_editable = false; - let has_direction_attribute = false; - let has_style_attribute = false; if (is_custom_element) { // cloneNode is faster, but it does not instantiate the underlying class of the @@ -117,12 +115,6 @@ export function RegularElement(node, context) { if (attribute.type === 'Attribute') { names.add(attribute.name); - if (attribute.name === 'dir') { - has_direction_attribute = true; - } - if (attribute.name === 'style') { - has_style_attribute = true; - } if ( (attribute.name === 'value' || attribute.name === 'checked') && !is_text_attribute(attribute) @@ -275,7 +267,7 @@ export function RegularElement(node, context) { node_id, context, is_attributes_reactive, - has_style_attribute || node.metadata.has_spread + names.has('style') || node.metadata.has_spread ); if ( @@ -380,7 +372,7 @@ export function RegularElement(node, context) { context.state.after_update.push(...child_state.after_update); } - if (has_direction_attribute) { + if (names.has('dir')) { // This fixes an issue with Chromium where updates to text content within an element // does not update the direction when set to auto. If we just re-assign the dir, this fixes it. const dir = b.member(node_id, 'dir'); From 0c34327e19198967c8b19fb428858b4f68c19ef5 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 20 Sep 2024 10:46:49 -0400 Subject: [PATCH 04/15] simplify --- .../client/visitors/RegularElement.js | 46 ++++++++----------- 1 file changed, 20 insertions(+), 26 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js index ddeba33f2846..28278e79d75d 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js @@ -85,9 +85,6 @@ export function RegularElement(node, context) { const is_custom_element = is_custom_element_node(node); let needs_content_reset = false; - /** @type {AST.BindDirective | null} */ - let value_binding = null; - let is_content_editable = false; if (is_custom_element) { @@ -105,15 +102,17 @@ export function RegularElement(node, context) { } } - /** @type {Set} */ - const names = new Set(); + /** @type {Map} */ + const lookup = new Map(); + + /** @type {Map} */ + const bindings = new Map(); - /** @type {Set} */ - const bindings = new Set(); + let has_spread = false; for (const attribute of node.attributes) { if (attribute.type === 'Attribute') { - names.add(attribute.name); + lookup.set(attribute.name, attribute); if ( (attribute.name === 'value' || attribute.name === 'checked') && @@ -128,16 +127,11 @@ export function RegularElement(node, context) { is_content_editable = true; } } else if (attribute.type === 'SpreadAttribute') { - names.add('*'); - needs_content_reset = true; + has_spread = true; } - if (attribute.type === 'BindDirective') { - bindings.add(attribute.name); - if (attribute.name === 'value') { - value_binding = attribute; - needs_content_reset = true; - } + if (attribute.type === 'BindDirective') { + bindings.set(attribute.name, attribute); } } @@ -184,12 +178,12 @@ export function RegularElement(node, context) { context.state.init.push(b.stmt(b.call('$.remove_input_defaults', context.state.node))); } - if (needs_content_reset && node.name === 'textarea') { + if (node.name === 'textarea' && (has_spread || bindings.has('value') || needs_content_reset)) { context.state.init.push(b.stmt(b.call('$.remove_textarea_child', context.state.node))); } - if (value_binding !== null && node.name === 'select') { - setup_select_synchronization(value_binding, context); + if (node.name === 'select' && bindings.has('value')) { + setup_select_synchronization(/** @type {AST.BindDirective} */ (bindings.get('value')), context); } const node_id = context.state.node; @@ -206,7 +200,7 @@ export function RegularElement(node, context) { node, node_id, // If value binding exists, that one takes care of calling $.init_select - value_binding === null && node.name === 'select' + node.name === 'select' && !bindings.has('value') ); is_attributes_reactive = true; } else { @@ -256,7 +250,7 @@ export function RegularElement(node, context) { } // Apply the src and loading attributes for elements after the element is appended to the document - if (node.name === 'img' && (names.has('*') || names.has('loading'))) { + if (node.name === 'img' && (has_spread || lookup.has('loading'))) { context.state.after_update.push(b.stmt(b.call('$.handle_lazy_img', node_id))); } @@ -267,14 +261,14 @@ export function RegularElement(node, context) { node_id, context, is_attributes_reactive, - names.has('style') || node.metadata.has_spread + lookup.has('style') || node.metadata.has_spread ); if ( is_load_error_element(node.name) && - (names.has('*') || - names.has('onload') || - names.has('onerror') || + (has_spread || + lookup.has('onload') || + lookup.has('onerror') || node.attributes.some((attribute) => attribute.type === 'UseDirective')) ) { context.state.after_update.push(b.stmt(b.call('$.replay_events', node_id))); @@ -372,7 +366,7 @@ export function RegularElement(node, context) { context.state.after_update.push(...child_state.after_update); } - if (names.has('dir')) { + if (lookup.has('dir')) { // This fixes an issue with Chromium where updates to text content within an element // does not update the direction when set to auto. If we just re-assign the dir, this fixes it. const dir = b.member(node_id, 'dir'); From 702226dee8deaaee3caa00324ce61e5777a8a083 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 20 Sep 2024 10:49:45 -0400 Subject: [PATCH 05/15] more --- .../client/visitors/RegularElement.js | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js index 28278e79d75d..822c49ca1232 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js @@ -85,8 +85,6 @@ export function RegularElement(node, context) { const is_custom_element = is_custom_element_node(node); let needs_content_reset = false; - let is_content_editable = false; - if (is_custom_element) { // cloneNode is faster, but it does not instantiate the underlying class of the // custom element until the template is connected to the dom, which would @@ -119,12 +117,6 @@ export function RegularElement(node, context) { !is_text_attribute(attribute) ) { needs_content_reset = true; - } else if ( - attribute.name === 'contenteditable' && - (attribute.value === true || - (is_text_attribute(attribute) && attribute.value[0].data === 'true')) - ) { - is_content_editable = true; } } else if (attribute.type === 'SpreadAttribute') { has_spread = true; @@ -156,9 +148,17 @@ export function RegularElement(node, context) { } } - child_metadata.bound_contenteditable = - is_content_editable && - (bindings.has('innerHTML') || bindings.has('innerText') || bindings.has('textContent')); + if (bindings.has('innerHTML') || bindings.has('innerText') || bindings.has('textContent')) { + const contenteditable = lookup.get('contenteditable'); + + if ( + contenteditable && + (contenteditable.value === true || + (is_text_attribute(contenteditable) && contenteditable.value[0].data === 'true')) + ) { + child_metadata.bound_contenteditable = true; + } + } if ( node.name === 'input' && From c76ca70621abcc4e8cfcde74bf94b0d12f28621c Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 20 Sep 2024 10:56:11 -0400 Subject: [PATCH 06/15] more --- .../client/visitors/RegularElement.js | 37 +++++++------------ 1 file changed, 14 insertions(+), 23 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js index 822c49ca1232..5898e3fd56de 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js @@ -83,7 +83,6 @@ export function RegularElement(node, context) { const lets = []; const is_custom_element = is_custom_element_node(node); - let needs_content_reset = false; if (is_custom_element) { // cloneNode is faster, but it does not instantiate the underlying class of the @@ -93,13 +92,6 @@ export function RegularElement(node, context) { metadata.context.template_needs_import_node = true; } - // visit let directives first, to set state - for (const attribute of node.attributes) { - if (attribute.type === 'LetDirective') { - lets.push(/** @type {ExpressionStatement} */ (context.visit(attribute))); - } - } - /** @type {Map} */ const lookup = new Map(); @@ -109,21 +101,15 @@ export function RegularElement(node, context) { let has_spread = false; for (const attribute of node.attributes) { - if (attribute.type === 'Attribute') { - lookup.set(attribute.name, attribute); - - if ( - (attribute.name === 'value' || attribute.name === 'checked') && - !is_text_attribute(attribute) - ) { - needs_content_reset = true; - } - } else if (attribute.type === 'SpreadAttribute') { + if (attribute.type === 'SpreadAttribute') { has_spread = true; - } - - if (attribute.type === 'BindDirective') { + } else if (attribute.type === 'Attribute') { + lookup.set(attribute.name, attribute); + } else if (attribute.type === 'BindDirective') { bindings.set(attribute.name, attribute); + } else if (attribute.type === 'LetDirective') { + // visit let directives before everything else, to set state + lets.push(/** @type {ExpressionStatement} */ (context.visit(attribute))); } } @@ -178,8 +164,13 @@ export function RegularElement(node, context) { context.state.init.push(b.stmt(b.call('$.remove_input_defaults', context.state.node))); } - if (node.name === 'textarea' && (has_spread || bindings.has('value') || needs_content_reset)) { - context.state.init.push(b.stmt(b.call('$.remove_textarea_child', context.state.node))); + if (node.name === 'textarea') { + const attribute = lookup.get('value') ?? lookup.get('checked'); + const needs_content_reset = attribute && !is_text_attribute(attribute); + + if (has_spread || bindings.has('value') || needs_content_reset) { + context.state.init.push(b.stmt(b.call('$.remove_textarea_child', context.state.node))); + } } if (node.name === 'select' && bindings.has('value')) { From 822fe6b6d493fb493b7ce62b24caa6fb4b2f6154 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 20 Sep 2024 11:14:58 -0400 Subject: [PATCH 07/15] use a switch --- .../client/visitors/RegularElement.js | 80 ++++++++++++------- 1 file changed, 52 insertions(+), 28 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js index 5898e3fd56de..6e5c1a03e923 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js @@ -62,7 +62,6 @@ export function RegularElement(node, context) { context.state.metadata.context.template_contains_script_tag = true; } - const metadata = context.state.metadata; const child_metadata = { ...context.state.metadata, namespace: determine_namespace_for_children(node, context.state.metadata.namespace) @@ -79,6 +78,9 @@ export function RegularElement(node, context) { /** @type {AST.StyleDirective[]} */ const style_directives = []; + /** @type {Array} */ @@ -99,37 +101,62 @@ export function RegularElement(node, context) { const bindings = new Map(); let has_spread = false; + let has_use = false; for (const attribute of node.attributes) { - if (attribute.type === 'SpreadAttribute') { - has_spread = true; - } else if (attribute.type === 'Attribute') { - lookup.set(attribute.name, attribute); - } else if (attribute.type === 'BindDirective') { - bindings.set(attribute.name, attribute); - } else if (attribute.type === 'LetDirective') { - // visit let directives before everything else, to set state - lets.push(/** @type {ExpressionStatement} */ (context.visit(attribute))); + switch (attribute.type) { + case 'SpreadAttribute': + attributes.push(attribute); + has_spread = true; + break; + + case 'Attribute': + attributes.push(attribute); + lookup.set(attribute.name, attribute); + break; + + case 'BindDirective': + bindings.set(attribute.name, attribute); + other_directives.push(attribute); + break; + + case 'ClassDirective': + class_directives.push(attribute); + break; + + case 'StyleDirective': + style_directives.push(attribute); + break; + + case 'LetDirective': + // visit let directives before everything else, to set state + lets.push(/** @type {ExpressionStatement} */ (context.visit(attribute))); + break; + + case 'UseDirective': + has_use = true; + other_directives.push(attribute); + break; + + case 'OnDirective': + other_directives.push(attribute); + break; + + case 'AnimateDirective': + case 'TransitionDirective': + other_directives.push(attribute); + break; } } - for (const attribute of node.attributes) { - if (attribute.type === 'Attribute') { - attributes.push(attribute); - } else if (attribute.type === 'SpreadAttribute') { - attributes.push(attribute); - } else if (attribute.type === 'ClassDirective') { - class_directives.push(attribute); - } else if (attribute.type === 'StyleDirective') { - style_directives.push(attribute); - } else if (attribute.type === 'OnDirective') { + for (const attribute of other_directives) { + if (attribute.type === 'OnDirective') { const handler = /** @type {Expression} */ (context.visit(attribute)); - const has_action_directive = node.attributes.find((a) => a.type === 'UseDirective'); context.state.after_update.push( - b.stmt(has_action_directive ? b.call('$.effect', b.thunk(handler)) : handler) + b.stmt(has_use ? b.call('$.effect', b.thunk(handler)) : handler) ); - } else if (attribute.type !== 'LetDirective') { + } else { context.visit(attribute); } } @@ -257,10 +284,7 @@ export function RegularElement(node, context) { if ( is_load_error_element(node.name) && - (has_spread || - lookup.has('onload') || - lookup.has('onerror') || - node.attributes.some((attribute) => attribute.type === 'UseDirective')) + (has_spread || has_use || lookup.has('onload') || lookup.has('onerror')) ) { context.state.after_update.push(b.stmt(b.call('$.replay_events', node_id))); } From 8865a9976270acd51f5c3dae925fc27efaa59eef Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 20 Sep 2024 11:17:10 -0400 Subject: [PATCH 08/15] alphabetize --- .../client/visitors/RegularElement.js | 27 ++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js index 6e5c1a03e923..bb7b21493f6c 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js @@ -105,9 +105,8 @@ export function RegularElement(node, context) { for (const attribute of node.attributes) { switch (attribute.type) { - case 'SpreadAttribute': - attributes.push(attribute); - has_spread = true; + case 'AnimateDirective': + other_directives.push(attribute); break; case 'Attribute': @@ -124,28 +123,32 @@ export function RegularElement(node, context) { class_directives.push(attribute); break; - case 'StyleDirective': - style_directives.push(attribute); - break; - case 'LetDirective': // visit let directives before everything else, to set state lets.push(/** @type {ExpressionStatement} */ (context.visit(attribute))); break; - case 'UseDirective': - has_use = true; + case 'OnDirective': other_directives.push(attribute); break; - case 'OnDirective': - other_directives.push(attribute); + case 'SpreadAttribute': + attributes.push(attribute); + has_spread = true; + break; + + case 'StyleDirective': + style_directives.push(attribute); break; - case 'AnimateDirective': case 'TransitionDirective': other_directives.push(attribute); break; + + case 'UseDirective': + has_use = true; + other_directives.push(attribute); + break; } } From c3501f8191262c5ee0b7c084d2958617fedc4fff Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 20 Sep 2024 11:19:44 -0400 Subject: [PATCH 09/15] simplify --- .../phases/3-transform/client/visitors/RegularElement.js | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js index bb7b21493f6c..3e26a6bbf515 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js @@ -294,14 +294,11 @@ export function RegularElement(node, context) { context.state.template.push('>'); - /** @type {SourceLocation[]} */ - const child_locations = []; - /** @type {ComponentClientTransformState} */ const state = { ...context.state, metadata: child_metadata, - locations: child_locations, + locations: [], scope: /** @type {Scope} */ (context.state.scopes.get(node.fragment)), preserve_whitespace: context.state.preserve_whitespace || node.name === 'pre' || node.name === 'textarea' @@ -391,9 +388,9 @@ export function RegularElement(node, context) { context.state.update.push(b.stmt(b.assignment('=', dir, dir))); } - if (child_locations.length > 0) { + if (state.locations.length > 0) { // @ts-expect-error - location.push(child_locations); + location.push(state.locations); } if (!is_void(node.name)) { From ba97c5ec192de200380c4d12aa441d6a22a70d2f Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 20 Sep 2024 11:21:52 -0400 Subject: [PATCH 10/15] group stuff --- .../client/visitors/RegularElement.js | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js index 3e26a6bbf515..bcb38b890b9e 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js @@ -62,11 +62,6 @@ export function RegularElement(node, context) { context.state.metadata.context.template_contains_script_tag = true; } - const child_metadata = { - ...context.state.metadata, - namespace: determine_namespace_for_children(node, context.state.metadata.namespace) - }; - context.state.template.push(`<${node.name}`); /** @type {Array} */ @@ -164,18 +159,6 @@ export function RegularElement(node, context) { } } - if (bindings.has('innerHTML') || bindings.has('innerText') || bindings.has('textContent')) { - const contenteditable = lookup.get('contenteditable'); - - if ( - contenteditable && - (contenteditable.value === true || - (is_text_attribute(contenteditable) && contenteditable.value[0].data === 'true')) - ) { - child_metadata.bound_contenteditable = true; - } - } - if ( node.name === 'input' && node.attributes.some((attribute) => { @@ -294,6 +277,23 @@ export function RegularElement(node, context) { context.state.template.push('>'); + const child_metadata = { + ...context.state.metadata, + namespace: determine_namespace_for_children(node, context.state.metadata.namespace) + }; + + if (bindings.has('innerHTML') || bindings.has('innerText') || bindings.has('textContent')) { + const contenteditable = lookup.get('contenteditable'); + + if ( + contenteditable && + (contenteditable.value === true || + (is_text_attribute(contenteditable) && contenteditable.value[0].data === 'true')) + ) { + child_metadata.bound_contenteditable = true; + } + } + /** @type {ComponentClientTransformState} */ const state = { ...context.state, From b72f743625e1a10c13edc34e65899e1e07770ce9 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 20 Sep 2024 11:24:37 -0400 Subject: [PATCH 11/15] rename --- .../phases/3-transform/client/visitors/RegularElement.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js index bcb38b890b9e..3edee637068b 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js @@ -277,7 +277,7 @@ export function RegularElement(node, context) { context.state.template.push('>'); - const child_metadata = { + const metadata = { ...context.state.metadata, namespace: determine_namespace_for_children(node, context.state.metadata.namespace) }; @@ -290,14 +290,14 @@ export function RegularElement(node, context) { (contenteditable.value === true || (is_text_attribute(contenteditable) && contenteditable.value[0].data === 'true')) ) { - child_metadata.bound_contenteditable = true; + metadata.bound_contenteditable = true; } } /** @type {ComponentClientTransformState} */ const state = { ...context.state, - metadata: child_metadata, + metadata, locations: [], scope: /** @type {Scope} */ (context.state.scopes.get(node.fragment)), preserve_whitespace: @@ -308,7 +308,7 @@ export function RegularElement(node, context) { node, node.fragment.nodes, context.path, - child_metadata.namespace, + state.metadata.namespace, state, node.name === 'script' || state.preserve_whitespace, state.options.preserveComments From 6c5801371c4b5f1609d08027f2622865ac5f6ae7 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 20 Sep 2024 11:27:30 -0400 Subject: [PATCH 12/15] simplify --- .../phases/3-transform/client/visitors/RegularElement.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js index 3edee637068b..694660ed104a 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js @@ -314,9 +314,6 @@ export function RegularElement(node, context) { state.options.preserveComments ); - /** Whether or not we need to wrap the children in `{...}` to avoid declaration conflicts */ - const has_declaration = node.fragment.nodes.some((node) => node.type === 'SnippetBlock'); - /** @type {typeof state} */ const child_state = { ...state, init: [], update: [], after_update: [] }; @@ -367,7 +364,8 @@ export function RegularElement(node, context) { } } - if (has_declaration) { + if (node.fragment.nodes.some((node) => node.type === 'SnippetBlock')) { + // Wrap children in `{...}` to avoid declaration conflicts context.state.init.push( b.block([ ...child_state.init, From 50a9ebcb359e35a60114abae42ed4bfb0d9dfd7a Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 20 Sep 2024 11:30:24 -0400 Subject: [PATCH 13/15] shuffle --- .../client/visitors/RegularElement.js | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js index 694660ed104a..cd1a329f1653 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js @@ -58,6 +58,16 @@ export function RegularElement(node, context) { return; } + const is_custom_element = is_custom_element_node(node); + + if (is_custom_element) { + // cloneNode is faster, but it does not instantiate the underlying class of the + // custom element until the template is connected to the dom, which would + // cause problems when setting properties on the custom element. + // Therefore we need to use importNode instead, which doesn't have this caveat. + context.state.metadata.context.template_needs_import_node = true; + } + if (node.name === 'script') { context.state.metadata.context.template_contains_script_tag = true; } @@ -79,16 +89,6 @@ export function RegularElement(node, context) { /** @type {ExpressionStatement[]} */ const lets = []; - const is_custom_element = is_custom_element_node(node); - - if (is_custom_element) { - // cloneNode is faster, but it does not instantiate the underlying class of the - // custom element until the template is connected to the dom, which would - // cause problems when setting properties on the custom element. - // Therefore we need to use importNode instead, which doesn't have this caveat. - context.state.metadata.context.template_needs_import_node = true; - } - /** @type {Map} */ const lookup = new Map(); From 3bce292b5ce6c31b6f97bcd57f35f454642d0c99 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 20 Sep 2024 11:39:34 -0400 Subject: [PATCH 14/15] shuffle --- .../client/visitors/RegularElement.js | 34 +++++++++---------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js index cd1a329f1653..52ad8344c056 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js @@ -161,18 +161,16 @@ export function RegularElement(node, context) { if ( node.name === 'input' && - node.attributes.some((attribute) => { - return ( - attribute.type === 'SpreadAttribute' || - (attribute.type === 'Attribute' && + (has_spread || + bindings.has('value') || + bindings.has('checked') || + bindings.has('group') || + attributes.some( + (attribute) => + attribute.type === 'Attribute' && (attribute.name === 'value' || attribute.name === 'checked') && - !is_text_attribute(attribute)) || - (attribute.type === 'BindDirective' && - (attribute.name === 'group' || - attribute.name === 'checked' || - attribute.name === 'value')) - ); - }) + !is_text_attribute(attribute) + )) ) { context.state.init.push(b.stmt(b.call('$.remove_input_defaults', context.state.node))); } @@ -190,11 +188,11 @@ export function RegularElement(node, context) { setup_select_synchronization(/** @type {AST.BindDirective} */ (bindings.get('value')), context); } - const node_id = context.state.node; - // Let bindings first, they can be used on attributes context.state.init.push(...lets); + const node_id = context.state.node; + // Then do attributes let is_attributes_reactive = false; if (node.metadata.has_spread) { @@ -253,11 +251,6 @@ export function RegularElement(node, context) { } } - // Apply the src and loading attributes for elements after the element is appended to the document - if (node.name === 'img' && (has_spread || lookup.has('loading'))) { - context.state.after_update.push(b.stmt(b.call('$.handle_lazy_img', node_id))); - } - // class/style directives must be applied last since they could override class/style attributes build_class_directives(class_directives, node_id, context, is_attributes_reactive); build_style_directives( @@ -268,6 +261,11 @@ export function RegularElement(node, context) { lookup.has('style') || node.metadata.has_spread ); + // Apply the src and loading attributes for elements after the element is appended to the document + if (node.name === 'img' && (has_spread || lookup.has('loading'))) { + context.state.after_update.push(b.stmt(b.call('$.handle_lazy_img', node_id))); + } + if ( is_load_error_element(node.name) && (has_spread || has_use || lookup.has('onload') || lookup.has('onerror')) From dd10090f353794b8690553672886b495baa5f75b Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 20 Sep 2024 11:57:54 -0400 Subject: [PATCH 15/15] doh --- .../phases/3-transform/client/visitors/RegularElement.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js index 52ad8344c056..fd8aeb6e4c6a 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js @@ -83,7 +83,7 @@ export function RegularElement(node, context) { /** @type {AST.StyleDirective[]} */ const style_directives = []; - /** @type {Array} */ const other_directives = []; /** @type {ExpressionStatement[]} */