Fix `{...spread}` props (#522)

* fix(#521): allow spread props

* chore: add spread prop tests

* fix: falsy expressions should only be skipped in 'Expression' case

* fix: support primitives in expressions (objects, arrays)

6 files changed, 107 insertions, 32 deletions

diff --git a/.changeset/shy-cats-brake.md b/.changeset/shy-cats-brake.md
new file mode 100644
index 000000000..abeedd148
--- /dev/null
+++ b/.changeset/shy-cats-brake.md
@@ -0,0 +1,6 @@
+---
+'astro': patch
+'@astrojs/parser': patch
+---
+
+Fix #521, allowing `{...spread}` props to work again
diff --git a/packages/astro-parser/src/parse/read/expression.ts b/packages/astro-parser/src/parse/read/expression.ts
index c1fd6031b..98d94e26a 100644
--- a/packages/astro-parser/src/parse/read/expression.ts
+++ b/packages/astro-parser/src/parse/read/expression.ts
@@ -240,7 +240,6 @@ export const parse_expression_at = (source: string, index: number): Expression =
   return expression;
 };
 
-// @ts-ignore
 export default function read_expression(parser: Parser) {
   try {
     const expression = parse_expression_at(parser.template, parser.index);
diff --git a/packages/astro-parser/src/parse/state/tag.ts b/packages/astro-parser/src/parse/state/tag.ts
index 28783df67..719baa55d 100644
--- a/packages/astro-parser/src/parse/state/tag.ts
+++ b/packages/astro-parser/src/parse/state/tag.ts
@@ -349,8 +349,7 @@ function read_attribute(parser: Parser, unique_names: Set<string>) {
     parser.allow_whitespace();
 
     if (parser.eat('...')) {
-      const { expression } = read_expression(parser);
-
+      const expression = read_expression(parser);
       parser.allow_whitespace();
       parser.eat('}', true);
 
diff --git a/packages/astro/src/compiler/codegen/index.ts b/packages/astro/src/compiler/codegen/index.ts
index 531c502f7..0455ee0dd 100644
--- a/packages/astro/src/compiler/codegen/index.ts
+++ b/packages/astro/src/compiler/codegen/index.ts
@@ -1,4 +1,4 @@
-import type { Ast, Script, Style, TemplateNode } from '@astrojs/parser';
+import type { Ast, Script, Style, TemplateNode, Expression } from '@astrojs/parser';
 import type { CompileOptions } from '../../@types/compiler';
 import type { AstroConfig, AstroMarkdownOptions, TransformResult, ComponentInfo, Components } from '../../@types/astro';
 import type { ImportDeclaration, ExportNamedDeclaration, VariableDeclarator, Identifier, ImportDefaultSpecifier } from '@babel/types';
@@ -17,11 +17,10 @@ import { error, warn, parseError } from '../../logger.js';
 import { fetchContent } from './content.js';
 import { isFetchContent } from './utils.js';
 import { yellow } from 'kleur/colors';
-import { isComponentTag } from '../utils';
+import { isComponentTag, positionAt } from '../utils.js';
 import { renderMarkdown } from '@astrojs/markdown-support';
 import { transform } from '../transform/index.js';
 import { PRISM_IMPORT } from '../transform/prism.js';
-import { positionAt } from '../utils';
 import { nodeBuiltinsSet } from '../../node_builtins.js';
 import { readFileSync } from 'fs';
 
@@ -34,9 +33,10 @@ const { transformSync } = esbuild;
 interface Attribute {
   start: number;
   end: number;
-  type: 'Attribute';
+  type: 'Attribute'|'Spread';
   name: string;
   value: TemplateNode[] | boolean;
+  expression?: Expression;
 }
 
 interface CodeGenOptions {
@@ -46,9 +46,16 @@ interface CodeGenOptions {
 }
 
 /** Retrieve attributes from TemplateNode */
-function getAttributes(attrs: Attribute[]): Record<string, string> {
+async function getAttributes(attrs: Attribute[], state: CodegenState, compileOptions: CompileOptions): Promise<Record<string, string>> {
   let result: Record<string, string> = {};
   for (const attr of attrs) {
+    if (attr.type === 'Spread') {
+      const code = await compileExpression(attr.expression as Expression, state, compileOptions);
+      if (code) {
+        result[`...(${code})`] = '';
+      }
+      continue;
+    }
     if (attr.value === true) {
       result[attr.name] = JSON.stringify(attr.value);
       continue;
@@ -83,18 +90,18 @@ function getAttributes(attrs: Attribute[]): Record<string, string> {
     }
     switch (val.type) {
       case 'MustacheTag': {
-        // FIXME: this won't work when JSX element can appear in attributes (rare but possible).
-        const codeChunks = val.expression.codeChunks[0];
-        if (codeChunks) {
-          result[attr.name] = '(' + codeChunks + ')';
-        } else {
-          throw new Error(`Parse error: ${attr.name}={}`); // if bad codeChunk, throw error
+        const code = await compileExpression(val.expression, state, compileOptions);
+        if (code) {
+          result[attr.name] = '(' + code + ')';
         }
         continue;
       }
       case 'Text':
         result[attr.name] = JSON.stringify(getTextFromAttribute(val));
         continue;
+      case 'AttributeShorthand':
+        result[attr.name] = '(' + attr.name + ')';
+        continue;
       default:
         throw new Error(`UNKNOWN: ${val.type}`);
     }
@@ -126,7 +133,11 @@ function getTextFromAttribute(attr: any): string {
 function generateAttributes(attrs: Record<string, string>): string {
   let result = '{';
   for (const [key, val] of Object.entries(attrs)) {
-    result += JSON.stringify(key) + ':' + val + ',';
+    if (key.startsWith('...')) {
+      result += key + ',';
+    } else {
+      result += JSON.stringify(key) + ':' + val + ',';
+    }
   }
   return result + '}';
 }
@@ -172,11 +183,37 @@ function getComponentWrapper(_name: string, { url, importSpecifier }: ComponentI
   };
 }
 
+/**
+ * Convert an Expression Node to a string
+ *
+ * @param expression Expression Node to compile
+ * @param state CodegenState
+ * @param compileOptions CompileOptions
+ */
+async function compileExpression(node: Expression, state: CodegenState, compileOptions: CompileOptions) {
+  const children: string[] = await Promise.all((node.children ?? []).map((child) => compileHtml(child, state, compileOptions)));
+  let raw = '';
+  let nextChildIndex = 0;
+  for (const chunk of node.codeChunks) {
+    raw += chunk;
+    if (nextChildIndex < children.length) {
+      raw += children[nextChildIndex++];
+    }
+  }
+  const location = { start: node.start, end: node.end };
+  let code = transpileExpressionSafe(raw, { state, compileOptions, location });
+  if (code === null) throw new Error(`Unable to compile expression`);
+  code = code.trim().replace(/\;$/, '');
+  return code;
+}
+
 /** Evaluate expression (safely) */
-function compileExpressionSafe(
+function transpileExpressionSafe(
   raw: string,
   { state, compileOptions, location }: { state: CodegenState; compileOptions: CompileOptions; location: { start: number; end: number } }
 ): string | null {
+  // We have to wrap `raw` with parens to support primitives (objects, arrays)!
+  raw = `(${raw})`;
   try {
     let { code } = transformSync(raw, {
       loader: 'tsx',
@@ -497,21 +534,12 @@ async function compileHtml(enterNode: TemplateNode, state: CodegenState, compile
       async enter(node: TemplateNode, parent: TemplateNode) {
         switch (node.type) {
           case 'Expression': {
-            const children: string[] = await Promise.all((node.children ?? []).map((child) => compileHtml(child, state, compileOptions)));
-            let raw = '';
-            let nextChildIndex = 0;
-            for (const chunk of node.codeChunks) {
-              raw += chunk;
-              if (nextChildIndex < children.length) {
-                raw += children[nextChildIndex++];
-              }
+            let code = await compileExpression(node as Expression, state, compileOptions);
+            if (FALSY_EXPRESSIONS.has(code)) {
+              this.skip();
+              break;
             }
-            const location = { start: node.start, end: node.end };
-            // TODO Do we need to compile this now, or should we compile the entire module at the end?
-            let code = compileExpressionSafe(raw, { state, compileOptions, location });
-            if (code === null) throw new Error(`Unable to compile expression`);
-            code = code.trim().replace(/\;$/, '');
-            if (!FALSY_EXPRESSIONS.has(code)) {
+            if (code !== '') {
               if (state.markers.insideMarkdown) {
                 buffers[curr] += `{${code}}`;
               } else {
@@ -566,7 +594,7 @@ async function compileHtml(enterNode: TemplateNode, state: CodegenState, compile
               throw new Error('AHHHH');
             }
             try {
-              const attributes = getAttributes(node.attributes);
+              const attributes = await getAttributes(node.attributes, state, compileOptions);
 
               buffers.out += buffers.out === '' ? '' : ',';
 
@@ -623,7 +651,8 @@ async function compileHtml(enterNode: TemplateNode, state: CodegenState, compile
             }
             return;
           }
-          case 'Attribute': {
+          case 'Attribute':
+          case 'Spread': {
             this.skip();
             return;
           }
@@ -671,6 +700,7 @@ async function compileHtml(enterNode: TemplateNode, state: CodegenState, compile
           }
           case 'Text':
           case 'Attribute':
+          case 'Spread':
           case 'Comment':
           case 'Expression':
           case 'MustacheTag':
diff --git a/packages/astro/test/astro-basic.test.js b/packages/astro/test/astro-basic.test.js
index cf964e3b1..38f62085e 100644
--- a/packages/astro/test/astro-basic.test.js
+++ b/packages/astro/test/astro-basic.test.js
@@ -68,4 +68,31 @@ Basics('Allows forward-slashes in mustache tags (#407)', async ({ runtime }) =>
   assert.equal($('a[href="/post/three"]').length, 1);
 });
 
+Basics('Allows spread attributes (#521)', async ({ runtime }) => {
+  const result = await runtime.load('/spread');
+  const html = result.contents;
+  const $ = doc(html);
+
+  assert.equal($('#spread-leading').length, 1);
+  assert.equal($('#spread-leading').attr('a'), '0');
+  assert.equal($('#spread-leading').attr('b'), '1');
+  assert.equal($('#spread-leading').attr('c'), '2');
+
+  assert.equal($('#spread-trailing').length, 1);
+  assert.equal($('#spread-trailing').attr('a'), '0');
+  assert.equal($('#spread-trailing').attr('b'), '1');
+  assert.equal($('#spread-trailing').attr('c'), '2');
+});
+
+Basics('Allows spread attributes with TypeScript (#521)', async ({ runtime }) => {
+  const result = await runtime.load('/spread');
+  const html = result.contents;
+  const $ = doc(html);
+
+  assert.equal($('#spread-ts').length, 1);
+  assert.equal($('#spread-ts').attr('a'), '0');
+  assert.equal($('#spread-ts').attr('b'), '1');
+  assert.equal($('#spread-ts').attr('c'), '2');
+});
+
 Basics.run();
diff --git a/packages/astro/test/fixtures/astro-basic/src/pages/spread.astro b/packages/astro/test/fixtures/astro-basic/src/pages/spread.astro
new file mode 100644
index 000000000..fc1e06ed1
--- /dev/null
+++ b/packages/astro/test/fixtures/astro-basic/src/pages/spread.astro
@@ -0,0 +1,14 @@
+---
+const spread = { a: 0, b: 1, c: 2 };
+---
+
+<html>
+  <head>
+    <!-- Head Stuff -->
+  </head>
+  <body>
+    <div {...spread} id="spread-leading" />
+    <div id="spread-trailing" {...spread} />
+    <div id="spread-ts" {...(spread as any)} />
+  </body>
+</html>