From 34b8750490f8ff32de0dcbeadae551a4a9ee0d14 Mon Sep 17 00:00:00 2001 From: Alex Date: Thu, 19 Oct 2023 12:07:00 -0400 Subject: [PATCH] Improved the validator error messaging when validating schemas Using schemaPath instead of dataPath, because sometimes dataPath is not set and using instancePath which isn't even a valid property in Ajv. This seems like an issue with Ajv to me at this stage. Anyways, this works for now too with the time investment allowed. Making sure to raise the handleError via the useEffect handle so that the listener can trigger a state change on another component freely, without React throwing a warning in console. --- src/app.tsx | 44 ++------------- src/chart-validator.ts | 28 +++++++++- src/chart.tsx | 124 ++++++++++++++++++++++++++++------------- 3 files changed, 116 insertions(+), 80 deletions(-) diff --git a/src/app.tsx b/src/app.tsx index 97f7e12..28e9861 100644 --- a/src/app.tsx +++ b/src/app.tsx @@ -1,5 +1,5 @@ import { Chart } from './chart'; -import { ValidatorResult } from './chart-validator'; +import { ChartValidator, ValidatorResult } from './chart-validator'; /** * Create a container to visualize a GeoChart in a standalone manner. @@ -38,37 +38,12 @@ export function App(): JSX.Element { */ const handleError = (dataErrors: ValidatorResult, optionsErrors: ValidatorResult) => { // Gather all error messages - let msgData = ''; - dataErrors.errors?.forEach((m: string) => { - msgData += `${m}\n`; - }); - - // Gather all error messages - let msgOptions = ''; - optionsErrors.errors?.forEach((m: string) => { - msgOptions += `${m}\n`; - }); + const msgAll = ChartValidator.parseValidatorResultsMessages([dataErrors, optionsErrors]); // Show the error (actually, can't because the snackbar is linked to a map at the moment and geochart is standalone) // TODO: Decide if we want the snackbar outside of a map or not and use showError or not - cgpv.api.utilities.showError('', msgData); - cgpv.api.utilities.showError('', msgOptions); - console.error(dataErrors.errors, optionsErrors.errors); - alert('There was an error parsing the Chart inputs. View console for details.'); - }; - - /** - * Handles when the Chart X Axis has changed. - */ - const handleChartXAxisChanged = () => { - console.log('Handle Chart X Axis'); - }; - - /** - * Handles when the Chart Y Axis has changed. - */ - const handleChartYAxisChanged = () => { - console.log('Handle Chart Y Axis'); + cgpv.api.utilities.showError('', msgAll); + alert(`There was an error parsing the Chart inputs.\n\n${msgAll}\n\nView console for details.`); }; // Effect hook to add and remove event listeners @@ -80,16 +55,7 @@ export function App(): JSX.Element { }); // Render the Chart - return ( - - ); + return ; } export default App; diff --git a/src/chart-validator.ts b/src/chart-validator.ts index 45247e1..a548b12 100644 --- a/src/chart-validator.ts +++ b/src/chart-validator.ts @@ -4,6 +4,7 @@ import Ajv from 'ajv'; * Represents the result of a Chart data or options inputs validations. */ export type ValidatorResult = { + param: string; valid: boolean; errors?: string[]; }; @@ -145,9 +146,11 @@ export class ChartValidator { // Validate const valid = validate(data) as boolean; return { + param: 'data', valid, errors: validate.errors?.map((e: Ajv.ErrorObject) => { - return e.message || 'generic schema error'; + const m = e.message || 'generic schema error'; + return `${e.schemaPath} | ${e.keyword} | ${m}`; }), }; }; @@ -162,10 +165,31 @@ export class ChartValidator { // Validate const valid = validate(options) as boolean; return { + param: 'options', valid, errors: validate.errors?.map((e: Ajv.ErrorObject) => { - return e.message || 'generic schema error'; + const m = e.message || 'generic schema error'; + return `${e.schemaPath} | ${e.keyword} | ${m}`; }), }; }; + + public static parseValidatorResultsMessages(valRes: ValidatorResult[]) { + // Gather all error messages for data input + let msg = ''; + valRes.forEach((v) => { + // Redirect + msg += ChartValidator.parseValidatorResultMessage(v); + }); + return msg.replace(/^\n+|\n+$/gm, ''); + } + + public static parseValidatorResultMessage(valRes: ValidatorResult) { + // Gather all error messages for data input + let msg = ''; + valRes.errors?.forEach((m: string) => { + msg += `${m}\n`; + }); + return msg.replace(/^\n+|\n+$/gm, ''); + } } diff --git a/src/chart.tsx b/src/chart.tsx index 4cf82c5..f5047f4 100644 --- a/src/chart.tsx +++ b/src/chart.tsx @@ -48,61 +48,45 @@ export function Chart(props: TypeChartChartProps): JSX.Element { // eslint-disable-next-line @typescript-eslint/no-explicit-any const w = window as any; const { cgpv } = w; - // eslint-disable-next-line @typescript-eslint/no-unused-vars const { useEffect, useState, useRef, CSSProperties } = cgpv.react; const { Grid, Checkbox, Slider, Typography } = cgpv.ui.elements; - const { style: elStyle, data, options: elOptions, action: elAction } = props; + const { + style: elStyle, + data, + options: elOptions, + action: elAction, + defaultColors, + handleSliderXChanged, + handleSliderYChanged, + handleError, + } = props; // Cast the style const style = elStyle as typeof CSSProperties; // Attribute the ChartJS default colors - if (props.defaultColors?.backgroundColor) ChartJS.defaults.backgroundColor = props.defaultColors?.backgroundColor; - if (props.defaultColors?.borderColor) ChartJS.defaults.borderColor = props.defaultColors?.borderColor; - if (props.defaultColors?.color) ChartJS.defaults.color = props.defaultColors?.color; + if (defaultColors?.backgroundColor) ChartJS.defaults.backgroundColor = defaultColors?.backgroundColor; + if (defaultColors?.borderColor) ChartJS.defaults.borderColor = defaultColors?.borderColor; + if (defaultColors?.color) ChartJS.defaults.color = defaultColors?.color; // Merge default options const options: GeoChartOptions = { ...Chart.defaultProps.options, ...elOptions } as GeoChartOptions; - - // If options and data are specified - if (options && data) { - // Validate the data and options as received - const validator = new ChartValidator(); - const resOptions: ValidatorResult = validator.validateOptions(options); - const resData: ValidatorResult = validator.validateData(data); - - // If any errors - if (!resOptions.valid || !resData.valid) { - // If a callback is defined - if (props.handleError) props.handleError(resData, resOptions); - else console.error(resData, resOptions); - } + if (!options?.geochart?.chart) { + // Deep assign, in case geochart was specified in elOptions, but geochart wasn't (losing the default for 'chart' by accident) + options.geochart.chart = Chart.defaultProps.options.geochart.chart as GeoChartType; } // STATE / REF SECTION ******* const [redraw, setRedraw] = useState(elAction?.shouldRedraw); const chartRef = useRef(null); - // const [selectedDatasets, setSelectedDatasets] = useState(); - - // If redraw is true, reset the property, set the redraw property to true for the chart, then prep a timer to reset it to false after the redraw has happened. - // A bit funky, but as documented online. - if (elAction?.shouldRedraw) { - elAction!.shouldRedraw = false; - setRedraw(true); - setTimeout(() => { - setRedraw(false); - }, 200); - } /** * Handles when the X Slider changes * @param value number | number[] Indicates the slider value */ const handleSliderXChange = (value: number | number[]) => { - // If callback set - if (props.handleSliderXChanged) { - props.handleSliderXChanged!(value); - } + // Callback + handleSliderXChanged?.(value); }; /** @@ -110,10 +94,8 @@ export function Chart(props: TypeChartChartProps): JSX.Element { * @param value number | number[] Indicates the slider value */ const handleSliderYChange = (value: number | number[]) => { - // If callback set - if (props.handleSliderYChanged) { - props.handleSliderYChanged!(value); - } + // Callback + handleSliderYChanged?.(value); }; /** @@ -251,13 +233,73 @@ export function Chart(props: TypeChartChartProps): JSX.Element { return
; }; - return renderChartContainer(); + /** + * Renders the whole Chart container JSX.Element or an empty div + * @returns The whole Chart container JSX.Element or an empty div + */ + const renderChartContainerFailed = (): JSX.Element => { + return
Error rendering the Chart. Check console for details.
; + }; + + // + // PROCEED WITH LOGIC HERE! + // + + // If options and data are specified + let resOptions: ValidatorResult | undefined; + let resData: ValidatorResult | undefined; + if (options && data) { + // Validate the data and options as received + const validator = new ChartValidator(); + resOptions = validator.validateOptions(options) || undefined; + resData = validator.validateData(data); + } + + // Effect hook to raise the error on the correct React state. + // This is because it's quite probable the handling of the error will want to modify the state of another + // component (e.g. Snackbar) and React will throw a warning if this is not done in the useEffect(). + useEffect(() => { + // If the options or data schemas were checked and had errors + if (resData && resOptions && (!resData.valid || !resOptions.valid)) { + // If a callback is defined + handleError?.(resData, resOptions); + console.error(resData, resOptions); + } + }, [handleError, resData, resOptions]); + + // If options and data are specified + if (options && data) { + // If no errors + if (resOptions?.valid && resData?.valid) { + // If redraw is true, reset the property, set the redraw property to true for the chart, then prep a timer to reset it to false after the redraw has happened. + // A bit funky, but as documented online. + if (elAction?.shouldRedraw) { + elAction!.shouldRedraw = false; + setRedraw(true); + setTimeout(() => { + setRedraw(false); + }, 200); + } + + // Render the chart + return renderChartContainer(); + } + + // Failed to render + return renderChartContainerFailed(); + } + + // Nothing to render, no errors either + return
; } /** * React's default properties for the GeoChart */ Chart.defaultProps = { + style: null, + defaultColors: null, + data: null, options: { responsive: true, plugins: { @@ -269,4 +311,8 @@ Chart.defaultProps = { chart: 'line', }, }, + action: null, + handleSliderXChanged: null, + handleSliderYChanged: null, + handleError: null, };