improve sketch error messages in the editor console (wip)

client/modules/IDE/components/Editor/consoleErrorDecoration.js

@@ -36,6 +36,9 @@ const ERROR_DECORATION = Decoration.line({
 
 // Add an error decoration to a specific line number
 export function addErrorDecoration(view, lineNumber) {
+  if (!view || !lineNumber) return;
+  const totalLines = view.state.doc.lines;
+  if (lineNumber < 1 || lineNumber > totalLines) return;
   const docLineNumber = view.state.doc.line(lineNumber);
   view.dispatch({
     effects: ADD_ERROR_DECORATION.of([
@@ -46,6 +49,7 @@ export function addErrorDecoration(view, lineNumber) {
 
 // Remove all error decorations
 export function removeErrorDecorations(view) {
+  if (!view) return;
   view.dispatch({
     effects: FILTER_ERROR_DECORATION.of(() => false)
   });

client/modules/IDE/components/Editor/index.jsx

@@ -148,27 +148,47 @@ function Editor({
   useEffect(() => {
     const consoleErrors = consoleEvents.filter((e) => e.method === 'error');
 
-    if (consoleErrors.length > 0) {
-      const firstError = consoleErrors[0];
-      const errorObj = { stack: firstError.data[0].toString() };
-      StackTrace.fromError(errorObj).then((stackLines) => {
-        expandConsole();
-        const line = stackLines.find(
-          (l) => l.fileName && l.fileName.startsWith('/')
+    removeErrorDecorations(codemirrorView.current);
+
+    if (consoleErrors.length === 0) return;
+
+    const resolveFrameFromError = (errorEntry) => {
+      const metaStack = errorEntry.meta && errorEntry.meta.stack;
+      if (Array.isArray(metaStack) && metaStack.length > 0) {
+        return Promise.resolve(
+          metaStack.find((f) => f.fileName && f.fileName.startsWith('/')) ||
+            metaStack[0]
         );
-        if (!line) return;
-        const fileNameArray = line.fileName.split('/');
-        const fileName = fileNameArray.slice(-1)[0];
-        const filePath = fileNameArray.slice(0, -1).join('/');
-        const fileWithError = files.find(
-          (f) => f.name === fileName && f.filePath === filePath
+      }
+      return StackTrace.fromError({
+        stack: errorEntry.data[0].toString()
+      }).then((stackLines) =>
+        stackLines.find((l) => l.fileName && l.fileName.startsWith('/'))
+      );
+    };
+
+    const matchFile = (frame) => {
+      if (!frame || !frame.fileName) return null;
+      const parts = frame.fileName.split('/');
+      const fileName = parts.slice(-1)[0];
+      const filePath = parts.slice(0, -1).join('/');
+      return files.find((f) => f.name === fileName && f.filePath === filePath);
+    };
+
+    Promise.all(consoleErrors.map(resolveFrameFromError)).then((frames) => {
+      const pairs = frames
+        .map((frame) => ({ frame, file: matchFile(frame) }))
+        .filter((p) => p.frame && p.file);
+      if (pairs.length === 0) return;
+      expandConsole();
+      const targetFileId = pairs[0].file.id;
+      setSelectedFile(targetFileId);
+      pairs
+        .filter((p) => p.file.id === targetFileId)
+        .forEach((p) =>
+          addErrorDecoration(codemirrorView.current, p.frame.lineNumber)
         );
-        setSelectedFile(fileWithError.id);
-        addErrorDecoration(codemirrorView.current, line.lineNumber);
-      });
-    } else {
-      removeErrorDecorations(codemirrorView.current);
-    }
+    });
   }, [consoleEvents]);
 
   const editorSectionClass = classNames({

client/modules/Preview/EmbedFrame.jsx

@@ -21,6 +21,8 @@ import resolvePathsForElementsWithAttribute from '../../../server/utils/resolveU
 
 let objectUrls = {};
 let objectPaths = {};
+let jshintErrors = [];
+let jshintErrorKeys = new Set();
 
 const Frame = styled.iframe`
   min-height: 100%;
@@ -56,23 +58,49 @@ function resolveCSSLinksInString(content, files) {
   return newContent;
 }
 
-function jsPreprocess(jsText) {
+const JSHINT_OPTIONS = {
+  esversion: 11,
+  asi: true,
+  laxbreak: true,
+  laxcomma: true
+};
+
+function jsPreprocess(jsText, fileName, lineOffset = 0) {
   let newContent = jsText;
   // check the code for js errors before sending it to strip comments
   // or loops.
-  JSHINT(newContent);
+  JSHINT(newContent, JSHINT_OPTIONS);
+
+  const fatal = JSHINT.errors.filter(
+    (err) => err && (!err.code || err.code.startsWith('E'))
+  );
 
-  if (JSHINT.errors.length === 0) {
+  if (fatal.length === 0) {
     newContent = decomment(newContent, {
       ignore: /\/\/\s*noprotect/g,
       space: true
     });
     newContent = loopProtect(newContent);
+    return newContent;
   }
-  return newContent;
+  fatal.forEach((err) => {
+    const line = (err.line || 1) + lineOffset;
+    const key = `${fileName}:${line}:${err.reason}`;
+    if (jshintErrorKeys.has(key)) return;
+    jshintErrorKeys.add(key);
+    jshintErrors.push({
+      file: fileName,
+      line,
+      character: err.character,
+      reason: err.reason,
+      evidence: err.evidence,
+      code: err.code
+    });
+  });
+  return `/* p5 sketch suppressed due to syntax errors in ${fileName}, see console */`;
 }
 
-function resolveJSLinksInString(content, files) {
+function resolveJSLinksInString(content, files, fileName, lineOffset = 0) {
   let newContent = content;
   let jsFileStrings = content.match(STRING_REGEX);
   jsFileStrings = jsFileStrings || [];
@@ -98,10 +126,17 @@ function resolveJSLinksInString(content, files) {
     }
   });
 
-  return jsPreprocess(newContent);
+  return jsPreprocess(newContent, fileName, lineOffset);
+}
+
+function getInlineScriptLineOffset(htmlContent, scriptInner) {
+  if (!htmlContent || !scriptInner) return 0;
+  const ix = htmlContent.indexOf(scriptInner);
+  if (ix < 0) return 0;
+  return htmlContent.substring(0, ix).split('\n').length - 1;
 }
 
-function resolveScripts(sketchDoc, files) {
+function resolveScripts(sketchDoc, files, htmlContent) {
   const scriptsInHTML = sketchDoc.getElementsByTagName('script');
   const scriptsInHTMLArray = Array.prototype.slice.call(scriptsInHTML);
   scriptsInHTMLArray.forEach((script) => {
@@ -136,7 +171,12 @@ function resolveScripts(sketchDoc, files) {
       ) !== null
     ) {
       script.setAttribute('crossorigin', '');
-      script.innerHTML = resolveJSLinksInString(script.innerHTML, files); // eslint-disable-line
+      const inlineOffset = getInlineScriptLineOffset(
+        htmlContent,
+        script.innerHTML
+      );
+      script.innerHTML = resolveJSLinksInString(script.innerHTML, files, 'index.html', inlineOffset); // eslint-disable-line
+      script.dataset.inlineOffset = String(inlineOffset);
     }
   });
 }
@@ -175,7 +215,11 @@ function resolveJSAndCSSLinks(files) {
   files.forEach((file) => {
     const newFile = { ...file };
     if (file.name.match(/.*\.js$/i)) {
-      newFile.content = resolveJSLinksInString(newFile.content, files);
+      newFile.content = resolveJSLinksInString(
+        newFile.content,
+        files,
+        file.name
+      );
     } else if (file.name.match(/.*\.css$/i)) {
       newFile.content = resolveCSSLinksInString(newFile.content, files);
     }
@@ -188,7 +232,8 @@ function addLoopProtect(sketchDoc) {
   const scriptsInHTML = sketchDoc.getElementsByTagName('script');
   const scriptsInHTMLArray = Array.prototype.slice.call(scriptsInHTML);
   scriptsInHTMLArray.forEach((script) => {
-    script.innerHTML = jsPreprocess(script.innerHTML); // eslint-disable-line
+    const inlineOffset = parseInt(script.dataset.inlineOffset || '0', 10);
+    script.innerHTML = jsPreprocess(script.innerHTML, 'index.html', inlineOffset); // eslint-disable-line
   });
 }
 
@@ -197,6 +242,8 @@ function injectLocalFiles(files, htmlFile, options) {
   let scriptOffs = [];
   objectUrls = {};
   objectPaths = {};
+  jshintErrors = [];
+  jshintErrorKeys = new Set();
   const resolvedFiles = resolveJSAndCSSLinks(files);
   const parser = new DOMParser();
   const sketchDoc = parser.parseFromString(htmlFile.content, 'text/html');
@@ -209,7 +256,7 @@ function injectLocalFiles(files, htmlFile, options) {
   resolvePathsForElementsWithAttribute('href', sketchDoc, resolvedFiles);
   // should also include background, data, poster, but these are used way less often
 
-  resolveScripts(sketchDoc, resolvedFiles);
+  resolveScripts(sketchDoc, resolvedFiles, htmlFile.content);
   resolveStyles(sketchDoc, resolvedFiles);
 
   if (textOutput || gridOutput) {
@@ -241,13 +288,14 @@ p5.prototype.registerMethod('afterSetup', p5.prototype.ensureAccessibleCanvas);`
   const sketchDocString = `<!DOCTYPE HTML>\n${sketchDoc.documentElement.outerHTML}`;
   scriptOffs = getAllScriptOffsets(sketchDocString);
   const consoleErrorsScript = sketchDoc.createElement('script');
+  addLoopProtect(sketchDoc);
   consoleErrorsScript.innerHTML = `
     window.offs = ${JSON.stringify(scriptOffs)};
     window.objectUrls = ${JSON.stringify(objectUrls)};
     window.objectPaths = ${JSON.stringify(objectPaths)};
+    window.__jshintErrors = ${JSON.stringify(jshintErrors)};
     window.editorOrigin = '${getConfig('EDITOR_URL')}';
   `;
-  addLoopProtect(sketchDoc);
   sketchDoc.head.prepend(consoleErrorsScript);
 
   return `<!DOCTYPE HTML>\n${sketchDoc.documentElement.outerHTML}`;