Fix set:html behavior with null (#2790)

* feat: improve set:html behavior for null/undefined * chore: add changeset * refactor: improve set:html and set:text documentation * test: improve set:html tests * refactor: better types for server API
withastro · Mar 14, 2022 · 6b34840 · 6b34840
1 parent de2b246
commit 6b34840
Show file tree

Hide file tree

Showing 5 changed files with 57 additions and 6 deletions.
diff --git a/.changeset/serious-guests-matter.md b/.changeset/serious-guests-matter.md
@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Improve `set:html` behavior for `null` and `undefined` values
diff --git a/packages/astro/src/runtime/server/escape.ts b/packages/astro/src/runtime/server/escape.ts
@@ -1,6 +1,7 @@
 const entities = { '"': 'quot', '&': 'amp', "'": 'apos', '<': 'lt', '>': 'gt' } as const;
 
-export const escapeHTML = (string: any) => string.replace(/["'&<>]/g, (char: keyof typeof entities) => '&' + entities[char] + ';');
+// This util is only ever run on expression values that we already know are of type `string`
+export const escapeHTML = (str: string) => str.replace(/["'&<>]/g, (char: string) => '&' + entities[char as keyof typeof entities] + ';');
 
 /**
  * RawString is a "blessed" version of String
@@ -11,7 +12,14 @@ export class UnescapedString extends String {}
 /**
  * unescapeHTML marks a string as raw, unescaped HTML.
  * This should only be generated internally, not a public API.
- *
- * Need to cast the return value `as unknown as string` so TS doesn't yell at us.
  */
-export const unescapeHTML = (str: any) => new UnescapedString(str) as unknown as string;
+export const unescapeHTML = (value: any) => {
+ // Cast any `string` values to `UnescapedString` to mark them as ignored
+ // The `as unknown as string` is necessary for TypeScript to treat this as `string`
+ if (typeof value === 'string') {
+ return new UnescapedString(value) as unknown as string;
+ }
+ // Just return values that are `number`, `null`, `undefined` etc
+ // The compiler will recursively stringify these correctly
+ return value
+}
diff --git a/packages/astro/src/runtime/server/index.ts b/packages/astro/src/runtime/server/index.ts
@@ -418,7 +418,7 @@ export async function renderEndpoint(mod: EndpointHandler, params: any) {
 }
 
 // Calls a component and renders it into a string of HTML
-export async function renderToString(result: SSRResult, componentFactory: AstroComponentFactory, props: any, children: any) {
+export async function renderToString(result: SSRResult, componentFactory: AstroComponentFactory, props: any, children: any): Promise<string> {
  const Component = await componentFactory(result, props, children);
  let template = await renderAstroComponent(Component);
 
@@ -439,7 +439,7 @@ const uniqueElements = (item: any, index: number, all: any[]) => {
 
 // Renders a page to completion by first calling the factory callback, waiting for its result, and then appending
 // styles and scripts into the head.
-export async function renderHead(result: SSRResult) {
+export async function renderHead(result: SSRResult): Promise<string> {
  const styles = Array.from(result.styles)
  .filter(uniqueElements)
  .map((style) => {

diff --git a/packages/astro/test/astro-directives.test.js b/packages/astro/test/astro-directives.test.js
@@ -17,4 +17,30 @@ describe('Directives', async () => {
  expect($('script#inline')).to.have.lengthOf(1);
  expect($('script#inline').toString()).to.include('let foo = "bar"');
  });
+
+ it('set:html', async () => {
+ const html = await fixture.readFile('/set-html/index.html');
+ const $ = cheerio.load(html);
+
+ expect($('#text')).to.have.lengthOf(1);
+ expect($('#text').text()).to.equal('a');
+
+ expect($('#zero')).to.have.lengthOf(1);
+ expect($('#zero').text()).to.equal('0');
+
+ expect($('#number')).to.have.lengthOf(1);
+ expect($('#number').text()).to.equal('1');
+
+ expect($('#undefined')).to.have.lengthOf(1);
+ expect($('#undefined').text()).to.equal('');
+
+ expect($('#null')).to.have.lengthOf(1);
+ expect($('#null').text()).to.equal('');
+
+ expect($('#false')).to.have.lengthOf(1);
+ expect($('#false').text()).to.equal('');
+
+ expect($('#true')).to.have.lengthOf(1);
+ expect($('#true').text()).to.equal('true');
+ });
 });
diff --git a/packages/astro/test/fixtures/astro-directives/src/pages/set-html.astro b/packages/astro/test/fixtures/astro-directives/src/pages/set-html.astro
@@ -0,0 +1,12 @@
+<html>
+ <head></head>
+ <body>
+ <div id="text" set:html={"a"} />
+ <div id="zero" set:html={0} />
+ <div id="number" set:html={1} />
+ <div id="false" set:html={false} />
+ <div id="true" set:html={true} />
+ <div id="undefined" set:html={undefined} />
+ <div id="null" set:html={null} />
+ </body>
+</html>