From 9d74f45ad653eddbffe41d8fa79de3079ea3c7c1 Mon Sep 17 00:00:00 2001 From: Yuvvan Talreja <31566676+yuvvantalreja@users.noreply.github.com> Date: Sun, 21 Sep 2025 18:15:31 -0400 Subject: [PATCH 1/4] fix: non-terminating error logs --- src/components/app.tsx | 185 +++++++++++------- .../input-panel/spec-editor/renderer.tsx | 2 + src/components/renderer/index.tsx | 21 +- src/components/renderer/renderer.tsx | 2 + src/utils/logger.ts | 64 ++++-- 5 files changed, 178 insertions(+), 96 deletions(-) diff --git a/src/components/app.tsx b/src/components/app.tsx index fcefd2ac9..a2e6f14a7 100644 --- a/src/components/app.tsx +++ b/src/components/app.tsx @@ -184,65 +184,15 @@ const App: React.FC = (props) => { [setExample, setEmptySpec, setGist], ); - useEffect(() => { - const handleMessage = (evt: MessageEvent) => { - const data = evt.data as MessageData; - if (!data.spec) { - return; - } - setState((s) => ({...s, baseUrl: evt.origin})); - console.info('[Vega-Editor] Received Message', evt.origin, data); - - if (data.config) { - setState((s) => ({...s, configEditorString: stringify(data.config)})); - } - - if (data.spec || data.file) { - // FIXME: remove any - (evt as any).source.postMessage(true, '*'); - } - if (data.spec) { - setState((s) => ({...s, editorString: data.spec})); - } - if (data.renderer) { - setState((s) => ({...s, renderer: data.renderer})); - } - }; - - window.addEventListener('message', handleMessage, false); - - if (params?.mode) { - const mode = params.mode.toLowerCase(); - if (mode === 'vega' || mode === 'vega-lite') { - setState((s) => ({...s, mode: mode as Mode})); - } - } - setSpecInUrl(params); + // Modular parsers + const parseVegaLiteSpec = useCallback( + (editorString: string, configEditorString: string, logLevel: number) => { + const currLogger = new LocalLogger(); + currLogger.level(logLevel); - return () => { - window.removeEventListener('message', handleMessage, false); - }; - }, [params, setState, setSpecInUrl]); - - // Parse Logic - useEffect(() => { - if (state.manualParse) { - if (!state.parse) { - return; - } - } else { - if (!state.editorString || state.editorString.trim() === '') { - return; - } - } - - const currLogger = new LocalLogger(); - currLogger.level(state.logLevel); - - try { - if (state.mode === Mode.VegaLite) { - const vegaLiteSpec: vegaLite.TopLevelSpec = parseJSONCOrThrow(state.editorString); - const config: Config = parseJSONCOrThrow(state.configEditorString); + try { + const vegaLiteSpec: vegaLite.TopLevelSpec = parseJSONCOrThrow(editorString); + const config: Config = parseJSONCOrThrow(configEditorString); const options = { config, @@ -264,7 +214,7 @@ const App: React.FC = (props) => { validateVegaLite(vegaLiteSpec, currLogger); const compileResult = - state.editorString !== '{}' ? vegaLite.compile(vegaLiteSpec, options) : {spec: {}, normalized: {}}; + editorString !== '{}' ? vegaLite.compile(vegaLiteSpec, options) : {spec: {}, normalized: {}}; const normalizedSpec = compileResult.normalized; setState((s) => ({ @@ -279,8 +229,28 @@ const App: React.FC = (props) => { debugs: currLogger.debugs, error: null, })); - } else { - const spec = parseJSONCOrThrow(state.editorString); + } catch (error: any) { + setState((s) => ({ + ...s, + error: {message: error.message}, + parse: false, + errors: currLogger.errors, + warns: currLogger.warns, + infos: currLogger.infos, + debugs: currLogger.debugs, + })); + } + }, + [setState], + ); + + const parseVegaSpec = useCallback( + (editorString: string, logLevel: number) => { + const currLogger = new LocalLogger(); + currLogger.level(logLevel); + + try { + const spec = parseJSONCOrThrow(editorString); if (spec.$schema) { try { const parsed = schemaParser(spec.$schema); @@ -305,19 +275,90 @@ const App: React.FC = (props) => { debugs: currLogger.debugs, error: null, })); + } catch (error: any) { + setState((s) => ({ + ...s, + error: {message: error.message}, + parse: false, + errors: currLogger.errors, + warns: currLogger.warns, + infos: currLogger.infos, + debugs: currLogger.debugs, + })); + } + }, + [setState], + ); + + useEffect(() => { + const handleMessage = (evt: MessageEvent) => { + const data = evt.data as MessageData; + if (!data.spec) { + return; + } + setState((s) => ({...s, baseUrl: evt.origin})); + console.info('[Vega-Editor] Received Message', evt.origin, data); + + if (data.config) { + setState((s) => ({...s, configEditorString: stringify(data.config)})); + } + + if (data.spec || data.file) { + // FIXME: remove any + (evt as any).source.postMessage(true, '*'); + } + if (data.spec) { + setState((s) => ({...s, editorString: data.spec})); } - } catch (error) { - setState((s) => ({ - ...s, - error: {message: error.message}, - parse: false, - errors: currLogger.errors, - warns: currLogger.warns, - infos: currLogger.infos, - debugs: currLogger.debugs, - })); + if (data.renderer) { + setState((s) => ({...s, renderer: data.renderer})); + } + }; + + window.addEventListener('message', handleMessage, false); + + if (params?.mode) { + const mode = params.mode.toLowerCase(); + if (mode === 'vega' || mode === 'vega-lite') { + setState((s) => ({...s, mode: mode as Mode})); + } + } + setSpecInUrl(params); + + return () => { + window.removeEventListener('message', handleMessage, false); + }; + }, [params, setState, setSpecInUrl]); + + // Parse Logic + useEffect(() => { + if (state.manualParse) { + if (!state.parse) { + return; + } + } else { + if (!state.editorString || state.editorString.trim() === '') { + return; + } + } + + if (state.mode === Mode.VegaLite) { + parseVegaLiteSpec(state.editorString, state.configEditorString, state.logLevel); + } else { + parseVegaSpec(state.editorString, state.logLevel); } - }, [state.editorString, state.mode, state.parse, state.config, state.logLevel, setState]); + }, [ + state.editorString, + state.mode, + state.parse, + state.config, + state.logLevel, + state.manualParse, + state.configEditorString, + setState, + parseVegaLiteSpec, + parseVegaSpec, + ]); useEffect(() => { if (state.mergeConfigSpec) { diff --git a/src/components/input-panel/spec-editor/renderer.tsx b/src/components/input-panel/spec-editor/renderer.tsx index 272888550..90aea1241 100644 --- a/src/components/input-panel/spec-editor/renderer.tsx +++ b/src/components/input-panel/spec-editor/renderer.tsx @@ -67,6 +67,8 @@ const EditorWithNavigation: React.FC<{ const debouncedUpdateSpec = useCallback(debounce(700, updateSpec), [updateSpec]); + // Decompressing URl + useEffect(() => { if (compressed) { let spec: string = LZString.decompressFromEncodedURIComponent(compressed); diff --git a/src/components/renderer/index.tsx b/src/components/renderer/index.tsx index a148b79ab..d627b4697 100644 --- a/src/components/renderer/index.tsx +++ b/src/components/renderer/index.tsx @@ -7,6 +7,18 @@ const RendererContainer: React.FC = () => { const {state, setState} = useAppContext(); const {setRuntime, recordPulse} = useDataflowActions(); + const resetLogs = React.useCallback( + () => + setState((s) => ({ + ...s, + errors: [], + warns: [], + infos: [], + debugs: [], + })), + [setState], + ); + const props = { baseURL: state.baseURL, config: state.config, @@ -23,14 +35,7 @@ const RendererContainer: React.FC = () => { backgroundColor: state.backgroundColor, expressionInterpreter: state.expressionInterpreter, setError: (error: {message: string} | null) => setState((s) => ({...s, error})), - resetLogs: () => - setState((s) => ({ - ...s, - errors: [], - warns: [], - infos: [], - debugs: [], - })), + resetLogs, }; return ( diff --git a/src/components/renderer/renderer.tsx b/src/components/renderer/renderer.tsx index 935667e70..13ae3d50f 100644 --- a/src/components/renderer/renderer.tsx +++ b/src/components/renderer/renderer.tsx @@ -143,6 +143,7 @@ export default function Renderer(props: RendererProps) { } }, [location.pathname, navigate]); + // fullscreen keyboard shortcuts useEffect(() => { const onKey = (e: KeyboardEvent) => { if (e.keyCode === KEYCODES.ESCAPE && size.fullscreen) { @@ -163,6 +164,7 @@ export default function Renderer(props: RendererProps) { return () => document.removeEventListener('keydown', onKey); }, [size.fullscreen, openPortal, closePortal]); + // FIX useEffect(() => { const params = location.pathname.split('/'); if (params[params.length - 1] === 'view') { diff --git a/src/utils/logger.ts b/src/utils/logger.ts index a4c9b1801..5ebb1975b 100644 --- a/src/utils/logger.ts +++ b/src/utils/logger.ts @@ -98,10 +98,18 @@ export class DispatchingLogger implements vega.LoggerInterface { if (this.level() >= vega.Warn) { console.warn(...args); - this.setState((state) => ({ - ...state, - warns: [...state.warns, args.join(' ')], - })); + const message = args.join(' '); + this.setState((state) => { + // Prevent duplicate consecutive warnings + const lastWarn = state.warns[state.warns.length - 1]; + if (lastWarn === message) { + return state; + } + return { + ...state, + warns: [...state.warns, message], + }; + }); } return this; @@ -118,10 +126,18 @@ export class DispatchingLogger implements vega.LoggerInterface { if (this.level() >= vega.Info) { console.info(...args); - this.setState((state) => ({ - ...state, - infos: [...state.infos, args.join(' ')], - })); + const message = args.join(' '); + this.setState((state) => { + // Prevent duplicate consecutive info messages + const lastInfo = state.infos[state.infos.length - 1]; + if (lastInfo === message) { + return state; + } + return { + ...state, + infos: [...state.infos, message], + }; + }); } return this; @@ -138,10 +154,18 @@ export class DispatchingLogger implements vega.LoggerInterface { if (this.level() >= vega.Debug) { console.debug(...args); - this.setState((state) => ({ - ...state, - debugs: [...state.debugs, args.join(' ')], - })); + const message = args.join(' '); + this.setState((state) => { + // Prevent duplicate consecutive debug messages + const lastDebug = state.debugs[state.debugs.length - 1]; + if (lastDebug === message) { + return state; + } + return { + ...state, + debugs: [...state.debugs, message], + }; + }); } return this; @@ -157,10 +181,18 @@ export class DispatchingLogger implements vega.LoggerInterface { // Always log errors regardless of level console.error(...args); - this.setState((state) => ({ - ...state, - errors: [...state.errors, args.join(' ')], - })); + const message = args.join(' '); + this.setState((state) => { + // Prevent duplicate consecutive errors to avoid infinite loops + const lastError = state.errors[state.errors.length - 1]; + if (lastError === message) { + return state; + } + return { + ...state, + errors: [...state.errors, message], + }; + }); return this; }; From 343f1e1ef1254c62b54485b29efef247c86b8d12 Mon Sep 17 00:00:00 2001 From: Yuvvan Talreja <31566676+yuvvantalreja@users.noreply.github.com> Date: Thu, 25 Sep 2025 15:31:35 -0400 Subject: [PATCH 2/4] fix: unnecessary parse in auto mode --- src/components/header/renderer.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/header/renderer.tsx b/src/components/header/renderer.tsx index 3ed6527e3..d835f07d1 100644 --- a/src/components/header/renderer.tsx +++ b/src/components/header/renderer.tsx @@ -533,7 +533,7 @@ const Header: React.FC = ({showExample}) => { className="header-button" id="run-button" onClick={() => { - setState((s) => ({...s, parse: true})); + if (manualParse) setState((s) => ({...s, parse: true})); }} > From 12d6e061fc5c713e57e9759571f5fe3339ec61e7 Mon Sep 17 00:00:00 2001 From: Yuvvan Talreja <31566676+yuvvantalreja@users.noreply.github.com> Date: Fri, 26 Sep 2025 13:10:27 -0400 Subject: [PATCH 3/4] fix: increase debounce to fix cursor lag --- src/components/input-panel/spec-editor/renderer.tsx | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/components/input-panel/spec-editor/renderer.tsx b/src/components/input-panel/spec-editor/renderer.tsx index 90aea1241..00dea48fd 100644 --- a/src/components/input-panel/spec-editor/renderer.tsx +++ b/src/components/input-panel/spec-editor/renderer.tsx @@ -56,16 +56,14 @@ const EditorWithNavigation: React.FC<{ if (parsedMode === Mode.Vega) { props.updateVegaSpec(spec, config); - } else if (parsedMode === Mode.VegaLite) { - props.updateVegaLiteSpec(spec, config); } else { - props.updateEditorString(spec); + props.updateVegaLiteSpec(spec, config); } }, - [mode, props.updateVegaSpec, props.updateVegaLiteSpec, props.updateEditorString], + [mode, props.updateVegaSpec, props.updateVegaLiteSpec], ); - const debouncedUpdateSpec = useCallback(debounce(700, updateSpec), [updateSpec]); + const debouncedUpdateSpec = useCallback(debounce(1200, updateSpec), [updateSpec]); // Decompressing URl From 123a592249d56a2cbc120f84c4709be79d1e3acd Mon Sep 17 00:00:00 2001 From: Yuvvan Talreja <31566676+yuvvantalreja@users.noreply.github.com> Date: Fri, 26 Sep 2025 14:25:51 -0400 Subject: [PATCH 4/4] refactor: added helper function for error duplication logic --- .../input-panel/spec-editor/renderer.tsx | 2 - src/components/renderer/renderer.tsx | 1 - src/utils/logger.ts | 59 +++++-------------- 3 files changed, 15 insertions(+), 47 deletions(-) diff --git a/src/components/input-panel/spec-editor/renderer.tsx b/src/components/input-panel/spec-editor/renderer.tsx index 00dea48fd..3f56710d7 100644 --- a/src/components/input-panel/spec-editor/renderer.tsx +++ b/src/components/input-panel/spec-editor/renderer.tsx @@ -65,8 +65,6 @@ const EditorWithNavigation: React.FC<{ const debouncedUpdateSpec = useCallback(debounce(1200, updateSpec), [updateSpec]); - // Decompressing URl - useEffect(() => { if (compressed) { let spec: string = LZString.decompressFromEncodedURIComponent(compressed); diff --git a/src/components/renderer/renderer.tsx b/src/components/renderer/renderer.tsx index 13ae3d50f..e864b7f2b 100644 --- a/src/components/renderer/renderer.tsx +++ b/src/components/renderer/renderer.tsx @@ -164,7 +164,6 @@ export default function Renderer(props: RendererProps) { return () => document.removeEventListener('keydown', onKey); }, [size.fullscreen, openPortal, closePortal]); - // FIX useEffect(() => { const params = location.pathname.split('/'); if (params[params.length - 1] === 'view') { diff --git a/src/utils/logger.ts b/src/utils/logger.ts index 5ebb1975b..a5475bc96 100644 --- a/src/utils/logger.ts +++ b/src/utils/logger.ts @@ -63,6 +63,17 @@ export class DispatchingLogger implements vega.LoggerInterface { this.setState = setState; } + private updateMessageArray(state: any, messageArrayKey: string, message: string): any { + const lastMessage = state[messageArrayKey][state[messageArrayKey].length - 1]; + if (lastMessage === message) { + return state; + } + return { + ...state, + [messageArrayKey]: [...state[messageArrayKey], message], + }; + } + public updateCurrentState(state: any) { this.currentState = state; } @@ -99,17 +110,7 @@ export class DispatchingLogger implements vega.LoggerInterface { console.warn(...args); const message = args.join(' '); - this.setState((state) => { - // Prevent duplicate consecutive warnings - const lastWarn = state.warns[state.warns.length - 1]; - if (lastWarn === message) { - return state; - } - return { - ...state, - warns: [...state.warns, message], - }; - }); + this.setState((state) => this.updateMessageArray(state, 'warns', message)); } return this; @@ -127,17 +128,7 @@ export class DispatchingLogger implements vega.LoggerInterface { console.info(...args); const message = args.join(' '); - this.setState((state) => { - // Prevent duplicate consecutive info messages - const lastInfo = state.infos[state.infos.length - 1]; - if (lastInfo === message) { - return state; - } - return { - ...state, - infos: [...state.infos, message], - }; - }); + this.setState((state) => this.updateMessageArray(state, 'infos', message)); } return this; @@ -155,17 +146,7 @@ export class DispatchingLogger implements vega.LoggerInterface { console.debug(...args); const message = args.join(' '); - this.setState((state) => { - // Prevent duplicate consecutive debug messages - const lastDebug = state.debugs[state.debugs.length - 1]; - if (lastDebug === message) { - return state; - } - return { - ...state, - debugs: [...state.debugs, message], - }; - }); + this.setState((state) => this.updateMessageArray(state, 'debugs', message)); } return this; @@ -182,17 +163,7 @@ export class DispatchingLogger implements vega.LoggerInterface { console.error(...args); const message = args.join(' '); - this.setState((state) => { - // Prevent duplicate consecutive errors to avoid infinite loops - const lastError = state.errors[state.errors.length - 1]; - if (lastError === message) { - return state; - } - return { - ...state, - errors: [...state.errors, message], - }; - }); + this.setState((state) => this.updateMessageArray(state, 'errors', message)); return this; };