From 969c884e31502615de0d1a88d11a62960d73f6ae Mon Sep 17 00:00:00 2001
From: halfnelson <dpershouse@gmail.com>
Date: Sun, 3 Feb 2019 10:51:19 +1000
Subject: [PATCH] repl improvements - Dont bind props until after component is
 created - Remove debug logs - destroy repl proxy event listener on compont
 destory - all repl-proxy commands now return a value

---
 .../repl/_components/Output/Viewer.html       | 22 +++--
 site/src/routes/repl/_utils/replProxy.js      | 15 +--
 site/static/repl-runner.js                    | 96 +++++++++++--------
 3 files changed, 81 insertions(+), 52 deletions(-)
diff --git a/site/src/routes/repl/_components/Output/Viewer.html b/site/src/routes/repl/_components/Output/Viewer.html
index e62e0d695b..7fc6856d3e 100644
--- a/site/src/routes/repl/_components/Output/Viewer.html
+++ b/site/src/routes/repl/_components/Output/Viewer.html
@@ -1,5 +1,5 @@
 <script>
-	import { onMount, createEventDispatcher } from 'svelte';
+	import { onMount, onDestroy, createEventDispatcher } from 'svelte';
 	import getLocationFromStack from '../../_utils/getLocationFromStack.js';
 	import ReplProxy from '../../_utils/replProxy.js';
 	import { decode } from 'sourcemap-codec';
@@ -67,9 +67,14 @@
 
 	let createComponent;
 	let init;
-	
+	onDestroy(() => {
+		if (replProxy) {
+			replProxy.destroy();
+		}
+	});
 	onMount(() => {
 		replProxy = new ReplProxy(refs.child);
+	
 		refs.child.addEventListener('load', () => {
 
 			replProxy.onPropUpdate = (prop, value) => {
@@ -163,6 +168,9 @@
 							props: ${JSON.stringify($values_store)}
 						});
 					`)
+					.then(()=> {
+						replProxy.bindProps(props);
+					})
 					.catch(e => {
 						// TODO show in UI
 						hasComponent = false;
@@ -196,6 +204,10 @@
 
 					// TODO do we need to clear out SSR HTML?
 					createComponent();
+					props_handler = props => {
+						replProxy.bindProps(props)
+					};
+					replProxy.bindProps(props);
 				};
 			}
 
@@ -209,9 +221,7 @@
 				init();
 			};
 
-			props_handler = props => {
-				replProxy.bindProps(props)
-			};
+			
 		});
 	});
 
@@ -279,7 +289,7 @@
 </style>
 
 <div class="iframe-container">
-	<iframe title="Result" bind:this={refs.child} sandbox="allow-scripts allow-popups" class="{error || pending || pendingImports ? 'greyed-out' : ''}" srcdoc='
+	<iframe title="Result" bind:this={refs.child} sandbox="allow-scripts allow-popups allow-forms allow-pointer-lock allow-top-navigation" class="{error || pending || pendingImports ? 'greyed-out' : ''}" srcdoc='
 		<!doctype html>
 		<html>
 			<head>
diff --git a/site/src/routes/repl/_utils/replProxy.js b/site/src/routes/repl/_utils/replProxy.js
index 922d6e3f18..4025dffe6b 100644
--- a/site/src/routes/repl/_utils/replProxy.js
+++ b/site/src/routes/repl/_utils/replProxy.js
@@ -5,20 +5,23 @@ export default class ReplProxy {
         this.pendingCmds = new Map();
         this.onPropUpdate = null;
         this.onFetchProgress = null;
-
-        window.addEventListener("message", ev => this.handleReplMessage(ev), false);
+        this.handle_event = (ev) => this.handleReplMessage(ev);
+        window.addEventListener("message", this.handle_event, false);
     }
 
+    destroy() {
+        window.removeEventListener("message", this.handle_event);
+    }
   
     iframeCommand(command, args) {
         return new Promise( (resolve, reject) => {
             this.cmdId = this.cmdId + 1; 
+            this.pendingCmds.set(this.cmdId, { resolve: resolve, reject: reject });
             this.iframe.contentWindow.postMessage({
                 action: command,
                 cmdId: this.cmdId,
                 args: args
             }, '*')
-            this.pendingCmds.set(this.cmdId, { resolve: resolve, reject: reject });
         });
     }
 
@@ -27,21 +30,21 @@ export default class ReplProxy {
         let id = cmdData.cmdId;
         let handler = this.pendingCmds.get(id);
         if (handler) {
+           
             this.pendingCmds.delete(id);
             if (action == "cmdError") {
                 let { message, stack } = cmdData;
                 let e = new Error(message);
                 e.stack = stack;
-                console.log("cmd fail");
+                console.log("repl cmd fail");
                 handler.reject(e)
             }
 
             if (action == "cmdOk") {
-                console.log("cmd okay");
                 handler.resolve(cmdData.args)
             }
         } else {
-            console.error("command not found", id);
+            console.error("command not found", id, cmdData, [...this.pendingCmds.keys()]);
         } 
     }
 
diff --git a/site/static/repl-runner.js b/site/static/repl-runner.js
index d1026839b5..0d87322b2a 100644
--- a/site/static/repl-runner.js
+++ b/site/static/repl-runner.js
@@ -39,15 +39,14 @@ function handleMessage(ev) {
     const sendOk = () => sendReply({ action: "cmdOk" });
     const sendError = (message, stack) => sendReply({ action: "cmdError", message, stack })
     
-    parent.postMessage({ action: "test" }, ev.origin);
-
+    
     if (action == "eval") {
         let { script } = ev.data.args;
         try {
             eval(script);
             sendOk();
         } catch (e) {
-            sendError(e.message, e.stack)
+            sendError(e.message, e.stack);
         }
     }
 
@@ -56,54 +55,71 @@ function handleMessage(ev) {
         
         if (!window.component) {
             // TODO can this happen?
-            console.error(`no component to bind to`);
+            console.warn('no component to bind to');
+            sendOk();
             return;
         }
 
-        props.forEach(prop => {
-            // TODO should there be a public API for binding?
-            // e.g. `component.$watch(prop, handler)`?
-            // (answer: probably)
-            window.component.$$.bound[prop] = value => {
-                sendMessage({ action:"prop_update", args: { prop, value } })
-            };
-        });
+        try {
+            props.forEach(prop => {
+                // TODO should there be a public API for binding?
+                // e.g. `component.$watch(prop, handler)`?
+                // (answer: probably)
+                window.component.$$.bound[prop] = value => {
+                    sendMessage({ action:"prop_update", args: { prop, value } })
+                };
+            });
+            sendOk();
+        } catch (e) {
+            
+            sendError(e.message, e.stack);
+        }
     }
 
     if (action == "set_prop") {
-        if (!window.component) {
-            return;
+        try {
+            if (!window.component) {
+                return;
+            }
+            let { prop, value } = ev.data.args;
+            component[prop] = value;
+            sendOk();
+        } catch (e) {
+            sendError(e.message, e.stack);
         }
-        let { prop, value } = ev.data.args;
-        component[prop] = value;
     }
     
     if (action == "catch_clicks") {
-        let topOrigin = ev.origin;
-        document.body.addEventListener('click', event => {
-            if (event.which !== 1) return;
-            if (event.metaKey || event.ctrlKey || event.shiftKey) return;
-            if (event.defaultPrevented) return;
-        
-            // ensure target is a link
-            let el = event.target;
-            while (el && el.nodeName !== 'A') el = el.parentNode;
-            if (!el || el.nodeName !== 'A') return;
-        
-            if (el.hasAttribute('download') || el.getAttribute('rel') === 'external' || el.target) return;
-        
-            event.preventDefault();
-        
-            if (el.href.startsWith(topOrigin)) {
-                const url = new URL(el.href);
-                if (url.hash[0] === '#') {
-                    window.location.hash = url.hash;
-                    return;
+        try {
+            let topOrigin = ev.origin;
+            document.body.addEventListener('click', event => {
+                if (event.which !== 1) return;
+                if (event.metaKey || event.ctrlKey || event.shiftKey) return;
+                if (event.defaultPrevented) return;
+            
+                // ensure target is a link
+                let el = event.target;
+                while (el && el.nodeName !== 'A') el = el.parentNode;
+                if (!el || el.nodeName !== 'A') return;
+            
+                if (el.hasAttribute('download') || el.getAttribute('rel') === 'external' || el.target) return;
+            
+                event.preventDefault();
+            
+                if (el.href.startsWith(topOrigin)) {
+                    const url = new URL(el.href);
+                    if (url.hash[0] === '#') {
+                        window.location.hash = url.hash;
+                        return;
+                    }
                 }
-            }
-        
-            window.open(el.href, '_blank');
-        });
+            
+                window.open(el.href, '_blank');
+            });
+            sendOk();
+        } catch(e) {
+            sendError(e.message, e.stack);
+        }
     }
 
 
