From e620432eb4c2e442c02c3b00cd97bb237e11f309 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ro=C5=BCek?= Date: Wed, 21 Jun 2023 13:45:38 +0200 Subject: [PATCH] feat: show internal schema errors in a nicer way (#227) --- src/__tests__/index.spec.tsx | 2 + src/components/SchemaRow/SchemaRow.tsx | 48 +++---------------- .../SchemaRow/TopLevelSchemaRow.tsx | 8 +--- src/components/__tests__/SchemaRow.spec.tsx | 18 +++---- .../__tests__/TopLevelSchemaRow.spec.tsx | 24 +++++----- src/components/shared/Error.tsx | 43 +++++++++++++++++ src/components/shared/Validations.tsx | 25 +--------- src/utils/getInternalSchemaError.ts | 29 +++++++++++ 8 files changed, 105 insertions(+), 92 deletions(-) create mode 100644 src/components/shared/Error.tsx create mode 100644 src/utils/getInternalSchemaError.ts diff --git a/src/__tests__/index.spec.tsx b/src/__tests__/index.spec.tsx index 33cffbc8..ef686840 100644 --- a/src/__tests__/index.spec.tsx +++ b/src/__tests__/index.spec.tsx @@ -685,6 +685,7 @@ describe('$ref resolving', () => {
$ref(#/foo)[]
+
@@ -693,6 +694,7 @@ describe('$ref resolving', () => {
#/foo
+ diff --git a/src/components/SchemaRow/SchemaRow.tsx b/src/components/SchemaRow/SchemaRow.tsx index f6a0ea16..bd89d998 100644 --- a/src/components/SchemaRow/SchemaRow.tsx +++ b/src/components/SchemaRow/SchemaRow.tsx @@ -1,12 +1,5 @@ -import { - isMirroredNode, - isReferenceNode, - isRegularNode, - ReferenceNode, - SchemaNode, - SchemaNodeKind, -} from '@stoplight/json-schema-tree'; -import { Box, Flex, Icon, NodeAnnotation, Select, SpaceVals, VStack } from '@stoplight/mosaic'; +import { isMirroredNode, isReferenceNode, isRegularNode, SchemaNode } from '@stoplight/json-schema-tree'; +import { Box, Flex, NodeAnnotation, Select, SpaceVals, VStack } from '@stoplight/mosaic'; import type { ChangeType } from '@stoplight/types'; import { Atom } from 'jotai'; import { useAtomValue, useUpdateAtom } from 'jotai/utils'; @@ -16,9 +9,10 @@ import * as React from 'react'; import { COMBINER_NAME_MAP } from '../../consts'; import { useJSVOptionsContext } from '../../contexts'; import { getNodeId, getOriginalNodeId } from '../../hash'; -import { calculateChildrenToShow, isFlattenableNode, isPropertyRequired } from '../../tree'; -import { Caret, Description, getInternalSchemaError, getValidationsFromSchema, Types, Validations } from '../shared'; +import { calculateChildrenToShow, isPropertyRequired } from '../../tree'; +import { Caret, Description, getValidationsFromSchema, Types, Validations } from '../shared'; import { ChildStack } from '../shared/ChildStack'; +import { Error } from '../shared/Error'; import { Properties, useHasProperties } from '../shared/Properties'; import { hoveredNodeAtom, isNodeHoveredAtom } from './state'; import { useChoices } from './useChoices'; @@ -60,24 +54,6 @@ export const SchemaRow: React.FunctionComponent = React.memo( const typeToShow = selectedChoice.type; const description = isRegularNode(typeToShow) ? typeToShow.annotations.description : null; - const refNode = React.useMemo(() => { - if (isReferenceNode(schemaNode)) { - return schemaNode; - } - - if ( - isRegularNode(schemaNode) && - (isFlattenableNode(schemaNode) || - (schemaNode.primaryType === SchemaNodeKind.Array && schemaNode.children?.length === 1)) - ) { - return (schemaNode.children?.find(isReferenceNode) as ReferenceNode | undefined) ?? null; - } - - return null; - }, [schemaNode]); - - const isBrokenRef = typeof refNode?.error === 'string'; - const rootLevel = renderRootTreeLines ? 1 : 2; const childNodes = React.useMemo(() => calculateChildrenToShow(typeToShow), [typeToShow]); const combiner = isRegularNode(schemaNode) && schemaNode.combiners?.length ? schemaNode.combiners[0] : null; @@ -89,8 +65,6 @@ export const SchemaRow: React.FunctionComponent = React.memo( const validations = isRegularNode(schemaNode) ? schemaNode.validations : {}; const hasProperties = useHasProperties({ required, deprecated, validations }); - const internalSchemaError = getInternalSchemaError(schemaNode); - const annotationRootOffset = renderRootTreeLines ? 0 : 8; let annotationLeftOffset = -20 - annotationRootOffset; if (nestingLevel > 1) { @@ -199,20 +173,12 @@ export const SchemaRow: React.FunctionComponent = React.memo( validations={isRegularNode(schemaNode) ? getValidationsFromSchema(schemaNode) : {}} hideExamples={hideExamples} /> - - {(isBrokenRef || internalSchemaError.hasError) && ( - - )} + + {renderRowAddon ? {renderRowAddon({ schemaNode, nestingLevel })} : null} - {isCollapsible && isExpanded ? ( - {internalSchemaError.hasError && ( - - )} + ); } diff --git a/src/components/__tests__/SchemaRow.spec.tsx b/src/components/__tests__/SchemaRow.spec.tsx index c62d1b65..bd6351e2 100644 --- a/src/components/__tests__/SchemaRow.spec.tsx +++ b/src/components/__tests__/SchemaRow.spec.tsx @@ -30,7 +30,7 @@ describe('SchemaRow component', () => { it('given no custom resolver, should render a generic error message', () => { const wrapper = mount(); - expect(wrapper.find(Icon).at(1)).toHaveProp('title', `Could not resolve '#/properties/foo'`); + expect(wrapper.find(Icon).at(1)).toHaveProp('aria-label', `Could not resolve '#/properties/foo'`); wrapper.unmount(); }); @@ -44,7 +44,7 @@ describe('SchemaRow component', () => { }); const wrapper = mount(); - expect(wrapper.find(Icon).at(1)).toHaveProp('title', message); + expect(wrapper.find(Icon).at(1)).toHaveProp('aria-label', message); wrapper.unmount(); }); }); @@ -53,7 +53,7 @@ describe('SchemaRow component', () => { let tree: RootNode; let schema: JSONSchema4; - it('given an object schema is marked as internal, a permission denied error messsage should be shown', () => { + it('given an object schema is marked as internal, a permission denied error message should be shown', () => { schema = { type: 'object', 'x-sl-internally-excluded': true, @@ -61,7 +61,7 @@ describe('SchemaRow component', () => { }; tree = buildTree(schema); const wrapper = mount(); - expect(wrapper.find(Icon).at(0)).toHaveProp('title', `You do not have permission to view this reference`); + expect(wrapper.find(Icon).at(0)).toHaveProp('aria-label', `You do not have permission to view this reference`); wrapper.unmount(); }); @@ -73,7 +73,7 @@ describe('SchemaRow component', () => { }; tree = buildTree(schema); const wrapper = mount(); - expect(wrapper.find(Icon).at(0)).toHaveProp('title', `You do not have permission to view this reference`); + expect(wrapper.find(Icon).at(0)).toHaveProp('aria-label', `You do not have permission to view this reference`); wrapper.unmount(); }); @@ -85,7 +85,7 @@ describe('SchemaRow component', () => { }; tree = buildTree(schema); const wrapper = mount(); - expect(wrapper.find(Icon).at(0)).toHaveProp('title', `You do not have permission to view this reference`); + expect(wrapper.find(Icon).at(0)).toHaveProp('aria-label', `You do not have permission to view this reference`); wrapper.unmount(); }); @@ -97,7 +97,7 @@ describe('SchemaRow component', () => { }; tree = buildTree(schema); const wrapper = mount(); - expect(wrapper.find(Icon).at(0)).toHaveProp('title', `You do not have permission to view this reference`); + expect(wrapper.find(Icon).at(0)).toHaveProp('aria-label', `You do not have permission to view this reference`); wrapper.unmount(); }); @@ -109,7 +109,7 @@ describe('SchemaRow component', () => { }; tree = buildTree(schema); const wrapper = mount(); - expect(wrapper.find(Icon).at(0)).toHaveProp('title', `You do not have permission to view this reference`); + expect(wrapper.find(Icon).at(0)).toHaveProp('aria-label', `You do not have permission to view this reference`); wrapper.unmount(); }); @@ -125,7 +125,7 @@ describe('SchemaRow component', () => { }; tree = buildTree(schema); const wrapper = mount(); - expect(wrapper.find(Icon).at(0)).toHaveProp('title', `You do not have permission to view this reference`); + expect(wrapper.find(Icon).at(0)).toHaveProp('aria-label', `You do not have permission to view this reference`); wrapper.unmount(); }); }); diff --git a/src/components/__tests__/TopLevelSchemaRow.spec.tsx b/src/components/__tests__/TopLevelSchemaRow.spec.tsx index 3b04e96f..3e93f7c7 100644 --- a/src/components/__tests__/TopLevelSchemaRow.spec.tsx +++ b/src/components/__tests__/TopLevelSchemaRow.spec.tsx @@ -13,7 +13,7 @@ describe('resolving permission error', () => { let tree: RootNode; let schema: JSONSchema4; - it('given an object schema has a mixture of properties with and without x-sl-error-message, a permission denied error messsage should be shown on properties with x-sl-error-message', () => { + it('given an object schema has a mixture of properties with and without x-sl-error-message, a permission denied error message should be shown on properties with x-sl-error-message', () => { schema = { title: 'User', type: 'object', @@ -41,14 +41,14 @@ describe('resolving permission error', () => { tree = buildTree(schema); const wrapper = mount(); - expect(wrapper.find(Icon).at(0)).toHaveProp('title', `You do not have permission to view this reference`); - expect(wrapper.find(Icon).at(1)).toHaveProp('title', `You do not have permission to view this reference`); - expect(wrapper.find(Icon).at(2)).not.toHaveProp('title', `You do not have permission to view this reference`); - expect(wrapper.find(Icon).at(3)).not.toHaveProp('title', `You do not have permission to view this reference`); + expect(wrapper.find(Icon).at(0)).toHaveProp('aria-label', `You do not have permission to view this reference`); + expect(wrapper.find(Icon).at(1)).toHaveProp('aria-label', `You do not have permission to view this reference`); + expect(wrapper.find(Icon).at(2)).not.toHaveProp('aria-label', `You do not have permission to view this reference`); + expect(wrapper.find(Icon).at(3)).not.toHaveProp('aria-label', `You do not have permission to view this reference`); wrapper.unmount(); }); - it('given an object schema with all properties containing x-sl-error-message, a permission denied error messsage should be shown for each', () => { + it('given an object schema with all properties containing x-sl-error-message, a permission denied error message should be shown for each', () => { schema = { title: 'User', type: 'object', @@ -74,13 +74,13 @@ describe('resolving permission error', () => { tree = buildTree(schema); const wrapper = mount(); - expect(wrapper.find(Icon).at(0)).toHaveProp('title', `You do not have permission to view this reference`); - expect(wrapper.find(Icon).at(1)).toHaveProp('title', `You do not have permission to view this reference`); - expect(wrapper.find(Icon).at(2)).toHaveProp('title', `You do not have permission to view this reference`); + expect(wrapper.find(Icon).at(0)).toHaveProp('aria-label', `You do not have permission to view this reference`); + expect(wrapper.find(Icon).at(1)).toHaveProp('aria-label', `You do not have permission to view this reference`); + expect(wrapper.find(Icon).at(2)).toHaveProp('aria-label', `You do not have permission to view this reference`); wrapper.unmount(); }); - it('given an object schema where the toplevel contains x-sl-error-message, a permission denied error messsage should be shown', () => { + it('given an object schema where the toplevel contains x-sl-error-message, a permission denied error message should be shown', () => { schema = { type: 'object', 'x-sl-error-message': 'You do not have permission to view this reference', @@ -88,11 +88,11 @@ describe('resolving permission error', () => { }; tree = buildTree(schema); const wrapper = mount(); - expect(wrapper.find(Icon).at(0)).toHaveProp('title', `You do not have permission to view this reference`); + expect(wrapper.find(Icon).at(0)).toHaveProp('aria-label', `You do not have permission to view this reference`); wrapper.unmount(); }); - it('given an object schema has properties without ax-sl-error-message, a permission denied error messsage should not be shown', () => { + it('given an object schema has properties without ax-sl-error-message, a permission denied error message should not be shown', () => { schema = { title: 'User', type: 'object', diff --git a/src/components/shared/Error.tsx b/src/components/shared/Error.tsx new file mode 100644 index 00000000..d02aee51 --- /dev/null +++ b/src/components/shared/Error.tsx @@ -0,0 +1,43 @@ +import { isReferenceNode, isRegularNode, ReferenceNode, SchemaNode, SchemaNodeKind } from '@stoplight/json-schema-tree'; +import { Box, Icon, Tooltip } from '@stoplight/mosaic'; +import * as React from 'react'; + +import { isFlattenableNode } from '../../tree'; +import { getInternalSchemaError } from '../../utils/getInternalSchemaError'; + +function useRefNode(schemaNode: SchemaNode) { + return React.useMemo(() => { + if (isReferenceNode(schemaNode)) { + return schemaNode; + } + + if ( + isRegularNode(schemaNode) && + (isFlattenableNode(schemaNode) || + (schemaNode.primaryType === SchemaNodeKind.Array && schemaNode.children?.length === 1)) + ) { + return (schemaNode.children?.find(isReferenceNode) as ReferenceNode | undefined) ?? null; + } + + return null; + }, [schemaNode]); +} + +export const Error: React.FC<{ schemaNode: SchemaNode }> = ({ schemaNode }) => { + const refNode = useRefNode(schemaNode); + const error = getInternalSchemaError(schemaNode) ?? refNode?.error; + + if (typeof error !== 'string') return null; + + return ( + + + + } + > + {error} + + ); +}; diff --git a/src/components/shared/Validations.tsx b/src/components/shared/Validations.tsx index 49402f0d..65c7bbd6 100644 --- a/src/components/shared/Validations.tsx +++ b/src/components/shared/Validations.tsx @@ -1,4 +1,4 @@ -import { isRegularNode, RegularNode, SchemaNode } from '@stoplight/json-schema-tree'; +import { isRegularNode, RegularNode } from '@stoplight/json-schema-tree'; import { Flex, HStack, Text } from '@stoplight/mosaic'; import { Dictionary } from '@stoplight/types'; import capitalize from 'lodash/capitalize.js'; @@ -211,26 +211,3 @@ function getFilteredValidations(schemaNode: RegularNode) { return schemaNode.validations; } - -export function getInternalSchemaError(schemaNode: SchemaNode, defaultErrorMessage?: string) { - let errorMessage: string | undefined; - const fragment: unknown = schemaNode.fragment; - if (typeof fragment === 'object' && fragment !== null) { - const fragmentErrorMessage = fragment['x-sl-error-message']; - if (typeof fragmentErrorMessage === 'string') { - errorMessage = fragmentErrorMessage ?? defaultErrorMessage; - } else { - const items: unknown = fragment['items']; - if (typeof items === 'object' && items !== null) { - const itemsErrorMessage = items['x-sl-error-message']; - if (typeof itemsErrorMessage === 'string') { - errorMessage = itemsErrorMessage ?? defaultErrorMessage; - } - } - } - } - return { - hasError: !!errorMessage, - error: errorMessage, - }; -} diff --git a/src/utils/getInternalSchemaError.ts b/src/utils/getInternalSchemaError.ts new file mode 100644 index 00000000..8cd2b381 --- /dev/null +++ b/src/utils/getInternalSchemaError.ts @@ -0,0 +1,29 @@ +import { isPlainObject } from '@stoplight/json'; +import type { SchemaNode } from '@stoplight/json-schema-tree'; + +export function getInternalSchemaError(schemaNode: SchemaNode): string | undefined { + let errorMessage; + const fragment: unknown = schemaNode.fragment; + if (!isPlainObject(fragment)) return; + + const xStoplight = fragment['x-stoplight']; + + if (isPlainObject(xStoplight) && typeof xStoplight['error-message'] === 'string') { + errorMessage = xStoplight['error-message']; + } else { + const fragmentErrorMessage = fragment['x-sl-error-message']; + if (typeof fragmentErrorMessage === 'string') { + errorMessage = fragmentErrorMessage; + } else { + const items: unknown = fragment['items']; + if (typeof items === 'object' && items !== null) { + const itemsErrorMessage = items['x-sl-error-message']; + if (typeof itemsErrorMessage === 'string') { + errorMessage = itemsErrorMessage; + } + } + } + } + + return errorMessage; +}