fix(ojs): Syntax errors not reported

Signed-off-by: Gordon Smith <[email protected]>

packages/observable-shim/package.json

@@ -43,4 +43,4 @@
         "url": "https://github.com/hpcc-systems/Visualization/issues"
     },
     "homepage": "https://github.com/hpcc-systems/Visualization"
-}
+}

packages/observable-shim/rollup.config.js

@@ -1,3 +1,4 @@
+import alias from "@rollup/plugin-alias";
 import sourcemaps from "rollup-plugin-sourcemaps";
 import nodeResolve from "@rollup/plugin-node-resolve";
 import postcss from "rollup-plugin-postcss";
@@ -6,6 +7,10 @@ import { globals } from "@hpcc-js/bundle";
 import pkg from "./package.json";
 
 const plugins = [
+    alias({
+        entries: [
+        ]
+    }),
     nodeResolve({
         preferBuiltins: true,
     }),

packages/observable-shim/src/index.ts

@@ -1,6 +1,6 @@
 export type { acorn } from "@observablehq/parser";
 export type { ohq } from "./types";
 
-export { walk, parseCell } from "@observablehq/parser";
+export { walk } from "@observablehq/parser";
 export { ancestor } from "acorn-walk";
-export { parseModule } from "./parse";
+export * from "./parse";

packages/observable-shim/src/parse.ts

@@ -1,7 +1,67 @@
-import { tokTypes as tt } from "acorn";
-import { CellParser } from "@observablehq/parser";
+// Compare with ../../../node_modules/@observablehq/parser/src/parse.js
+import { getLineInfo, tokTypes as tt } from "acorn";
+import { parseCell as ohqParseCell, CellParser } from "@observablehq/parser";
+import defaultGlobals from "../../../node_modules/@observablehq/parser/src/globals.js";
+import findReferences from "../../../node_modules/@observablehq/parser/src/references.js";
+import findFeatures from "../../../node_modules/@observablehq/parser/src/features.js";
+
+// Find references.
+// Check for illegal references to arguments.
+// Check for illegal assignments to global references.
+function parseReferences(cell, input, globals = defaultGlobals) {
+    if (!cell.body) {
+        cell.references = [];
+    } else if (cell.body.type === "ImportDeclaration") {
+        //  This is correct?!?
+        cell.references = cell.body.specifiers
+            ? cell.body.specifiers.map(i => i.imported)
+            : [];
+    } else {
+        try {
+            cell.references = findReferences(cell, globals);
+        } catch (error) {
+            if (error.node) {
+                const loc = getLineInfo(input, error.node.start);
+                error.message += ` (${loc.line}:${loc.column})`;
+                error.pos = error.node.start;
+                error.loc = loc;
+                delete error.node;
+            }
+            throw error;
+        }
+    }
+    return cell;
+}
+
+// Find features: file attachments, secrets, database clients.
+// Check for illegal references to arguments.
+// Check for illegal assignments to global references.
+function parseFeatures(cell, input) {
+    if (cell.body && cell.body.type !== "ImportDeclaration") {
+        try {
+            cell.fileAttachments = findFeatures(cell, "FileAttachment");
+            cell.databaseClients = findFeatures(cell, "DatabaseClient");
+            cell.secrets = findFeatures(cell, "Secret");
+        } catch (error) {
+            if (error.node) {
+                const loc = getLineInfo(input, error.node.start);
+                error.message += ` (${loc.line}:${loc.column})`;
+                error.pos = error.node.start;
+                error.loc = loc;
+                delete error.node;
+            }
+            throw error;
+        }
+    } else {
+        cell.fileAttachments = new Map();
+        cell.databaseClients = new Map();
+        cell.secrets = new Map();
+    }
+    return cell;
+}
 
 class ModuleParser extends CellParser {
+
     parseTopLevel(node) {
         if (!node.cells) node.cells = [];
         // @ts-ignore
@@ -20,7 +80,22 @@ class ModuleParser extends CellParser {
 }
 
 // @ts-ignore
-export function parseModule(input, { globals } = {}): string[] {
+export function parseModule(input, { globals } = {}) {
+    // @ts-ignore
+    const program = ModuleParser.parse(input, { ecmaVersion: 2020 });
+    for (const cell of program.cells) {
+        parseReferences(cell, input, globals);
+        parseFeatures(cell, input);
+    }
+    return program;
+}
+
+export function splitModule(input): string[] {
     // @ts-ignore
-    return ModuleParser.parse(input).cells.map(cell => input.substring(cell.start, cell.end));
+    return ModuleParser.parse(input, { ecmaVersion: "latest" }).cells.map(cell => input.substring(cell.start, cell.end));
+}
+
+export function parseCell(input: string) {
+    return ohqParseCell(input);
 }
+

packages/observablehq-compiler/src/__tests__/node.ts

@@ -98,8 +98,8 @@ describe("ojs", function () {
 
     await main.value("tenTimes");
 
-    for (const cell of define.cells) {
-      cell.dispose();
+    for (const cellID in define.cells) {
+      define.disposeCell(cellID);
       break;
     }
   });

packages/observablehq-compiler/src/compiler.ts

@@ -1,4 +1,4 @@
-import type { ohq } from "@hpcc-js/observable-shim";
+import { ohq, splitModule } from "@hpcc-js/observable-shim";
 
 import { endsWith, join } from "@hpcc-js/util";
 import { parseCell, ParsedImportCell } from "./cst";
@@ -178,31 +178,36 @@ async function createCell(node: ohq.Node, baseUrl: string) {
     const variables: VariableFunc[] = [];
     try {
         const text = node.mode && node.mode !== "js" ? `${node.mode}\`${encodeBacktick(node.value)}\`` : node.value;
-        const parsed = parseCell(text);
-        switch (parsed.type) {
-            case "import":
-                modules.push(await createModule(parsed, text, baseUrl));
-                break;
-            case "viewof":
-                variables.push(createVariable(true, parsed.variable.id, parsed.variable.inputs, parsed.variable.func));
-                variables.push(createVariable(false, parsed.variableValue.id, parsed.variableValue.inputs, parsed.variableValue.func, true));
-                break;
-            case "mutable":
-                variables.push(createVariable(false, parsed.initial.id, parsed.initial.inputs, parsed.initial.func));
-                variables.push(createVariable(false, parsed.variable.id, parsed.variable.inputs, parsed.variable.func));
-                variables.push(createVariable(true, parsed.variableValue.id, parsed.variableValue.inputs, parsed.variableValue.func, true));
-                break;
-            case "variable":
-                variables.push(createVariable(true, parsed.id, parsed.inputs, parsed.func));
-                break;
+        const parsedModule = splitModule(text);
+        for (const text of parsedModule) {
+            const parsed = parseCell(text);
+            switch (parsed.type) {
+                case "import":
+                    modules.push(await createModule(parsed, text, baseUrl));
+                    break;
+                case "viewof":
+                    variables.push(createVariable(true, parsed.variable.id, parsed.variable.inputs, parsed.variable.func));
+                    variables.push(createVariable(false, parsed.variableValue.id, parsed.variableValue.inputs, parsed.variableValue.func, true));
+                    break;
+                case "mutable":
+                    variables.push(createVariable(false, parsed.initial.id, parsed.initial.inputs, parsed.initial.func));
+                    variables.push(createVariable(false, parsed.variable.id, parsed.variable.inputs, parsed.variable.func));
+                    variables.push(createVariable(true, parsed.variableValue.id, parsed.variableValue.inputs, parsed.variableValue.func, true));
+                    break;
+                case "variable":
+                    variables.push(createVariable(true, parsed.id, parsed.inputs, parsed.func));
+                    break;
+            }
         }
     } catch (e) {
+        variables.push(createVariable(true, undefined, [], e.message));
     }
 
     const retVal = (runtime: ohq.Runtime, main: ohq.Module, inspector?: ohq.InspectorFactory) => {
         modules.forEach(imp => imp(runtime, main, inspector));
         variables.forEach(v => v(main, inspector));
     };
+    retVal.id = "" + node.id;
     retVal.modules = modules;
     retVal.variables = variables;
     retVal.dispose = () => {
@@ -227,7 +232,8 @@ export async function compile(notebook: ohq.Notebook, baseUrl: string = ".") {
 
     const files = notebook.files.map(f => createFile(f, baseUrl));
     const fileAttachments = new Map<string, any>(files);
-    let cells: CellFunc[] = await Promise.all(notebook.nodes.map(n => createCell(n, baseUrl)));
+    const _cells: CellFunc[] = await Promise.all(notebook.nodes.map(n => createCell(n, baseUrl)));
+    const cells = new Map<string, CellFunc>(_cells.map(c => [c.id, c]));
 
     const retVal = (runtime: ohq.Runtime, inspector?: ohq.InspectorFactory): ohq.Module => {
         const main = runtime.module();
@@ -241,18 +247,22 @@ export async function compile(notebook: ohq.Notebook, baseUrl: string = ".") {
     };
     retVal.fileAttachments = fileAttachments;
     retVal.cells = cells;
-    retVal.appendCell = async (n: ohq.Node, baseUrl: string = ".") => {
+    retVal.appendCell = async (n: ohq.Node, baseUrl) => {
         const cell = await createCell(n, baseUrl);
-        cells.push(cell);
+        retVal.disposeCell(cell.id);
+        cells.set(cell.id, cell);
         return cell;
     };
-    retVal.disposeCell = async (cell: CellFunc) => {
-        cells = cells.filter(c => c !== cell);
-        cell.dispose();
+    retVal.disposeCell = async (id: string) => {
+        const cell = cells.get(id);
+        if (cell) {
+            cells.delete(id);
+            cell.dispose();
+        }
     };
     retVal.dispose = () => {
         cells.forEach(cell => cell.dispose());
-        cells = [];
+        cells.clear();
     };
     retVal.write = (w: Writer) => {
         w.files(notebook.files);

packages/observablehq-compiler/src/index.ts

@@ -1,7 +1,6 @@
 export type { ohq } from "@hpcc-js/observable-shim";
 
 export * from "./compiler";
-export { ojs2notebook, omd2notebook, download } from "./util";
-export * from "./writer";
+export { ojs2notebook, omd2notebook, parseOmd, download } from "./util";
 
 import "../src/index.css";

packages/observablehq-compiler/src/util.ts

@@ -1,5 +1,5 @@
 import type { ohq } from "@hpcc-js/observable-shim";
-import { parseModule } from "@hpcc-js/observable-shim";
+import { parseCell, splitModule } from "@hpcc-js/observable-shim";
 
 const FuncTypes = {
     functionType: Object.getPrototypeOf(function () { }).constructor,
@@ -50,6 +50,8 @@ interface ParsedOJS {
     ojs: string;
     offset: number;
     inlineMD: boolean;
+    cell: any;
+    error: any;
 }
 
 export function encodeBacktick(str: string) {
@@ -59,14 +61,23 @@ export function encodeBacktick(str: string) {
 }
 
 function createParsedOJS(ojs: string, offset: number, inlineMD: boolean): ParsedOJS {
+    let cell;
+    let error;
+    try {
+        cell = parseCell(ojs);
+    } catch (e) {
+        error = e;
+    }
     return {
         ojs,
         offset,
-        inlineMD
+        inlineMD,
+        cell,
+        error
     };
 }
 
-function parseOmd(_: string): ParsedOJS[] {
+export function parseOmd(_: string): ParsedOJS[] {
     const retVal: ParsedOJS[] = [];
     //  Load Markdown  ---
     const re = /(```(?:\s|\S)[\s\S]*?```)/g;
@@ -101,7 +112,7 @@ export function notebook2ojs(_: string): ParsedOJS[] {
 }
 
 export function ojs2notebook(ojs: string): ohq.Notebook {
-    const cells = parseModule(ojs);
+    const cells = splitModule(ojs);
     return {
         files: [],
         nodes: cells.map((cell, idx) => {