Fix falsy values (#275)

* fix(#274): improve attribute handling

* chore: add test for JSX expressions

* fix: falsy expressions should not render

* chore: add changeset

* test: update expression tests

* fix: render 0 if value is {0}

.changeset/cold-paws-remember.md

@@ -0,0 +1,7 @@
+---
+'astro': patch
+---
+
+Fixed a bug where Astro did not conform to JSX Expressions' [`&&`](https://reactjs.org/docs/conditional-rendering.html#inline-if-with-logical--operator) syntax.
+
+Also fixed a bug where `<span data-attr="" />` would render as `<span data-attr="undefined" />`.

packages/astro/src/ast.ts

@@ -11,6 +11,7 @@ export function getAttr(attributes: Attribute[], name: string): Attribute | unde
 
 /** Get TemplateNode attribute by value */
 export function getAttrValue(attributes: Attribute[], name: string): string | undefined {
+  if (attributes.length === 0) return '';
   const attr = getAttr(attributes, name);
   if (attr) {
     return attr.value[0]?.data;

packages/astro/src/compiler/codegen/index.ts

@@ -58,6 +58,10 @@ function getAttributes(attrs: Attribute[]): Record<string, string> {
       // note: attr.value shouldn’t be `undefined`, but a bad transform would cause a compile error here, so prevent that
       continue;
     }
+    if (attr.value.length === 0) {
+      result[attr.name] = '""';
+      continue;
+    }
     if (attr.value.length > 1) {
       result[attr.name] =
         '(' +
@@ -418,6 +422,8 @@ function dedent(str: string) {
   return !arr || !first ? str : str.replace(new RegExp(`^[ \\t]{0,${first}}`, 'gm'), '');
 }
 
+const FALSY_EXPRESSIONS = new Set(['false','null','undefined','void 0']);
+
 /** Compile page markup */
 async function compileHtml(enterNode: TemplateNode, state: CodegenState, compileOptions: CompileOptions): Promise<string> {
   return new Promise((resolve) => {
@@ -475,11 +481,13 @@ async function compileHtml(enterNode: TemplateNode, state: CodegenState, compile
             }
             // TODO Do we need to compile this now, or should we compile the entire module at the end?
             let code = compileExpressionSafe(raw).trim().replace(/\;$/, '');
+            if (!FALSY_EXPRESSIONS.has(code)) {
               if (state.markers.insideMarkdown) {
                 buffers[curr] += `{${code}}`;
               } else {
                 buffers[curr] += `,(${code})`;
               }
+            }
             this.skip();
             break;
           }

packages/astro/src/frontend/h.ts

@@ -19,7 +19,9 @@ function* _h(tag: string, attrs: HProps, children: Array<HChild>) {
   yield `<${tag}`;
   if (attrs) {
     for (let [key, value] of Object.entries(attrs)) {
-      yield ` ${key}="${value}"`;
+      if (value === '') yield ` ${key}=""`;
+      else if (value == null) yield '';
+      else yield ` ${key}="${value}"`;
     }
   }
   yield '>';
@@ -37,7 +39,7 @@ function* _h(tag: string, attrs: HProps, children: Array<HChild>) {
       yield child();
     } else if (typeof child === 'string') {
       yield child;
-    } else if (!child) {
+    } else if (!child && child !== 0) {
       // do nothing, safe to ignore falsey values.
     } else {
       yield child;

packages/astro/test/astro-attrs.test.js

@@ -0,0 +1,28 @@
+import { suite } from 'uvu';
+import * as assert from 'uvu/assert';
+import { doc } from './test-utils.js';
+import { setup } from './helpers.js';
+
+const Attributes = suite('Attributes test');
+
+setup(Attributes, './fixtures/astro-attrs');
+
+Attributes('Passes attributes to elements as expected', async ({ runtime }) => {
+  const result = await runtime.load('/');
+  if (result.error) throw new Error(result.error);
+
+  const $ = doc(result.contents);
+
+  const ids = ['false-str', 'true-str', 'false', 'true', 'empty', 'null', 'undefined'];
+  const specs = ['false', 'true', 'false', 'true', '', undefined, undefined];
+
+  let i = 0;
+  for (const id of ids) {
+    const spec = specs[i];
+    const attr = $(`#${id}`).attr('attr');
+    assert.equal(attr, spec, `Passes ${id} as "${spec}"`);
+    i++;
+  }
+});
+
+Attributes.run();

packages/astro/test/astro-expr.test.js

@@ -58,4 +58,17 @@ Expressions('Allows multiple JSX children in mustache', async ({ runtime }) => {
   assert.ok(result.contents.includes('#f') && !result.contents.includes('#t'));
 });
 
+Expressions('Does not render falsy values using &&', async ({ runtime }) => {
+  const result = await runtime.load('/falsy');
+  if (result.error) throw new Error(result.error);
+  
+  const $ = doc(result.contents);
+  
+  assert.equal($('#true').length, 1, `Expected {true && <span id="true" />} to render`);
+  assert.equal($('#zero').text(), '0', `Expected {0 && "VALUE"} to render "0"`);
+  assert.equal($('#false').length, 0, `Expected {false && <span id="false" />} not to render`);
+  assert.equal($('#null').length, 0, `Expected {null && <span id="null" />} not to render`);
+  assert.equal($('#undefined').length, 0, `Expected {undefined && <span id="undefined" />} not to render`);
+});
+
 Expressions.run();

packages/astro/test/fixtures/astro-attrs/snowpack.config.json

@@ -0,0 +1,3 @@
+{
+  "workspaceRoot": "../../../../../"
+}

packages/astro/test/fixtures/astro-attrs/src/pages/index.astro

@@ -0,0 +1,8 @@
+<span id="false-str" attr="false" />
+<span id="true-str" attr="true" />
+<span id="true" attr={true} />
+<span id="false" attr={false} />
+<span id="empty" attr="" />
+<span id="null" attr={null} />
+<span id="undefined" attr={undefined} />
+

packages/astro/test/fixtures/astro-expr/src/pages/falsy.astro

@@ -0,0 +1,12 @@
+<html lang="en">
+<head>
+  <title>My site</title>
+</head>
+<body>
+  {false && <span id="false" />}
+  {null && <span id="null" />}
+  {undefined && <span id="undefined" />}
+  {true && <span id="true" />}
+  <span id="zero">{0 && "VALUE"}</span>
+</body>
+</html>