chore: convert member expression property strings to identifiers (#12890)

Rich-Harris · web-flow · commit 47e8ad761956 · 2024-08-17T11:23:54.000-04:00
* chore: convert member expression property strings to identifiers

* oops

* tweak

* while we're here
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
@@ -473,12 +473,8 @@ export function client_component(analysis, options) {
 		const incoming = b.member(b.id('module.default'), HMR, true);
 
 		const accept_fn_body = [
-			b.stmt(
-				b.assignment('=', b.member(incoming, b.id('source')), b.member(existing, b.id('source')))
-			),
-			b.stmt(
-				b.call('$.set', b.member(existing, b.id('source')), b.member(incoming, b.id('original')))
-			)
+			b.stmt(b.assignment('=', b.member(incoming, 'source'), b.member(existing, 'source'))),
+			b.stmt(b.call('$.set', b.member(existing, 'source'), b.member(incoming, 'original')))
 		];
 
 		if (analysis.css.hash) {
@@ -488,7 +484,7 @@ export function client_component(analysis, options) {
 					b.call(
 						b.member(
 							b.call('document.querySelector', b.literal('#' + analysis.css.hash)),
-							b.id('remove'),
+							'remove',
 							false,
 							true
 						)
@@ -498,9 +494,7 @@ export function client_component(analysis, options) {
 		}
 
 		const hmr = b.block([
-			b.stmt(
-				b.assignment('=', id, b.call('$.hmr', id, b.thunk(b.member(existing, b.id('source')))))
-			),
+			b.stmt(b.assignment('=', id, b.call('$.hmr', id, b.thunk(b.member(existing, 'source'))))),
 
 			b.stmt(b.call('import.meta.hot.accept', b.arrow([b.id('module')], b.block(accept_fn_body))))
 		]);
@@ -515,11 +509,7 @@ export function client_component(analysis, options) {
 			// add `App[$.FILENAME] = 'App.svelte'` so that we can print useful messages later
 			body.unshift(
 				b.stmt(
-					b.assignment(
-						'=',
-						b.member(b.id(analysis.name), b.id('$.FILENAME'), true),
-						b.literal(filename)
-					)
+					b.assignment('=', b.member(b.id(analysis.name), '$.FILENAME', true), b.literal(filename))
 				)
 			);
 		}
diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/Fragment.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/Fragment.js
@@ -96,7 +96,7 @@ export function Fragment(node, context) {
 			call = b.call(
 				'$.add_locations',
 				call,
-				b.member(b.id(context.state.analysis.name), b.id('$.FILENAME'), true),
+				b.member(b.id(context.state.analysis.name), '$.FILENAME', true),
 				build_locations(state.locations)
 			);
 		}
diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/LetDirective.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/LetDirective.js
@@ -33,7 +33,7 @@ export function LetDirective(node, context) {
 									b.object_pattern(node.expression.properties)
 								: // @ts-expect-error types don't match, but it can't contain spread elements and the structure is otherwise fine
 									b.array_pattern(node.expression.elements),
-							b.member(b.id('$$slotProps'), b.id(node.name))
+							b.member(b.id('$$slotProps'), node.name)
 						),
 						b.return(b.object(bindings.map((binding) => b.init(binding.node.name, binding.node))))
 					])
@@ -48,7 +48,7 @@ export function LetDirective(node, context) {
 
 		return b.const(
 			name,
-			create_derived(context.state, b.thunk(b.member(b.id('$$slotProps'), b.id(node.name))))
+			create_derived(context.state, b.thunk(b.member(b.id('$$slotProps'), node.name)))
 		);
 	}
 }
diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/MemberExpression.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/MemberExpression.js
@@ -11,15 +11,15 @@ export function MemberExpression(node, context) {
 	if (node.property.type === 'PrivateIdentifier') {
 		const field = context.state.private_state.get(node.property.name);
 		if (field) {
-			return context.state.in_constructor ? b.member(node, b.id('v')) : b.call('$.get', node);
+			return context.state.in_constructor ? b.member(node, 'v') : b.call('$.get', node);
 		}
 	} else if (node.object.type === 'ThisExpression') {
 		// rewrite `this.foo` as `this.#foo.v` inside a constructor
 		if (node.property.type === 'Identifier' && !node.computed) {
 			const field = context.state.public_state.get(node.property.name);
 
 			if (field && context.state.in_constructor) {
-				return b.member(b.member(b.this, field.id), b.id('v'));
+				return b.member(b.member(b.this, field.id), 'v');
 			}
 		}
 	}
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
@@ -322,9 +322,7 @@ export function RegularElement(node, context) {
 
 	if (text_content && !text_content.has_state) {
 		child_state.init.push(
-			b.stmt(
-				b.assignment('=', b.member(context.state.node, b.id('textContent')), text_content.value)
-			)
+			b.stmt(b.assignment('=', b.member(context.state.node, 'textContent'), text_content.value))
 		);
 	} else {
 		/** @type {Expression} */
@@ -339,7 +337,7 @@ export function RegularElement(node, context) {
 		if (node.name === 'template') {
 			needs_reset = true;
 			child_state.init.push(b.stmt(b.call('$.hydrate_template', arg)));
-			arg = b.member(arg, b.id('content'));
+			arg = b.member(arg, 'content');
 		}
 
 		process_children(trimmed, () => b.call('$.child', arg), true, {
@@ -369,9 +367,8 @@ export function RegularElement(node, context) {
 	if (has_direction_attribute) {
 		// 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.
-		context.state.update.push(
-			b.stmt(b.assignment('=', b.member(node_id, b.id('dir')), b.member(node_id, b.id('dir'))))
-		);
+		const dir = b.member(node_id, 'dir');
+		context.state.update.push(b.stmt(b.assignment('=', dir, dir)));
 	}
 
 	if (child_locations.length > 0) {
@@ -494,16 +491,16 @@ function build_element_spread_attributes(
 
 	const preserve_attribute_case =
 		element.metadata.svg || element.metadata.mathml || is_custom_element_node(element);
-	const id = context.state.scope.generate('attributes');
+	const id = b.id(context.state.scope.generate('attributes'));
 
 	const update = b.stmt(
 		b.assignment(
 			'=',
-			b.id(id),
+			id,
 			b.call(
 				'$.set_attributes',
 				element_id,
-				b.id(id),
+				id,
 				b.object(values),
 				context.state.analysis.css.hash !== '' && b.literal(context.state.analysis.css.hash),
 				preserve_attribute_case && b.true,
@@ -523,17 +520,17 @@ function build_element_spread_attributes(
 
 	if (needs_select_handling) {
 		context.state.init.push(
-			b.stmt(b.call('$.init_select', element_id, b.thunk(b.member(b.id(id), b.id('value')))))
+			b.stmt(b.call('$.init_select', element_id, b.thunk(b.member(id, 'value'))))
 		);
 		context.state.update.push(
 			b.if(
-				b.binary('in', b.literal('value'), b.id(id)),
+				b.binary('in', b.literal('value'), id),
 				b.block([
 					// This ensures a one-way street to the DOM in case it's <select {value}>
 					// and not <select bind:value>. We need it in addition to $.init_select
 					// because the select value is not reflected as an attribute, so the
 					// mutation observer wouldn't notice.
-					b.stmt(b.call('$.select_option', element_id, b.member(b.id(id), b.id('value'))))
+					b.stmt(b.call('$.select_option', element_id, b.member(id, 'value')))
 				])
 			)
 		);
@@ -596,7 +593,7 @@ function build_element_attribute_update_assignment(element, node_id, attribute,
 	} else if (name === 'checked') {
 		update = b.stmt(b.call('$.set_checked', node_id, value));
 	} else if (is_dom_property(name)) {
-		update = b.stmt(b.assignment('=', b.member(node_id, b.id(name)), value));
+		update = b.stmt(b.assignment('=', b.member(node_id, name), value));
 	} else {
 		const callee = name.startsWith('xlink') ? '$.set_xlink_attribute' : '$.set_attribute';
 		update = b.stmt(
@@ -666,9 +663,9 @@ function build_element_special_value_attribute(element, node_id, attribute, cont
 
 	const inner_assignment = b.assignment(
 		'=',
-		b.member(node_id, b.id('value')),
+		b.member(node_id, 'value'),
 		b.conditional(
-			b.binary('==', b.literal(null), b.assignment('=', b.member(node_id, b.id('__value')), value)),
+			b.binary('==', b.literal(null), b.assignment('=', b.member(node_id, '__value'), value)),
 			b.literal(''), // render null/undefined values as empty string to support placeholder options
 			value
 		)
diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SlotElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SlotElement.js
@@ -60,7 +60,7 @@ export function SlotElement(node, context) {
 
 	const expression = is_default
 		? b.call('$.default_slot', b.id('$$props'))
-		: b.member(b.member(b.id('$$props'), b.id('$$slots')), name, true, true);
+		: b.member(b.member(b.id('$$props'), '$$slots'), name, true, true);
 
 	const slot = b.call('$.slot', context.state.node, expression, props_expression, fallback);
 	context.state.init.push(b.stmt(slot));
diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/TitleElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/TitleElement.js
@@ -14,7 +14,7 @@ export function TitleElement(node, context) {
 		context.state
 	);
 
-	const statement = b.stmt(b.assignment('=', b.member(b.id('$.document'), b.id('title')), value));
+	const statement = b.stmt(b.assignment('=', b.id('$.document.title'), value));
 
 	if (has_state) {
 		context.state.update.push(statement);
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
@@ -372,7 +372,7 @@ export function build_component(node, component_name, context, anchor = context.
 
 		statements.push(
 			b.stmt(b.call('$.css_props', anchor, b.thunk(b.object(custom_css_props)))),
-			b.stmt(fn(b.member(anchor, b.id('lastChild')))),
+			b.stmt(fn(b.member(anchor, 'lastChild'))),
 			b.stmt(b.call('$.reset', anchor))
 		);
 	} else {
diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/events.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/events.js
@@ -54,11 +54,7 @@ export function visit_event_attribute(node, context) {
 
 		context.state.init.push(
 			b.stmt(
-				b.assignment(
-					'=',
-					b.member(context.state.node, b.id('__' + event_name)),
-					delegated_assignment
-				)
+				b.assignment('=', b.member(context.state.node, '__' + event_name), delegated_assignment)
 			)
 		);
 	} else {
@@ -143,7 +139,7 @@ export function build_event_handler(node, metadata, context) {
 	}
 
 	// wrap the handler in a function, so the expression is re-evaluated for each event
-	let call = b.call(b.member(handler, b.id('apply'), false, true), b.this, b.id('$$args'));
+	let call = b.call(b.member(handler, 'apply', false, true), b.this, b.id('$$args'));
 
 	if (dev) {
 		const loc = locator(/** @type {number} */ (node.start));
diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/fragment.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/fragment.js
@@ -49,7 +49,7 @@ export function process_children(nodes, expression, is_element, { visit, state }
 		} else if (has_state && !within_bound_contenteditable) {
 			state.update.push(update);
 		} else {
-			state.init.push(b.stmt(b.assignment('=', b.member(id, b.id('nodeValue')), value)));
+			state.init.push(b.stmt(b.assignment('=', b.member(id, 'nodeValue'), value)));
 		}
 
 		expression = (is_text) => b.call('$.sibling', id, is_text && b.true);
diff --git a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js
@@ -321,7 +321,7 @@ export function server_component(analysis, options) {
 									b.id(analysis.name),
 									b.object([
 										b.init('props', b.id('$$props')),
-										b.init('context', b.member(b.id('$$opts'), b.id('context'), false, true))
+										b.init('context', b.member(b.id('$$opts'), 'context', false, true))
 									])
 								)
 							)
@@ -360,11 +360,7 @@ export function server_component(analysis, options) {
 		// add `App[$.FILENAME] = 'App.svelte'` so that we can print useful messages later
 		body.unshift(
 			b.stmt(
-				b.assignment(
-					'=',
-					b.member(b.id(analysis.name), b.id('$.FILENAME'), true),
-					b.literal(filename)
-				)
+				b.assignment('=', b.member(b.id(analysis.name), '$.FILENAME', true), b.literal(filename))
 			)
 		);
 	}
diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/EachBlock.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/EachBlock.js
@@ -35,7 +35,7 @@ export function EachBlock(node, context) {
 
 	const for_loop = b.for(
 		b.let(index, b.literal(0)),
-		b.binary('<', index, b.member(array_id, b.id('length'))),
+		b.binary('<', index, b.member(array_id, 'length')),
 		b.update('++', index, false),
 		b.block(each)
 	);
@@ -51,7 +51,7 @@ export function EachBlock(node, context) {
 
 		state.template.push(
 			b.if(
-				b.binary('!==', b.member(array_id, b.id('length')), b.literal(0)),
+				b.binary('!==', b.member(array_id, 'length'), b.literal(0)),
 				b.block([open, for_loop]),
 				fallback
 			),
diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js
@@ -138,7 +138,7 @@ export function build_element_attributes(node, context) {
 							parent: attribute,
 							expression: is_checkbox
 								? b.call(
-										b.member(attribute.expression, b.id('includes')),
+										b.member(attribute.expression, 'includes'),
 										build_attribute_value(value_attribute.value, context)
 									)
 								: b.binary(
@@ -389,10 +389,7 @@ function build_class_directives(class_directives, class_attribute) {
 		end: -1,
 		parent: class_attribute,
 		expression: b.call(
-			b.member(
-				b.call(b.member(b.array(expressions), b.id('filter')), b.id('Boolean')),
-				b.id('join')
-			),
+			b.member(b.call(b.member(b.array(expressions), 'filter'), b.id('Boolean')), b.id('join')),
 			b.literal(' ')
 		),
 		metadata: {
diff --git a/packages/svelte/src/compiler/utils/ast.js b/packages/svelte/src/compiler/utils/ast.js
@@ -332,7 +332,7 @@ function _extract_paths(assignments = [], param, expression, update_expression,
 					if (element.type === 'RestElement') {
 						/** @type {DestructuredAssignment['expression']} */
 						const rest_expression = (object) =>
-							b.call(b.member(expression(object), b.id('slice')), b.literal(i));
+							b.call(b.member(expression(object), 'slice'), b.literal(i));
 						if (element.argument.type === 'Identifier') {
 							assignments.push({
 								node: element.argument,
diff --git a/packages/svelte/src/compiler/utils/builders.js b/packages/svelte/src/compiler/utils/builders.js
@@ -286,12 +286,16 @@ export function literal(value) {
 
 /**
  * @param {ESTree.Expression | ESTree.Super} object
- * @param {ESTree.Expression | ESTree.PrivateIdentifier} property
+ * @param {string | ESTree.Expression | ESTree.PrivateIdentifier} property
  * @param {boolean} computed
  * @param {boolean} optional
  * @returns {ESTree.MemberExpression}
  */
 export function member(object, property, computed = false, optional = false) {
+	if (typeof property === 'string') {
+		property = id(property);
+	}
+
 	return { type: 'MemberExpression', object, property, computed, optional };
 }
 

Original file line number	Diff line number	Diff line change
`@@ -96,7 +96,7 @@ export function Fragment(node, context) {`
`96`	`96`	`call = b.call(`
`97`	`97`	`'$.add_locations',`
`98`	`98`	`call,`
`99`		`- b.member(b.id(context.state.analysis.name), b.id('$.FILENAME'), true),`
	`99`	`+ b.member(b.id(context.state.analysis.name), '$.FILENAME', true),`
`100`	`100`	`build_locations(state.locations)`
`101`	`101`	`);`
`102`	`102`	`}`
Original file line number	Diff line number	Diff line change
`@@ -11,15 +11,15 @@ export function MemberExpression(node, context) {`
`11`	`11`	`if (node.property.type === 'PrivateIdentifier') {`
`12`	`12`	`const field = context.state.private_state.get(node.property.name);`
`13`	`13`	`if (field) {`
`14`		`- return context.state.in_constructor ? b.member(node, b.id('v')) : b.call('$.get', node);`
	`14`	`+ return context.state.in_constructor ? b.member(node, 'v') : b.call('$.get', node);`
`15`	`15`	`}`
`16`	`16`	`} else if (node.object.type === 'ThisExpression') {`
`17`	`17`	// rewrite `this.foo` as `this.#foo.v` inside a constructor
`18`	`18`	`if (node.property.type === 'Identifier' && !node.computed) {`
`19`	`19`	`const field = context.state.public_state.get(node.property.name);`
`20`	`20`
`21`	`21`	`if (field && context.state.in_constructor) {`
`22`		`- return b.member(b.member(b.this, field.id), b.id('v'));`
	`22`	`+ return b.member(b.member(b.this, field.id), 'v');`
`23`	`23`	`}`
`24`	`24`	`}`
`25`	`25`	`}`
Original file line number	Diff line number	Diff line change
`@@ -49,7 +49,7 @@ export function process_children(nodes, expression, is_element, { visit, state }`
`49`	`49`	`} else if (has_state && !within_bound_contenteditable) {`
`50`	`50`	`state.update.push(update);`
`51`	`51`	`} else {`
`52`		`- state.init.push(b.stmt(b.assignment('=', b.member(id, b.id('nodeValue')), value)));`
	`52`	`+ state.init.push(b.stmt(b.assignment('=', b.member(id, 'nodeValue'), value)));`
`53`	`53`	`}`
`54`	`54`
`55`	`55`	`expression = (is_text) => b.call('$.sibling', id, is_text && b.true);`