Removes prefix from prerender annotation.

Refs #71. This prefix is no longer needed since it is not wrapped in a raw HTML comment, but instead wrapped in a real tag. Now "creating" and "parsing" an annotation is really just calling `JSON.stringify()` and `JSON.parse()`. They don't provide that much utility as abstractions, but ok for now.
dgp1130 · Mar 12, 2023 · e0f489d · e0f489d
1 parent a40f86c
commit e0f489d
Show file tree

Hide file tree

Showing 13 changed files with 68 additions and 92 deletions.
diff --git a/common/models/prerender_annotation.mts b/common/models/prerender_annotation.mts
@@ -1,37 +1,28 @@
-const prefix = 'bazel:rules_prerender:PRIVATE_DO_NOT_DEPEND_OR_ELSE';
-
 /**
- * Creates an annotation comment and returns it, should be rendered directly
- * into prerendered HTML for the build process to use.
+ * Serializes an annotation and returns it, should be rendered directly into
+ * prerendered HTML for the build process to use.
  * 
  * @param annotation The annotation to convert to a string.
  * @return The annotation expressed as a string where it can be easily parsed by
  *     tooling.
  */
-export function createAnnotation(annotation: PrerenderAnnotation): string {
-    return `${prefix} - ${JSON.stringify(annotation)}`;
+export function serialize(annotation: PrerenderAnnotation): string {
+    return JSON.stringify(annotation, null, 4);
 }
 
 /**
- * Parse an annotation from HTML comment text. The input must **not** contain
- * the leading `<!--` or trailing `-->` of an HTML comment. Returns `undefined`
- * if the comment does not appear to be an annotation.
+ * Parse an annotation from HTML text content. The input must **not** contain
+ * the leading or trailing `<rules_prerender:annotation>` tag of the HTML
+ * element.
  * 
- * @param comment The comment to parse an annotation from.
- * @returns The parsed annotation or `undefined` if the comment does not appear
- *     to be an annotation.
+ * @param content The text content to parse an annotation from.
+ * @returns The parsed annotation.
  * @throws If the parsed annotation does not contain valid JSON. This is
  *     indicative of an annotation creation error, so this is effectively an
  *     assertion error.
  */
-export function parseAnnotation(comment: string): PrerenderAnnotation|undefined {
-    if (!comment.trim().startsWith(prefix)) {
-        return undefined;
-    }
-    const separatorIndex = comment.indexOf('-');
-    if (separatorIndex === -1) return undefined;
-    const json = comment.substring(separatorIndex + 1).trim();
-    return JSON.parse(json) as PrerenderAnnotation;
+export function deserialize(content: string): PrerenderAnnotation {
+    return JSON.parse(content) as PrerenderAnnotation;
 }
 
 /**

diff --git a/common/models/prerender_annotation_test.mts b/common/models/prerender_annotation_test.mts
@@ -1,4 +1,4 @@
-import { PrerenderAnnotation, createAnnotation, parseAnnotation, ScriptAnnotation, annotationsEqual, StyleAnnotation } from './prerender_annotation.mjs';
+import { PrerenderAnnotation, serialize, deserialize, ScriptAnnotation, annotationsEqual, StyleAnnotation } from './prerender_annotation.mjs';
 
 describe('prerender_annotation', () => {
     describe('PrerenderAnnotation', () => {
@@ -13,50 +13,38 @@ describe('prerender_annotation', () => {
         });
     });
 
-    describe('createAnnotation()', () => {
+    describe('serialize()', () => {
         it('converts the given annotation to a string format', () => {
-            const annotation = createAnnotation({
+            const annotation = serialize({
                 type: 'script',
                 path: './foo/bar/baz.js',
             });
 
-            expect(annotation)
-                .toBe('bazel:rules_prerender:PRIVATE_DO_NOT_DEPEND_OR_ELSE - {"type":"script","path":"./foo/bar/baz.js"}');
+            expect(annotation).toBe(`
+{
+    "type": "script",
+    "path": "./foo/bar/baz.js"
+}
+            `.trim());
         });
     });
 
-    describe('parseAnnotation()', () => {
+    describe('deserialize()', () => {
         it('returns the parsed annotation', () => {
-            const annotation = parseAnnotation(
-                'bazel:rules_prerender:PRIVATE_DO_NOT_DEPEND_OR_ELSE - {"type":"script","path":"./foo/bar/baz.js"}');
+            const annotation = deserialize(`
+{
+    "type": "script",
+    "path": "./foo/bar/baz.js"
+}
+            `.trim());
             expect(annotation).toEqual({
                 type: 'script',
                 path: './foo/bar/baz.js',
             });
         });
 
-        it('returns parsed annotation when given extra spacing', () => {
-            // Comments often have leading and trailing whitespace which should
-            // be ignored.
-            const annotation = parseAnnotation('  bazel:rules_prerender:PRIVATE_DO_NOT_DEPEND_OR_ELSE - {"type":"script","path":"./foo/bar/baz.js"}   ');
-            expect(annotation).toEqual({
-                type: 'script',
-                path: './foo/bar/baz.js',
-            });
-        });
-
-        it('returns `undefined` when not given an annotation', () => {
-            const annotation1 = parseAnnotation('wrong prefix');
-            expect(annotation1).toBeUndefined();
-
-            const annotation2 = parseAnnotation('bazel:rules_prerender:PRIVATE_DO_NOT_DEPEND_OR_ELSE no following hyphen');
-            expect(annotation2).toBeUndefined();
-        });
-
         it('throws when not given valid JSON', () => {
-            expect(() => parseAnnotation(
-                'bazel:rules_prerender:PRIVATE_DO_NOT_DEPEND_OR_ELSE - not JSON'))
-                .toThrow();
+            expect(() => deserialize('not JSON')).toThrow();
         });
     });
 

diff --git a/common/prerender_annotation_walker.mts b/common/prerender_annotation_walker.mts
@@ -1,5 +1,5 @@
 import { HTMLElement } from 'node-html-parser';
-import { parseAnnotation, PrerenderAnnotation } from './models/prerender_annotation.mjs';
+import { deserialize, PrerenderAnnotation } from './models/prerender_annotation.mjs';
 
 /**
  * A reference to a `node-html-parser` `Node` which contains a
@@ -31,9 +31,7 @@ function* walkAnnotations(els: Generator<HTMLElement, void, void>):
         if (el.tagName?.toLowerCase() !== 'rules_prerender:annotation') continue;
 
         // Parse the annotation.
-        const annotation = parseAnnotation(el.textContent);
-        if (!annotation) throw new Error(`Failed to parse annotation:\n${el.outerHTML}`);
-
+        const annotation = deserialize(el.textContent);
         yield { annotation, el };
     }
 }

diff --git a/common/prerender_annotation_walker_test.mts b/common/prerender_annotation_walker_test.mts
@@ -1,13 +1,13 @@
 import { HTMLElement, parse } from 'node-html-parser';
-import { createAnnotation } from './models/prerender_annotation.mjs';
+import { serialize } from './models/prerender_annotation.mjs';
 import { walkAllAnnotations } from './prerender_annotation_walker.mjs';
 
 describe('prerender_annotation_walker', () => {
     describe('walkAllAnnotations()', () => {
         it('walks all annotations', () => {
-            const annotation1 = createAnnotation({ type: 'script', path: 'wksp/foo.js' });
-            const annotation2 = createAnnotation({ type: 'script', path: 'wksp/bar.js' });
-            const annotation3 = createAnnotation({ type: 'script', path: 'wksp/baz.js' });
+            const annotation1 = serialize({ type: 'script', path: 'wksp/foo.js' });
+            const annotation2 = serialize({ type: 'script', path: 'wksp/bar.js' });
+            const annotation3 = serialize({ type: 'script', path: 'wksp/baz.js' });
 
             const root = parse(`
 <!DOCTYPE html>
@@ -33,7 +33,7 @@ describe('prerender_annotation_walker', () => {
         });
 
         it('can remove nodes from the tree', () => {
-            const annotation = createAnnotation({ type: 'script', path: 'wksp/foo.js' });
+            const annotation = serialize({ type: 'script', path: 'wksp/foo.js' });
 
             const root = parse(`
 <!DOCTYPE html>
@@ -67,7 +67,7 @@ describe('prerender_annotation_walker', () => {
         });
 
         it('can replace nodes in the tree', () => {
-            const annotation = createAnnotation({ type: 'script', path: 'wksp/foo.js' });
+            const annotation = serialize({ type: 'script', path: 'wksp/foo.js' });
 
             const root = parse(`
 <!DOCTYPE html>
@@ -119,8 +119,7 @@ describe('prerender_annotation_walker', () => {
 </html>
             `.trim());
 
-            expect(() => Array.from(walkAllAnnotations(root)))
-                .toThrowError(/Failed to parse annotation/);
+            expect(() => Array.from(walkAllAnnotations(root))).toThrow();
         });
     });
 });
diff --git a/packages/declarative_shadow_dom/declarative_shadow_dom_test.mts b/packages/declarative_shadow_dom/declarative_shadow_dom_test.mts
@@ -1,4 +1,4 @@
-import { createAnnotation } from '../../common/models/prerender_annotation.mjs';
+import { serialize } from '../../common/models/prerender_annotation.mjs';
 import { polyfillDeclarativeShadowDom } from './declarative_shadow_dom.mjs';
 
 describe('declarative_shadow_dom', () => {
@@ -7,7 +7,7 @@ describe('declarative_shadow_dom', () => {
             expect(polyfillDeclarativeShadowDom())
                 .toBe(`
 <rules_prerender:annotation>${
-    createAnnotation({
+    serialize({
         type: 'script',
         path: 'packages/declarative_shadow_dom/declarative_shadow_dom_polyfill.mjs',
     })

diff --git a/packages/rules_prerender/scripts.mts b/packages/rules_prerender/scripts.mts
@@ -1,5 +1,5 @@
 import * as path from 'path';
-import { createAnnotation } from '../../common/models/prerender_annotation.mjs';
+import { serialize } from '../../common/models/prerender_annotation.mjs';
 import { wkspRelative } from './paths.mjs';
 
 /**
@@ -26,7 +26,7 @@ export function includeScript(filePath: string, meta: ImportMeta): string {
             filePath}" from "${wkspRelativePath}".`);
     }
 
-    const annotation = createAnnotation({
+    const annotation = serialize({
         type: 'script',
         path: resolved,
     });

diff --git a/packages/rules_prerender/scripts_test.mts b/packages/rules_prerender/scripts_test.mts
@@ -1,4 +1,4 @@
-import { createAnnotation } from '../../common/models/prerender_annotation.mjs';
+import { serialize } from '../../common/models/prerender_annotation.mjs';
 import { includeScript } from './scripts.mjs';
 
 describe('scripts', () => {
@@ -11,7 +11,7 @@ describe('scripts', () => {
 
             expect(annotation).toBe(`
 <rules_prerender:annotation>${
-    createAnnotation({
+    serialize({
         type: 'script',
         path: 'path/to/pkg/baz.mjs',
     })
@@ -27,7 +27,7 @@ describe('scripts', () => {
 
             expect(annotation).toBe(`
 <rules_prerender:annotation>${
-    createAnnotation({
+    serialize({
         type: 'script',
         path: 'path/to/pkg/some/subdir/foo.mjs',
     })
@@ -43,7 +43,7 @@ describe('scripts', () => {
 
             expect(annotation).toBe(`
 <rules_prerender:annotation>${
-    createAnnotation({
+    serialize({
         type: 'script',
         path: 'path/foo.mjs',
     })

diff --git a/packages/rules_prerender/styles.mts b/packages/rules_prerender/styles.mts
@@ -1,5 +1,5 @@
 import * as path from 'path';
-import { createAnnotation } from '../../common/models/prerender_annotation.mjs';
+import { serialize } from '../../common/models/prerender_annotation.mjs';
 import { getMap as getInlineStyleMap } from './inline_style_map.mjs';
 import { wkspRelative } from './paths.mjs';
 
@@ -41,7 +41,7 @@ export function inlineStyle(importPath: string, meta: ImportMeta): string {
     }
 
     // Return an annotation with the real file path.
-    const annotation = createAnnotation({
+    const annotation = serialize({
         type: 'style',
         path: filePath,
     });

diff --git a/packages/rules_prerender/styles_test.mts b/packages/rules_prerender/styles_test.mts
@@ -1,5 +1,5 @@
 import * as inlineStyleMap from './inline_style_map.mjs';
-import { createAnnotation } from '../../common/models/prerender_annotation.mjs';
+import { serialize } from '../../common/models/prerender_annotation.mjs';
 import { inlineStyle, InlineStyleNotFoundError } from './styles.mjs';
 
 describe('styles', () => {
@@ -20,7 +20,7 @@ describe('styles', () => {
 
             expect(annotation).toBe(`
 <rules_prerender:annotation>${
-    createAnnotation({
+    serialize({
         type: 'style',
         path: 'some/real/file.css',
     })
@@ -40,7 +40,7 @@ describe('styles', () => {
 
             expect(annotation).toBe(`
 <rules_prerender:annotation>${
-    createAnnotation({
+    serialize({
         type: 'style',
         path: 'some/real/file.css',
     })
@@ -60,7 +60,7 @@ describe('styles', () => {
 
             expect(annotation).toBe(`
 <rules_prerender:annotation>${
-    createAnnotation({
+    serialize({
         type: 'style',
         path: 'some/real/file.css',
     })
@@ -92,7 +92,7 @@ describe('styles', () => {
 
             expect(inlineStyle('./foo.css', meta)).toBe(`
 <rules_prerender:annotation>${
-    createAnnotation({
+    serialize({
         type: 'style',
         path: 'path/to/pkg/foo.css',
     })

diff --git a/tools/binaries/annotation_extractor/annotation_extractor_test.mts b/tools/binaries/annotation_extractor/annotation_extractor_test.mts
@@ -3,7 +3,7 @@ import { mockPrerenderMetadata, mockScriptMetadata } from '../../../common/model
 import { PrerenderMetadata } from '../../../common/models/prerender_metadata.mjs';
 import { execBinary, ProcessResult } from '../../../common/testing/binary.mjs';
 import { useTempDir } from '../../../common/testing/temp_dir.mjs';
-import { createAnnotation } from '../../../common/models/prerender_annotation.mjs';
+import { serialize } from '../../../common/models/prerender_annotation.mjs';
 
 const extractor = 'tools/binaries/annotation_extractor/annotation_extractor.sh';
 
@@ -23,8 +23,8 @@ describe('annotation_extractor', () => {
     const tmpDir = useTempDir();
 
     it('extracts annotations', async () => {
-        const annotation1 = createAnnotation({ type: 'script', path: 'foo.js' });
-        const annotation2 = createAnnotation({ type: 'script', path: 'bar.js' });
+        const annotation1 = serialize({ type: 'script', path: 'foo.js' });
+        const annotation2 = serialize({ type: 'script', path: 'bar.js' });
 
         await fs.mkdir(`${tmpDir.get()}/input_html`, { recursive: true });
         await fs.mkdir(`${tmpDir.get()}/output_html`, { recursive: true });
@@ -114,7 +114,7 @@ describe('annotation_extractor', () => {
     });
 
     it('deduplicates extracted annotations', async () => {
-        const annotation = createAnnotation({ type: 'script', path: 'foo.js' });
+        const annotation = serialize({ type: 'script', path: 'foo.js' });
 
         await fs.mkdir(`${tmpDir.get()}/input_html`, { recursive: true });
         await fs.mkdir(`${tmpDir.get()}/output_html`, { recursive: true });
@@ -170,7 +170,7 @@ describe('annotation_extractor', () => {
     });
 
     it('extracts annotations from a file in a nested directory', async () => {
-        const annotation = createAnnotation({ type: 'script', path: 'foo.js' });
+        const annotation = serialize({ type: 'script', path: 'foo.js' });
 
         await fs.mkdir(`${tmpDir.get()}/input_html`, { recursive: true });
         await fs.mkdir(`${tmpDir.get()}/output_html`, { recursive: true });
@@ -231,7 +231,7 @@ describe('annotation_extractor', () => {
     });
 
     it('passes through non-HTML files', async () => {
-        const annotation = createAnnotation({ type: 'script', path: 'foo.js' });
+        const annotation = serialize({ type: 'script', path: 'foo.js' });
 
         await fs.mkdir(`${tmpDir.get()}/input_html`, { recursive: true });
         await fs.mkdir(`${tmpDir.get()}/output_html`, { recursive: true });

diff --git a/tools/binaries/annotation_extractor/extractor_test.mts b/tools/binaries/annotation_extractor/extractor_test.mts
@@ -1,10 +1,10 @@
 import { extract } from './extractor.mjs';
-import { createAnnotation } from '../../../common/models/prerender_annotation.mjs';
+import { serialize } from '../../../common/models/prerender_annotation.mjs';
 
 describe('extractor', () => {
     describe('extract()', () => {
         it('extracts annotations from the given HTML contents', () => {
-            const annotation = createAnnotation({ type: 'script', path: 'wksp/foo.js' });
+            const annotation = serialize({ type: 'script', path: 'wksp/foo.js' });
             const [ extracted, annotations ] = extract(`
 <!DOCTYPE html>
 <html>
@@ -39,7 +39,7 @@ describe('extractor', () => {
         });
 
         it('extracts a first node annotation', () => {
-            const annotation = createAnnotation({ type: 'script', path: 'wksp/foo.js' });
+            const annotation = serialize({ type: 'script', path: 'wksp/foo.js' });
             const [ extracted, annotations ] = extract(`
 <rules_prerender:annotation>${annotation}</rules_prerender:annotation><!DOCTYPE html>
 <html>
@@ -68,7 +68,7 @@ describe('extractor', () => {
         });
 
         it('ignores unrelated comments', () => {
-            const annotation = createAnnotation({ type: 'script', path: 'wksp/foo.js' });
+            const annotation = serialize({ type: 'script', path: 'wksp/foo.js' });
             const [ extracted, annotations ] = extract(`
 <!-- Some leading comment. -->
 <!DOCTYPE html>
@@ -107,7 +107,7 @@ describe('extractor', () => {
         });
 
         it('ignores styles', () => {
-            const annotation = createAnnotation({ type: 'style', path: 'wksp/foo.css' });
+            const annotation = serialize({ type: 'style', path: 'wksp/foo.css' });
 
             const [ extracted, annotations ] = extract(`
 <!DOCTYPE html>