From 47ac2ccd17031dca576175e79b8196db615b5c35 Mon Sep 17 00:00:00 2001 From: Nate Moore Date: Thu, 24 Jun 2021 17:41:10 -0500 Subject: [PATCH] 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) --- .changeset/shy-cats-brake.md | 6 ++ .../astro-parser/src/parse/read/expression.ts | 1 - packages/astro-parser/src/parse/state/tag.ts | 3 +- packages/astro/src/compiler/codegen/index.ts | 88 +++++++++++++------ packages/astro/test/astro-basic.test.js | 27 ++++++ .../astro-basic/src/pages/spread.astro | 14 +++ 6 files changed, 107 insertions(+), 32 deletions(-) create mode 100644 .changeset/shy-cats-brake.md create mode 100644 packages/astro/test/fixtures/astro-basic/src/pages/spread.astro 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) { 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 { +async function getAttributes(attrs: Attribute[], state: CodegenState, compileOptions: CompileOptions): Promise> { let result: Record = {}; 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 { } 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 { 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 }; +--- + + + + + + +
+
+
+ +