From 72f93e361fdd141bd242f403e070bdaf30f3d063 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Fri, 21 Mar 2025 09:25:29 +0100 Subject: [PATCH] chore: sprinkle comments here and there --- .../3-transform/client/transform-client.js | 2 +- .../client/transform-template/to-functions.js | 25 +++++++++++++++++++ .../client/transform-template/to-string.js | 13 ++++++++++ .../client/visitors/RegularElement.js | 6 ++++- .../client/visitors/shared/component.js | 2 ++ .../src/compiler/phases/3-transform/utils.js | 4 +++ 6 files changed, 50 insertions(+), 2 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js index 0481621ce1..8016c82873 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js @@ -167,7 +167,7 @@ export function client_component(analysis, options) { in_constructor: false, instance_level_snippets: [], module_level_snippets: [], - is_functional_template_mode: options.templatingMode === 'functional', + is_functional_template_mode: true, //options.templatingMode === 'functional', // these are set inside the `Fragment` visitor, and cannot be used until then init: /** @type {any} */ (null), diff --git a/packages/svelte/src/compiler/phases/3-transform/client/transform-template/to-functions.js b/packages/svelte/src/compiler/phases/3-transform/client/transform-template/to-functions.js index 9437fae20e..2126b00fde 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/transform-template/to-functions.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/transform-template/to-functions.js @@ -57,15 +57,20 @@ export function template_to_functions(items, namespace) { */ let last_current_element; + // if the first item is a comment we need to add another comment for effect.start if (items[0].kind === 'create_anchor') { items.unshift({ kind: 'create_anchor' }); } for (let instruction of items) { + // on push element we add the element to the stack, from this moment on every insert will + // happen on the last element in the stack if (instruction.kind === 'push_element' && last_current_element) { elements_stack.push(last_current_element); continue; } + // we closed one element, we remove it from the stack and eventually revert back + // the namespace to the previous one if (instruction.kind === 'pop_element') { const removed = elements_stack.pop(); if (removed?.namespaced) { @@ -77,6 +82,8 @@ export function template_to_functions(items, namespace) { continue; } + // if the inserted node is in the svg/mathml we push the namespace to the stack because we need to + // create with createElementNS if (instruction.metadata?.svg || instruction.metadata?.mathml) { namespace_stack.push(instruction.metadata.svg ? NAMESPACE_SVG : NAMESPACE_MATHML); } @@ -84,7 +91,12 @@ export function template_to_functions(items, namespace) { // @ts-expect-error we can't be here if `swap_current_element` but TS doesn't know that const value = map[instruction.kind]( ...[ + // for set prop we need to send the last element (not the one in the stack since + // it get's added to the stack only after the push_element instruction)...for all the rest + // the first prop is a the scope to generate the name of the variable ...(instruction.kind === 'set_prop' ? [last_current_element] : [scope]), + // for create element we also need to add the namespace...namespaces in the stack get's precedence over + // the "global" namespace (and if we are in a foreignObject we default to html) ...(instruction.kind === 'create_element' ? [ foreign_object_count > 0 @@ -102,9 +114,12 @@ export function template_to_functions(items, namespace) { ); if (value) { + // this will compose the body of the function body.push(value.call); } + // with set_prop we don't need to do anything else, in all other cases we also need to + // append the element/node/anchor to the current active element or push it in the elements array if (instruction.kind !== 'set_prop') { if (elements_stack.length >= 1 && value) { const { call } = map.insert(/** @type {Element} */ (elements_stack.at(-1)), value); @@ -112,6 +127,7 @@ export function template_to_functions(items, namespace) { } else if (value) { elements.push(b.id(value.name)); } + // keep track of the last created element (it will be pushed to the stack after the props are set) if (instruction.kind === 'create_element') { last_current_element = /** @type {Element} */ (value); if (last_current_element.element === 'foreignObject') { @@ -120,6 +136,7 @@ export function template_to_functions(items, namespace) { } } } + // every function needs to return a fragment so we create one and push all the elements there const fragment = scope.generate('fragment'); body.push(b.var(fragment, b.call('document.createDocumentFragment'))); body.push(b.call(fragment + '.append', ...elements)); @@ -159,6 +176,11 @@ function create_element(scope, namespace, element) { } const call = b.var(name, b.call(fn, ...args)); /** + * if there's an "is" attribute we can't just add it as a property, it needs to be + * specified on creation like this `document.createElement('button', { is: 'my-button' })` + * + * Since the props are appended after the creation we change the generated call arguments and we push + * the is attribute later on on `set_prop` * @param {string} value */ function add_is(value) { @@ -208,6 +230,7 @@ function create_text(scope, value) { * @param {string} value */ function set_prop(el, prop, value) { + // see comment above about the "is" attribute if (prop === 'is') { el.add_is(value); return; @@ -217,6 +240,7 @@ function set_prop(el, prop, value) { let fn = namespace !== prop ? '.setAttributeNS' : '.setAttribute'; let args = [b.literal(fix_attribute_casing(prop)), b.literal(value ?? '')]; + // attributes like `xlink:href` need to be set with the `xlink` namespace if (namespace === 'xlink') { args.unshift(b.literal('http://www.w3.org/1999/xlink')); } @@ -235,6 +259,7 @@ function set_prop(el, prop, value) { function insert(el, child, anchor) { return { call: b.call( + // if we have a template element we need to push into it's content rather than the element itself el.name + (el.element === 'template' ? '.content' : '') + '.insertBefore', b.id(child.name), b.id(anchor?.name ?? 'undefined') diff --git a/packages/svelte/src/compiler/phases/3-transform/client/transform-template/to-string.js b/packages/svelte/src/compiler/phases/3-transform/client/transform-template/to-string.js index c4a0b1565c..ed0b73dd3e 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/transform-template/to-string.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/transform-template/to-string.js @@ -20,10 +20,13 @@ export function template_to_string(items) { let last_current_element; for (let instruction of items) { + // on push element we add the element to the stack, from this moment on every insert will + // happen on the last element in the stack if (instruction.kind === 'push_element' && last_current_element) { elements_stack.push(last_current_element); continue; } + // we closed one element, we remove it from the stack if (instruction.kind === 'pop_element') { elements_stack.pop(); continue; @@ -34,16 +37,21 @@ export function template_to_string(items) { // @ts-expect-error we can't be here if `swap_current_element` but TS doesn't know that const value = map[instruction.kind]( ...[ + // for set prop we need to send the last element (not the one in the stack since + // it get's added to the stack only after the push_element instruction) ...(instruction.kind === 'set_prop' ? [last_current_element] : []), ...(instruction.args ?? []) ] ); + // with set_prop we don't need to do anything else, in all other cases we also need to + // append the element/node/anchor to the current active element or push it in the elements array if (instruction.kind !== 'set_prop') { if (elements_stack.length >= 1 && value) { map.insert(/** @type {Element} */ (elements_stack.at(-1)), value); } else if (value) { elements.push(value); } + // keep track of the last created element (it will be pushed to the stack after the props are set) if (instruction.kind === 'create_element') { last_current_element = /** @type {Element} */ (value); } @@ -141,7 +149,9 @@ let map = { function stringify(el) { let str = ``; if (el.kind === 'element') { + // we create the `; + // we stringify all the children and concatenate them for (let child of el.children ?? []) { str += stringify(child); } + // if it's not void we also add the closing tag if (!is_void(el.element)) { str += ``; } 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 db2ddf7fc2..9833868014 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 @@ -124,6 +124,8 @@ export function RegularElement(node, context) { kind: 'set_prop', args: [ 'is', + // if we are using the functional template mode we don't want to escape since we will + // create a text node from it which is already escaped context.state.is_functional_template_mode ? value.value : escape_html(value.value, true) @@ -312,7 +314,9 @@ export function RegularElement(node, context) { : [ value === true ? '' - : context.state.is_functional_template_mode + : // if we are using the functional template mode we don't want to escape since we will + // create a text node from it which is already escaped + context.state.is_functional_template_mode ? value : escape_html(value, true) ] diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js index 90fe4d93e1..3569574f1b 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js @@ -427,11 +427,13 @@ export function build_component(node, component_name, context, anchor = context. */ const template_operations = []; if (context.state.metadata.namespace === 'svg') { + // this boils down to template_operations.push({ kind: 'create_element', args: ['g'] }); template_operations.push({ kind: 'push_element' }); template_operations.push({ kind: 'create_anchor' }); template_operations.push({ kind: 'pop_element' }); } else { + // this boils down to template_operations.push({ kind: 'create_element', args: ['svelte-css-wrapper'] }); template_operations.push({ kind: 'set_prop', args: ['style', 'display: contents'] }); template_operations.push({ kind: 'push_element' }); diff --git a/packages/svelte/src/compiler/phases/3-transform/utils.js b/packages/svelte/src/compiler/phases/3-transform/utils.js index 93e4c34479..07beb27e02 100644 --- a/packages/svelte/src/compiler/phases/3-transform/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/utils.js @@ -276,12 +276,16 @@ export function clean_nodes( // initial newline inside a `
` is disregarded, if not followed by another newline
 	if (
 		parent.type === 'RegularElement' &&
+		// we also want to do the replacement on the textarea if we are in functional template mode because createTextNode behave differently
+		// then template.innerHTML
 		(parent.name === 'pre' || (is_functional_template_mode && parent.name === 'textarea')) &&
 		first?.type === 'Text'
 	) {
 		const text = first.data.replace(regex_starts_with_newline, '');
 		if (text !== first.data) {
 			const tmp = text.replace(regex_starts_with_newline, '');
+			// do an extra replacement if we are in functional template mode because createTextNode behave differently
+			// then template.innerHTML
 			if (text === tmp || is_functional_template_mode) {
 				first.data = text;
 				first.raw = first.raw.replace(regex_starts_with_newline, '');