diff --git a/.changeset/ref-array-lval-assignment.md b/.changeset/ref-array-lval-assignment.md new file mode 100644 index 000000000..e5226d8d4 --- /dev/null +++ b/.changeset/ref-array-lval-assignment.md @@ -0,0 +1,15 @@ +--- +"@solidjs/babel-plugin": patch +"@solidjs/compiler": patch +--- + +Assign bare variable refs inside `ref` arrays. `ref={[el]}` type-checked and +compiled but `el` was never assigned: the array branch passed the array +straight through, and the runtime only *calls* ref-array entries (a bare +variable evaluates to `undefined` at mount, so it was silently skipped). +Bare identifiers and member expressions inside a ref array are now lowered +to assignment callbacks — read the target once, call it with the element +when it holds a function, otherwise assign the element — matching the +non-array `ref={el}` contract. Nested arrays are recursed, callback refs +and const/module refs pass through untouched, and falsy slots +(`null`/`undefined`/`false`) keep short-circuiting. diff --git a/packages/babel-plugin/src/dom/element.ts b/packages/babel-plugin/src/dom/element.ts index 4c9fc20e6..5bb8ea7fc 100644 --- a/packages/babel-plugin/src/dom/element.ts +++ b/packages/babel-plugin/src/dom/element.ts @@ -32,7 +32,8 @@ import { inlineCallExpression, hasStaticMarker, canChildSlotAllocateIds, - isFunctionShapedHole + isFunctionShapedHole, + transformRefArrayLiteral } from "../shared/utils"; import { transformNode } from "../shared/transform"; import { InlineElements, BlockElements } from "./constants"; @@ -1003,6 +1004,11 @@ function transformAttributes( t.isFunction(value.expression) || t.isArrayExpression(value.expression) ) { + if (t.isArrayExpression(value.expression)) { + // Bare lval elements (e.g. `ref={[elRef]}`) become callback refs + // so the variable actually gets assigned (#3285). + transformRefArrayLiteral(path.scope, value.expression); + } results.exprs.unshift( t.expressionStatement( t.callExpression( diff --git a/packages/babel-plugin/src/shared/component.ts b/packages/babel-plugin/src/shared/component.ts index f180d25ee..6dc1ad131 100644 --- a/packages/babel-plugin/src/shared/component.ts +++ b/packages/babel-plugin/src/shared/component.ts @@ -7,7 +7,8 @@ import { filterChildren, trimWhitespace, transformCondition, - convertJSXIdentifier + convertJSXIdentifier, + transformRefArrayLiteral } from "./utils"; import { transformNode, getCreateTemplate } from "./transform"; import type { PluginConfig } from "../config"; @@ -212,6 +213,11 @@ export default function transformComponent( t.isFunction(value.expression) || t.isArrayExpression(value.expression) ) { + if (t.isArrayExpression(value.expression)) { + // Bare lval elements (e.g. `ref={[elRef]}`) become callback refs + // so the variable actually gets assigned (#3285). + transformRefArrayLiteral(path.scope, value.expression); + } runningObject.push( t.objectProperty(t.identifier("ref"), value.expression as t.Expression) ); diff --git a/packages/babel-plugin/src/shared/utils.ts b/packages/babel-plugin/src/shared/utils.ts index 663edde97..d2b724168 100644 --- a/packages/babel-plugin/src/shared/utils.ts +++ b/packages/babel-plugin/src/shared/utils.ts @@ -1,7 +1,7 @@ import * as t from "@babel/types"; import { addNamed } from "@babel/helper-module-imports"; import { DOMWithState } from "../../../web/src/constants.js"; -import type { NodePath, Visitor } from "@babel/traverse"; +import type { NodePath, Scope, Visitor } from "@babel/traverse"; import type { PluginConfig, RendererConfig } from "../config"; import type { BabelHubWithMetadata, @@ -569,6 +569,76 @@ export function canNativeSpread( return true; } +/** + * Rewrite bare assignment targets inside a `ref` array literal into callback + * refs, so `ref={[elRef]}` assigns the element just like `ref={elRef}` does + * (#3285). Nested arrays are recursed because the runtime flattens ref arrays; + * call expressions and const/module bindings (a function ref passed by + * reference, e.g. `ref={[callbackRef]}`) are left untouched. + * + * Mutable bindings and member targets keep the same runtime contract as the + * non-array lval path: the current value is read once — when it is a function + * it is invoked with the element, otherwise the element is assigned back. + * Mutates the array expression in place. + */ +export function transformRefArrayLiteral(scope: Scope, array: t.ArrayExpression): void { + for (let i = 0; i < array.elements.length; i++) { + let element = array.elements[i]; + if (element === null || t.isSpreadElement(element)) continue; + if (t.isArrayExpression(element)) { + transformRefArrayLiteral(scope, element); + continue; + } + // Normalize TS non-null / as / satisfies wrappers + while ( + t.isTSNonNullExpression(element) || + t.isTSAsExpression(element) || + t.isTSSatisfiesExpression(element) + ) { + element = (element as t.TSNonNullExpression | t.TSAsExpression | t.TSSatisfiesExpression) + .expression; + } + let target: t.LVal | null = null; + if (t.isIdentifier(element)) { + // Only declared, mutable bindings can be assignment targets. + // const/module bindings are function refs passed by reference, and + // globals (`undefined`, `NaN`, ...) are not assignable at all — leave + // both untouched so falsy ref-array slots keep short-circuiting. + const binding = scope.getBinding(element.name); + if (!binding || binding.kind === "const" || binding.kind === "module") continue; + target = element; + } else if (t.isMemberExpression(element) && !t.isOptionalMemberExpression(element)) { + // Optional member targets (`a?.b`) keep their existing best-effort + // behavior; they cannot be assigned without an object guard. + target = element; + } + if (!target) continue; + const param = scope.generateUidIdentifier("el$"); + const cached = scope.generateUidIdentifier("_ref$"); + // `(el$) => { var _ref$ = ; typeof _ref$ === "function" ? _ref$(el$) : = el$; }` + const callback = t.arrowFunctionExpression( + [param], + t.blockStatement([ + t.variableDeclaration("var", [ + t.variableDeclarator(cached, t.cloneNode(target) as t.Expression) + ]), + t.expressionStatement( + t.conditionalExpression( + t.binaryExpression( + "===", + t.unaryExpression("typeof", t.cloneNode(cached)), + t.stringLiteral("function") + ), + t.callExpression(t.cloneNode(cached), [t.cloneNode(param)]), + t.assignmentExpression("=", target, t.cloneNode(param)) + ) + ) + ]) + ); + array.elements[i] = callback; + } +} + export function inlineCallExpression(node: t.Expression): t.Expression { return t.isCallExpression(node) && !node.arguments.length && diff --git a/packages/babel-plugin/src/universal/element.ts b/packages/babel-plugin/src/universal/element.ts index a12741efd..327bfe087 100644 --- a/packages/babel-plugin/src/universal/element.ts +++ b/packages/babel-plugin/src/universal/element.ts @@ -11,7 +11,8 @@ import { convertJSXIdentifier, canNativeSpread, transformCondition, - escapeStringForTemplate + escapeStringForTemplate, + transformRefArrayLiteral } from "../shared/utils"; import { transformNode } from "../shared/transform"; import type { BabelPath, TransformInfo, TransformResult, UniversalTransformResult } from "../types"; @@ -149,6 +150,11 @@ function transformAttributes( t.isFunction(value.expression) || t.isArrayExpression(value.expression) ) { + if (t.isArrayExpression(value.expression)) { + // Bare lval elements (e.g. `ref={[elRef]}`) become callback refs + // so the variable actually gets assigned (#3285). + transformRefArrayLiteral(path.scope, value.expression); + } results.exprs.unshift( t.expressionStatement( t.callExpression( diff --git a/packages/babel-plugin/test/__dom_fixtures__/refArrays/code.js b/packages/babel-plugin/test/__dom_fixtures__/refArrays/code.js new file mode 100644 index 000000000..c2d6e39e1 --- /dev/null +++ b/packages/babel-plugin/test/__dom_fixtures__/refArrays/code.js @@ -0,0 +1,27 @@ +// Bare variables inside a ref array must be assigned at mount (#3285). +let elementRef; +const el1 =
; + +// Multiple bare variables, each gets its own assignment callback. +let a, b; +const el2 =
; + +// Callback refs keep working and pass through untouched. +const el3 =
calls.push(node)]} />; + +// Member-expression targets are assignment callbacks too. +const holder = {}; +const el4 =
; + +// Nested arrays are recursed so runtime flattening reaches every callback. +let nested; +const el5 =
; + +// Falsy/placeholder slots stay as-is (the runtime short-circuits them); +// globals like `undefined` are never treated as assignment targets. +const el6 =
; + +// A mutable binding that currently holds a function is *called*, not +// overwritten — same contract as the non-array lval ref branch. +let cb = () => {}; +const el7 =
; diff --git a/packages/babel-plugin/test/__dom_fixtures__/refArrays/output.js b/packages/babel-plugin/test/__dom_fixtures__/refArrays/output.js new file mode 100644 index 000000000..5067d1b66 --- /dev/null +++ b/packages/babel-plugin/test/__dom_fixtures__/refArrays/output.js @@ -0,0 +1,90 @@ +import { template as _$template } from "r-dom"; +import { ref as _$ref } from "r-dom"; +var _tmpl$ = /*#__PURE__*/ _$template(`
`); +// Bare variables inside a ref array must be assigned at mount (#3285). +let elementRef; +var _el$ = _tmpl$(); +_$ref( + () => [ + _el$2 => { + var _ref$ = elementRef; + typeof _ref$ === "function" ? _ref$(_el$2) : (elementRef = _el$2); + } + ], + _el$ +); +const el1 = _el$; + +// Multiple bare variables, each gets its own assignment callback. +let a, b; +var _el$3 = _tmpl$(); +_$ref( + () => [ + _el$4 => { + var _ref$2 = a; + typeof _ref$2 === "function" ? _ref$2(_el$4) : (a = _el$4); + }, + _el$5 => { + var _ref$3 = b; + typeof _ref$3 === "function" ? _ref$3(_el$5) : (b = _el$5); + } + ], + _el$3 +); +const el2 = _el$3; + +// Callback refs keep working and pass through untouched. +var _el$6 = _tmpl$(); +_$ref(() => [node => calls.push(node)], _el$6); +const el3 = _el$6; + +// Member-expression targets are assignment callbacks too. +const holder = {}; +var _el$7 = _tmpl$(); +_$ref( + () => [ + _el$8 => { + var _ref$4 = holder.el; + typeof _ref$4 === "function" ? _ref$4(_el$8) : (holder.el = _el$8); + } + ], + _el$7 +); +const el4 = _el$7; + +// Nested arrays are recursed so runtime flattening reaches every callback. +let nested; +var _el$9 = _tmpl$(); +_$ref( + () => [ + [ + _el$0 => { + var _ref$5 = nested; + typeof _ref$5 === "function" ? _ref$5(_el$0) : (nested = _el$0); + } + ] + ], + _el$9 +); +const el5 = _el$9; + +// Falsy/placeholder slots stay as-is (the runtime short-circuits them); +// globals like `undefined` are never treated as assignment targets. +var _el$1 = _tmpl$(); +_$ref(() => [null, undefined, false], _el$1); +const el6 = _el$1; + +// A mutable binding that currently holds a function is *called*, not +// overwritten — same contract as the non-array lval ref branch. +let cb = () => {}; +var _el$10 = _tmpl$(); +_$ref( + () => [ + _el$11 => { + var _ref$6 = cb; + typeof _ref$6 === "function" ? _ref$6(_el$11) : (cb = _el$11); + } + ], + _el$10 +); +const el7 = _el$10; diff --git a/packages/compiler/__tests__/babel-fixtures.test.js b/packages/compiler/__tests__/babel-fixtures.test.js index 718880aa7..31b2835c5 100644 --- a/packages/compiler/__tests__/babel-fixtures.test.js +++ b/packages/compiler/__tests__/babel-fixtures.test.js @@ -25,6 +25,7 @@ const fixtureParity = { keyedElements: parityLevel.subset, multipleClassAttributes: parityLevel.subset, namespaceElements: parityLevel.subset, + refArrays: parityLevel.subset, simpleElements: parityLevel.subset, textInterpolation: parityLevel.subset }; @@ -153,6 +154,8 @@ function supportedSubset(fixture) { return source; case "attributeExpressions": return source; + case "refArrays": + return source; default: throw new Error(`No supported AST-native subset for ${fixture}`); } diff --git a/packages/compiler/__tests__/fixtures/dom/refArrays/output.js b/packages/compiler/__tests__/fixtures/dom/refArrays/output.js new file mode 100644 index 000000000..200e3c6ea --- /dev/null +++ b/packages/compiler/__tests__/fixtures/dom/refArrays/output.js @@ -0,0 +1,74 @@ +import { template as _$template } from "r-dom"; +import { ref as _$ref } from "r-dom"; +var _tmpl$ = /* @__PURE__ */ _$template(`
`); +// Bare variables inside a ref array must be assigned at mount (#3285). +let elementRef; +var _el$ = _tmpl$(); +_$ref(() => { + return [(_ref$) => { + var _ref$2 = elementRef; + typeof _ref$2 === "function" ? _ref$2(_ref$) : elementRef = _ref$; + }]; +}, _el$); +const el1 = _el$; +// Multiple bare variables, each gets its own assignment callback. +let a, b; +var _el$2 = _tmpl$(); +_$ref(() => { + return [(_ref$3) => { + var _ref$4 = a; + typeof _ref$4 === "function" ? _ref$4(_ref$3) : a = _ref$3; + }, (_ref$5) => { + var _ref$6 = b; + typeof _ref$6 === "function" ? _ref$6(_ref$5) : b = _ref$5; + }]; +}, _el$2); +const el2 = _el$2; +var _el$3 = _tmpl$(); +_$ref(() => { + return [(node) => calls.push(node)]; +}, _el$3); +// Callback refs keep working and pass through untouched. +const el3 = _el$3; +// Member-expression targets are assignment callbacks too. +const holder = {}; +var _el$4 = _tmpl$(); +_$ref(() => { + return [(_ref$7) => { + var _ref$8 = holder.el; + typeof _ref$8 === "function" ? _ref$8(_ref$7) : holder.el = _ref$7; + }]; +}, _el$4); +const el4 = _el$4; +// Nested arrays are recursed so runtime flattening reaches every callback. +let nested; +var _el$5 = _tmpl$(); +_$ref(() => { + return [[(_ref$9) => { + var _ref$10 = nested; + typeof _ref$10 === "function" ? _ref$10(_ref$9) : nested = _ref$9; + }]]; +}, _el$5); +const el5 = _el$5; +var _el$6 = _tmpl$(); +_$ref(() => { + return [ + null, + undefined, + false + ]; +}, _el$6); +// Falsy/placeholder slots stay as-is (the runtime short-circuits them); +// globals like `undefined` are never treated as assignment targets. +const el6 = _el$6; +// A mutable binding that currently holds a function is *called*, not +// overwritten — same contract as the non-array lval ref branch. +let cb = () => {}; +var _el$7 = _tmpl$(); +_$ref(() => { + return [(_ref$11) => { + var _ref$12 = cb; + typeof _ref$12 === "function" ? _ref$12(_ref$11) : cb = _ref$11; + }]; +}, _el$7); +const el7 = _el$7; diff --git a/packages/compiler/src/dom/attrs.rs b/packages/compiler/src/dom/attrs.rs index fe65bae03..f36495ef3 100644 --- a/packages/compiler/src/dom/attrs.rs +++ b/packages/compiler/src/dom/attrs.rs @@ -379,6 +379,10 @@ impl<'a> AstDomTransform<'a, '_> { } } + // Lower bare lvals inside `ref={[el]}` arrays to assignment callbacks + // (https://github.com/solidjs/solid/issues/3285). + value = crate::shared::refs::transform_ref_array_literal(self, span, value); + let is_constant = matches!( &value, Expression::Identifier(identifier) diff --git a/packages/compiler/src/shared/bindings.rs b/packages/compiler/src/shared/bindings.rs index 6bd9e4528..c651bca47 100644 --- a/packages/compiler/src/shared/bindings.rs +++ b/packages/compiler/src/shared/bindings.rs @@ -189,6 +189,13 @@ impl BindingTable { } } + /// Whether `name` resolves to any tracked binding. Global identifiers + /// (`undefined`, `NaN`, `Infinity`, ...) return false — they are not + /// assignment targets and must pass through untouched. + pub(crate) fn is_declared(&self, name: &str) -> bool { + self.resolve(name).is_some() + } + pub(crate) fn is_const(&self, name: &str) -> bool { self.resolve(name).is_some_and(|binding| binding.is_const) } diff --git a/packages/compiler/src/shared/refs.rs b/packages/compiler/src/shared/refs.rs index 853973cbb..753cfef3e 100644 --- a/packages/compiler/src/shared/refs.rs +++ b/packages/compiler/src/shared/refs.rs @@ -11,6 +11,9 @@ pub(crate) trait RefPropertyContext<'a> { /// `binding.kind === "const" || binding.kind === "module"` for a plain /// identifier ref target. fn is_const_ref_binding(&self, name: &str) -> bool; + /// Whether `name` resolves to a tracked binding (`false` for globals + /// like `undefined`). + fn is_declared_ref_binding(&self, name: &str) -> bool; fn next_ref_id(&mut self) -> String; fn mark_uses_apply_ref(&mut self); } @@ -24,6 +27,10 @@ impl<'a> RefPropertyContext<'a> for AstDomTransform<'a, '_> { self.bindings.is_const(name) } + fn is_declared_ref_binding(&self, name: &str) -> bool { + self.bindings.is_declared(name) + } + fn next_ref_id(&mut self) -> String { AstDomTransform::next_ref_id(self) } @@ -36,7 +43,7 @@ impl<'a> RefPropertyContext<'a> for AstDomTransform<'a, '_> { pub(crate) fn component_ref_property<'a, C: RefPropertyContext<'a>>( ctx: &mut C, span: Span, - value: Expression<'a>, + mut value: Expression<'a>, setup: &mut std::vec::Vec>, ) -> Option> { let allocator = ctx.allocator(); @@ -63,6 +70,10 @@ pub(crate) fn component_ref_property<'a, C: RefPropertyContext<'a>>( return Some(object_property(value)); } } + // Bare lvals inside a `ref` array literal (`ref={[el]}`) must be lowered + // to assignment callbacks, mirroring `transformRefArrayLiteral` in the + // babel plugin (https://github.com/solidjs/solid/issues/3285). + value = transform_ref_array_literal(ctx, span, value); if matches!( value, Expression::ArrowFunctionExpression(_) @@ -133,6 +144,138 @@ pub(crate) fn ref_assignment_fallback<'a>( assignment_fallback(ctx.allocator, span, value, assignment_value) } +/// Rewrite bare assignment targets inside a `ref` array expression +/// (`ref={[el]}`) into callback refs, mirroring `transformRefArrayLiteral` +/// in the babel plugin (https://github.com/solidjs/solid/issues/3285). +/// +/// The runtime `applyRef` flattens ref arrays and only *calls* their entries; +/// it never assigns anything. A bare variable evaluates to its current value +/// (`undefined`) at mount time, so without this rewrite the ref is silently +/// dropped. Non-array values pass through unchanged; nested arrays are +/// recursed so runtime flattening still reaches every rewritten callback. +pub(crate) fn transform_ref_array_literal<'a, C: RefPropertyContext<'a>>( + ctx: &mut C, + span: Span, + mut value: Expression<'a>, +) -> Expression<'a> { + if let Expression::ArrayExpression(array) = &mut value { + transform_ref_array_elements(ctx, span, &mut array.elements); + } + value +} + +fn transform_ref_array_elements<'a, C: RefPropertyContext<'a>>( + ctx: &mut C, + span: Span, + elements: &mut oxc_allocator::Vec<'a, oxc_ast::ast::ArrayExpressionElement<'a>>, +) { + for element in elements.iter_mut() { + // Spread elements and elisions have no expression to rewrite. + let Some(expression) = element.as_expression_mut() else { + continue; + }; + if let Expression::ArrayExpression(inner) = expression { + transform_ref_array_elements(ctx, span, &mut inner.elements); + continue; + } + let is_lval_target = match expression { + Expression::Identifier(identifier) => { + let name = identifier.name.as_str(); + // Only declared, mutable bindings are assignment targets; + // globals (`undefined`, ...) and const/module refs pass + // through untouched. + ctx.is_declared_ref_binding(name) && !ctx.is_const_ref_binding(name) + } + Expression::StaticMemberExpression(member) => !member.optional, + Expression::ComputedMemberExpression(member) => !member.optional, + _ => false, + }; + if is_lval_target { + let target = expression.clone_in(ctx.allocator()); + *expression = ref_lval_callback(ctx, span, target); + } + } +} + +/// Build the callback-ref form the non-array lval branch lowers to: +/// `(param) => { var cached = ; return typeof cached === "function" +/// ? cached(param) : = param; }` +/// +/// Reading the target once preserves mutable bindings that currently hold a +/// function (`let cb = fn; ref={[cb]}` calls `fn` instead of overwriting +/// `cb`); const/module refs never reach this path and keep their +/// pass-by-reference semantics. +fn ref_lval_callback<'a, C: RefPropertyContext<'a>>( + ctx: &mut C, + span: Span, + target: Expression<'a>, +) -> Expression<'a> { + let allocator = ctx.allocator(); + let ast = crate::shared::ast_builder::AstBuilder::new(allocator); + let param_id = ctx.next_ref_id(); + let cached_id = ctx.next_ref_id(); + let param = ast.expression_identifier(span, ast.ident(¶m_id)); + let cached = ast.expression_identifier(span, ast.ident(&cached_id)); + + let mut statements = ast.vec(); + statements.push(crate::shared::ast::variable_statement( + allocator, + span, + VariableDeclarationKind::Var, + &cached_id, + target.clone_in(allocator), + )); + // Match the babel helper's test exactly: `typeof cached === "function"` + // (no `Array.isArray` arm — an element can never be a ref array here). + let test = ast.expression_binary( + span, + ast.expression_unary( + span, + oxc_ast::ast::UnaryOperator::Typeof, + cached.clone_in(allocator), + ), + oxc_ast::ast::BinaryOperator::StrictEquality, + ast.expression_string_literal(span, ast.str("function"), None), + ); + let call = ast.expression_call( + span, + cached.clone_in(allocator), + None, + ast.vec1(crate::shared::ast::expression_to_argument( + param.clone_in(allocator), + )), + false, + ); + let fallback = assignment_fallback(allocator, span, &target, param.clone_in(allocator)) + .expect("ref array lval targets have an assignment fallback"); + // Expression statement (not a `return`) so the generated arrow body + // matches the babel plugin's output byte-for-byte. + statements.push(ast.statement_expression( + span, + ast.expression_conditional(span, test, call, fallback), + )); + + let params = ast.vec1(ast.formal_parameter( + span, + ast.vec(), + ast.binding_pattern_binding_identifier(span, ast.ident(¶m_id)), + None, + None, + false, + None, + false, + false, + )); + let params = ast.formal_parameters( + span, + oxc_ast::ast::FormalParameterKind::ArrowFormalParameters, + params, + None, + ); + let body = ast.function_body(span, ast.vec(), statements); + ast.expression_arrow_function(span, false, false, None, params, None, body) +} + /// `typeof === "function" || Array.isArray()` pub(crate) fn callable_test<'a>( allocator: &'a oxc_allocator::Allocator, diff --git a/packages/compiler/src/ssr/transform.rs b/packages/compiler/src/ssr/transform.rs index 7984ab6cf..07198deb0 100644 --- a/packages/compiler/src/ssr/transform.rs +++ b/packages/compiler/src/ssr/transform.rs @@ -2997,6 +2997,10 @@ impl<'a> crate::shared::refs::RefPropertyContext<'a> for AstSsrTransform<'a, '_> self.bindings.is_const(name) } + fn is_declared_ref_binding(&self, name: &str) -> bool { + self.bindings.is_declared(name) + } + fn next_ref_id(&mut self) -> String { AstSsrTransform::next_ref_id(self) } diff --git a/packages/compiler/src/universal/transform.rs b/packages/compiler/src/universal/transform.rs index 7d0dd6d96..32d76dc06 100644 --- a/packages/compiler/src/universal/transform.rs +++ b/packages/compiler/src/universal/transform.rs @@ -842,6 +842,9 @@ impl<'a, 'source> AstUniversalTransform<'a, 'source> { mut value: Expression<'a>, ) -> std::vec::Vec> { self.visit_expression(&mut value); + // Lower bare lvals inside `ref={[el]}` arrays to assignment callbacks + // (https://github.com/solidjs/solid/issues/3285). + value = crate::shared::refs::transform_ref_array_literal(self, span, value); let elem = self.identifier_expression(span, element_id); let is_constant = matches!(&value, Expression::Identifier(identifier) if self.bindings.is_const(identifier.name.as_str())); @@ -1360,7 +1363,7 @@ impl<'a, 'source> AstUniversalTransform<'a, 'source> { fn universal_component_ref( &mut self, span: Span, - value: Expression<'a>, + mut value: Expression<'a>, setup: &mut std::vec::Vec>, ) -> Option> { if let Expression::Identifier(identifier) = &value { @@ -1369,9 +1372,15 @@ impl<'a, 'source> AstUniversalTransform<'a, 'source> { return Some(self.object_property(span, "ref", value)); } } + // Lower bare lvals inside `ref={[el]}` arrays to assignment callbacks + // (https://github.com/solidjs/solid/issues/3285), then pass the array + // through like the shared component transform does. + value = crate::shared::refs::transform_ref_array_literal(self, span, value); if matches!( value, - Expression::ArrowFunctionExpression(_) | Expression::FunctionExpression(_) + Expression::ArrowFunctionExpression(_) + | Expression::FunctionExpression(_) + | Expression::ArrayExpression(_) ) { return Some(self.object_property(span, "ref", value)); } @@ -1860,6 +1869,28 @@ impl<'a> crate::shared::component::ComponentLower<'a> for AstUniversalTransform< } } +impl<'a> crate::shared::refs::RefPropertyContext<'a> for AstUniversalTransform<'a, '_> { + fn allocator(&self) -> &'a Allocator { + self.allocator + } + + fn is_const_ref_binding(&self, name: &str) -> bool { + self.bindings.is_const(name) + } + + fn is_declared_ref_binding(&self, name: &str) -> bool { + self.bindings.is_declared(name) + } + + fn next_ref_id(&mut self) -> String { + AstUniversalTransform::next_ref_id(self) + } + + fn mark_uses_apply_ref(&mut self) { + self.uses_apply_ref = true; + } +} + impl<'a> ComponentPropContext<'a> for AstUniversalTransform<'a, '_> { fn allocator(&self) -> &'a Allocator { self.allocator diff --git a/packages/web/test/ref-array.spec.tsx b/packages/web/test/ref-array.spec.tsx new file mode 100644 index 000000000..6de67d618 --- /dev/null +++ b/packages/web/test/ref-array.spec.tsx @@ -0,0 +1,80 @@ +/** + * @jsxImportSource @solidjs/web + * @vitest-environment jsdom + */ +import { describe, expect, test, vi } from "vitest"; +import { createRoot } from "solid-js"; + +describe("ref arrays", () => { + test("assigns a bare variable inside a ref array (#3285)", () => { + let elementRef: HTMLDivElement | undefined; + const el = createRoot(() =>
) as HTMLDivElement; + expect(elementRef).toBe(el); + }); + + test("assigns multiple bare variables inside a ref array", () => { + let a: HTMLDivElement | undefined, b: HTMLDivElement | undefined; + const el = createRoot(() =>
) as HTMLDivElement; + expect(a).toBe(el); + expect(b).toBe(el); + }); + + test("still invokes callback refs inside an array", () => { + const calls: HTMLDivElement[] = []; + const el = createRoot(() => ( +
calls.push(node)]} /> + )) as HTMLDivElement; + expect(calls).toEqual([el]); + }); + + test("mixes bare variables and callbacks in one array", () => { + let viaVar: HTMLDivElement | undefined; + let viaCb: HTMLDivElement | undefined; + const el = createRoot(() => ( +
{ + viaCb = node; + } + ]} + /> + )) as HTMLDivElement; + expect(viaVar).toBe(el); + expect(viaCb).toBe(el); + }); + + test("flattens nested arrays of bare variables", () => { + let deep: HTMLDivElement | undefined; + const el = createRoot(() =>
) as HTMLDivElement; + expect(deep).toBe(el); + }); + + test("assigns member-expression targets inside an array", () => { + const holder: { el?: HTMLDivElement } = {}; + const el = createRoot(() =>
) as HTMLDivElement; + expect(holder.el).toBe(el); + }); + + test("invokes a const function ref passed by reference in an array", () => { + const useEl = vi.fn(); + const el = createRoot(() =>
) as HTMLDivElement; + expect(useEl).toHaveBeenCalledTimes(1); + expect(useEl).toHaveBeenCalledWith(el); + }); + + test("invokes a mutable binding holding a function instead of overwriting it", () => { + const calls: HTMLDivElement[] = []; + let cb: ((node: HTMLDivElement) => void) | undefined = (node: HTMLDivElement) => + calls.push(node); + const el = createRoot(() =>
) as HTMLDivElement; + expect(calls).toEqual([el]); + expect(cb).toBeTypeOf("function"); + }); + + test("tolerates falsy slots in a ref array", () => { + let elementRef: HTMLDivElement | undefined; + const el = createRoot(() =>
) as HTMLDivElement; + expect(elementRef).toBe(el); + }); +});