From 541a2e78d79ccd200a148f2135d5af591e0c6d0a Mon Sep 17 00:00:00 2001 From: Sebastian Malton Date: Tue, 17 May 2022 09:48:04 -0400 Subject: [PATCH] Fix async validation errors not being displayed - And fix validation errors sometimes being displayed multiple times Signed-off-by: Sebastian Malton --- src/common/utils/type-narrowing.ts | 8 +++ .../install-from-input/install-from-input.tsx | 13 ++--- .../components/+extensions/install.tsx | 31 ++++++----- src/renderer/components/input/input.tsx | 52 +++++++------------ .../components/input/input_validators.ts | 16 ++---- 5 files changed, 54 insertions(+), 66 deletions(-) diff --git a/src/common/utils/type-narrowing.ts b/src/common/utils/type-narrowing.ts index a138b2a0ec..eb5b6996cf 100644 --- a/src/common/utils/type-narrowing.ts +++ b/src/common/utils/type-narrowing.ts @@ -137,6 +137,14 @@ export function hasDefiniteField(field: Field): (val: return (val): val is T & { [f in Field]-?: NonNullable } => val[field] != null; } +export function isPromiseSettledRejected(result: PromiseSettledResult): result is PromiseRejectedResult { + return result.status === "rejected"; +} + +export function isPromiseSettledFulfilled(result: PromiseSettledResult): result is PromiseFulfilledResult { + return result.status === "fulfilled"; +} + export function isErrnoException(error: unknown): error is NodeJS.ErrnoException { return isObject(error) && hasOptionalTypedProperty(error, "code", isString) diff --git a/src/renderer/components/+extensions/install-from-input/install-from-input.tsx b/src/renderer/components/+extensions/install-from-input/install-from-input.tsx index 0b8912d8ec..eb5d6febe4 100644 --- a/src/renderer/components/+extensions/install-from-input/install-from-input.tsx +++ b/src/renderer/components/+extensions/install-from-input/install-from-input.tsx @@ -14,7 +14,6 @@ import { readFileNotify } from "../read-file-notify/read-file-notify"; import type { InstallRequest } from "../attempt-install/install-request"; import type { ExtensionInfo } from "../attempt-install-by-info.injectable"; import type { ExtensionInstallationStateStore } from "../../../../extensions/extension-installation-state-store/extension-installation-state-store"; -import { AsyncInputValidationError } from "../../input/input_validators"; export type InstallFromInput = (input: string) => Promise; @@ -51,16 +50,12 @@ export const installFromInput = ({ return await attemptInstall({ fileName, dataP: readFileNotify(input) }); } catch (error) { - if (error instanceof AsyncInputValidationError) { - const extNameCaptures = InputValidators.isExtensionNameInstallRegex.captures(input); + const extNameCaptures = InputValidators.isExtensionNameInstallRegex.captures(input); - if (extNameCaptures) { - const { name, version } = extNameCaptures; + if (extNameCaptures) { + const { name, version } = extNameCaptures; - return await attemptInstallByInfo({ name, version }); - } - } else { - throw error; + return await attemptInstallByInfo({ name, version }); } } diff --git a/src/renderer/components/+extensions/install.tsx b/src/renderer/components/+extensions/install.tsx index 52c0bd8906..816cfadce9 100644 --- a/src/renderer/components/+extensions/install.tsx +++ b/src/renderer/components/+extensions/install.tsx @@ -9,14 +9,12 @@ import { prevDefault } from "../../utils"; import { Button } from "../button"; import { Icon } from "../icon"; import { observer } from "mobx-react"; -import { Input, InputValidators } from "../input"; +import { asyncInputValidator, Input, InputValidators } from "../input"; import { SubTitle } from "../layout/sub-title"; import { TooltipPosition } from "../tooltip"; import type { ExtensionInstallationStateStore } from "../../../extensions/extension-installation-state-store/extension-installation-state-store"; -import extensionInstallationStateStoreInjectable - from "../../../extensions/extension-installation-state-store/extension-installation-state-store.injectable"; +import extensionInstallationStateStoreInjectable from "../../../extensions/extension-installation-state-store/extension-installation-state-store.injectable"; import { withInjectables } from "@ogre-tools/injectable-react"; -import { inputValidator } from "../input/input_validators"; export interface InstallProps { installPath: string; @@ -30,17 +28,22 @@ interface Dependencies { extensionInstallationStateStore: ExtensionInstallationStateStore; } -const installInputValidators = [ - InputValidators.isUrl, - InputValidators.isPath, - InputValidators.isExtensionNameInstall, -]; +const installInputValidator = asyncInputValidator({ + validate: async (value) => { + if ( + InputValidators.isUrl.validate(value) + || InputValidators.isExtensionNameInstall.validate(value) + ) { + return; + } -const installInputValidator = inputValidator({ - message: "Invalid URL, absolute path, or extension name", - validate: (value: string, props) => ( - installInputValidators.some(({ validate }) => validate(value, props)) - ), + try { + return await InputValidators.isPath.validate(value); + } catch { + throw new Error("Invalid URL, absolute path, or extension name"); + } + }, + debounce: InputValidators.isPath.debounce, }); const NonInjectedInstall: React.FC = ({ diff --git a/src/renderer/components/input/input.tsx b/src/renderer/components/input/input.tsx index 0c8014150f..9d206bd5aa 100644 --- a/src/renderer/components/input/input.tsx +++ b/src/renderer/components/input/input.tsx @@ -7,21 +7,19 @@ import "./input.scss"; import type { DOMAttributes, InputHTMLAttributes, TextareaHTMLAttributes } from "react"; import React from "react"; -import { autoBind, cssNames, debouncePromise, getRandId } from "../../utils"; +import { autoBind, cssNames, debouncePromise, getRandId, isPromiseSettledFulfilled } from "../../utils"; import { Icon } from "../icon"; import type { TooltipProps } from "../tooltip"; import { Tooltip } from "../tooltip"; import * as Validators from "./input_validators"; import type { InputValidator, InputValidation, InputValidationResult, SyncValidationMessageBuilder } from "./input_validators"; -import isFunction from "lodash/isFunction"; import uniqueId from "lodash/uniqueId"; import { debounce } from "lodash"; -const { conditionalValidators, AsyncInputValidationError, asyncInputValidator, inputValidator, ...InputValidators } = Validators; +const { conditionalValidators, asyncInputValidator, inputValidator, ...InputValidators } = Validators; export { InputValidators, - AsyncInputValidationError, asyncInputValidator, inputValidator, }; @@ -165,7 +163,7 @@ export class Input extends React.Component { async validate() { const value = this.getValue(); let validationId = (this.validationId = ""); // reset every time for async validators - const asyncValidators: Promise[] = []; + const asyncValidators: Promise[] = []; const errors: React.ReactNode[] = []; // run validators @@ -179,28 +177,13 @@ export class Input extends React.Component { if (!validationId) { this.validationId = validationId = uniqueId("validation_id_"); } - asyncValidators.push( - validator.validate(value, this.props).then( - () => null, // don't consider any valid result from promise since we interested in errors only - error => this.getValidatorError(value, validator) || error, - ), - ); - } - - const isValid = validator.validate(value, this.props); - - if (isValid === false) { - errors.push(this.getValidatorError(value, validator)); - } else if (isValid instanceof Promise) { - if (!validationId) { - this.validationId = validationId = uniqueId("validation_id_"); - } - asyncValidators.push( - isValid.then( - () => null, // don't consider any valid result from promise since we interested in errors only - error => this.getValidatorError(value, validator) || error, - ), - ); + asyncValidators.push((async () => { + try { + await validator.validate(value, this.props); + } catch (error) { + return this.getValidatorError(value, validator) || (error instanceof Error ? error.message : String(error)); + } + })()); } } @@ -210,10 +193,15 @@ export class Input extends React.Component { // handle async validators result if (asyncValidators.length > 0) { this.setState({ validating: true, valid: false }); - const asyncErrors = await Promise.all(asyncValidators); + const asyncErrors = await Promise.allSettled(asyncValidators); if (this.validationId === validationId) { - this.setValidation(errors.concat(...asyncErrors.filter(err => err))); + errors.push(...asyncErrors + .filter(isPromiseSettledFulfilled) + .map(res => res.value) + .filter(Boolean)); + + this.setValidation(errors); } } @@ -229,9 +217,9 @@ export class Input extends React.Component { } private getValidatorError(value: string, { message }: InputValidator) { - if (isFunction(message)) return message(value, this.props); - - return message || ""; + return typeof message === "function" + ? message(value, this.props) + : message; } private setupValidators() { diff --git a/src/renderer/components/input/input_validators.ts b/src/renderer/components/input/input_validators.ts index a9c03b6866..1f4ff7d459 100644 --- a/src/renderer/components/input/input_validators.ts +++ b/src/renderer/components/input/input_validators.ts @@ -8,9 +8,6 @@ import type { ReactNode } from "react"; import fse from "fs-extra"; import { TypedRegEx } from "typed-regex"; -export class AsyncInputValidationError extends Error { -} - export type InputValidationResult = IsAsync extends true ? Promise @@ -42,12 +39,11 @@ export type InputValidator; - message?: undefined; + message?: ReactNode | SyncValidationMessageBuilder; debounce: number; } ); @@ -131,10 +127,8 @@ export const isPath = asyncInputValidator({ debounce: 100, condition: ({ type }) => type === "text", validate: async value => { - try { - await fse.pathExists(value); - } catch { - throw new AsyncInputValidationError(`${value} is not a valid file path`); + if (!await fse.pathExists(value)) { + throw new Error(`"${value}" is not a valid file path`); } }, });