From 3d20623c32768f9224ca8e33888b761f957fff61 Mon Sep 17 00:00:00 2001
From: Nate Moore <natemoo-re@users.noreply.github.com>
Date: Fri, 28 May 2021 17:02:04 -0500
Subject: [PATCH] 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              |  7 +++++
 packages/astro/src/ast.ts                     |  1 +
 packages/astro/src/compiler/codegen/index.ts  | 16 ++++++++---
 packages/astro/src/frontend/h.ts              |  6 ++--
 packages/astro/test/astro-attrs.test.js       | 28 +++++++++++++++++++
 packages/astro/test/astro-expr.test.js        | 13 +++++++++
 .../fixtures/astro-attrs/snowpack.config.json |  3 ++
 .../astro-attrs/src/pages/index.astro         |  8 ++++++
 .../fixtures/astro-expr/src/pages/falsy.astro | 12 ++++++++
 9 files changed, 88 insertions(+), 6 deletions(-)
 create mode 100644 .changeset/cold-paws-remember.md
 create mode 100644 packages/astro/test/astro-attrs.test.js
 create mode 100644 packages/astro/test/fixtures/astro-attrs/snowpack.config.json
 create mode 100644 packages/astro/test/fixtures/astro-attrs/src/pages/index.astro
 create mode 100644 packages/astro/test/fixtures/astro-expr/src/pages/falsy.astro

diff --git a/.changeset/cold-paws-remember.md b/.changeset/cold-paws-remember.md
new file mode 100644
index 000000000..482d8e94d
--- /dev/null
+++ b/.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" />`.
diff --git a/packages/astro/src/ast.ts b/packages/astro/src/ast.ts
index f80084cb2..bee1d9b6f 100644
--- a/packages/astro/src/ast.ts
+++ b/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;
diff --git a/packages/astro/src/compiler/codegen/index.ts b/packages/astro/src/compiler/codegen/index.ts
index 97687b4a9..9271ab50a 100644
--- a/packages/astro/src/compiler/codegen/index.ts
+++ b/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,10 +481,12 @@ 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 (state.markers.insideMarkdown) {
-              buffers[curr] += `{${code}}`;
-            } else {
-              buffers[curr] += `,(${code})`;
+            if (!FALSY_EXPRESSIONS.has(code)) {
+              if (state.markers.insideMarkdown) {
+                buffers[curr] += `{${code}}`;
+              } else {
+                buffers[curr] += `,(${code})`;
+              }
             }
             this.skip();
             break;
diff --git a/packages/astro/src/frontend/h.ts b/packages/astro/src/frontend/h.ts
index ae54072a8..bcfb4833f 100644
--- a/packages/astro/src/frontend/h.ts
+++ b/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;
diff --git a/packages/astro/test/astro-attrs.test.js b/packages/astro/test/astro-attrs.test.js
new file mode 100644
index 000000000..fd58769a9
--- /dev/null
+++ b/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();
diff --git a/packages/astro/test/astro-expr.test.js b/packages/astro/test/astro-expr.test.js
index c424c3863..04a005aae 100644
--- a/packages/astro/test/astro-expr.test.js
+++ b/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();
diff --git a/packages/astro/test/fixtures/astro-attrs/snowpack.config.json b/packages/astro/test/fixtures/astro-attrs/snowpack.config.json
new file mode 100644
index 000000000..8f034781d
--- /dev/null
+++ b/packages/astro/test/fixtures/astro-attrs/snowpack.config.json
@@ -0,0 +1,3 @@
+{
+  "workspaceRoot": "../../../../../"
+}
diff --git a/packages/astro/test/fixtures/astro-attrs/src/pages/index.astro b/packages/astro/test/fixtures/astro-attrs/src/pages/index.astro
new file mode 100644
index 000000000..40f1a1f0a
--- /dev/null
+++ b/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} />
+
diff --git a/packages/astro/test/fixtures/astro-expr/src/pages/falsy.astro b/packages/astro/test/fixtures/astro-expr/src/pages/falsy.astro
new file mode 100644
index 000000000..294f71684
--- /dev/null
+++ b/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>