Skip to content

Commit

Permalink
feat: show internal schema errors in a nicer way (#227)
Browse files Browse the repository at this point in the history
  • Loading branch information
P0lip authored Jun 21, 2023
1 parent 78aaa58 commit e620432
Show file tree
Hide file tree
Showing 8 changed files with 105 additions and 92 deletions.
2 changes: 2 additions & 0 deletions src/__tests__/index.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -685,6 +685,7 @@ describe('$ref resolving', () => {
<div><span>$ref(#/foo)[]</span></div>
</div>
</div>
<span></span>
</div>
<div data-level=\\"0\\">
<div data-id=\\"98538b996305d\\">
Expand All @@ -693,6 +694,7 @@ describe('$ref resolving', () => {
<div><span>#/foo</span></div>
</div>
</div>
<span></span>
</div>
</div>
</div>
Expand Down
48 changes: 7 additions & 41 deletions src/components/SchemaRow/SchemaRow.tsx
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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';
Expand Down Expand Up @@ -60,24 +54,6 @@ export const SchemaRow: React.FunctionComponent<SchemaRowProps> = React.memo(
const typeToShow = selectedChoice.type;
const description = isRegularNode(typeToShow) ? typeToShow.annotations.description : null;

const refNode = React.useMemo<ReferenceNode | null>(() => {
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;
Expand All @@ -89,8 +65,6 @@ export const SchemaRow: React.FunctionComponent<SchemaRowProps> = 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) {
Expand Down Expand Up @@ -199,20 +173,12 @@ export const SchemaRow: React.FunctionComponent<SchemaRowProps> = React.memo(
validations={isRegularNode(schemaNode) ? getValidationsFromSchema(schemaNode) : {}}
hideExamples={hideExamples}
/>

{(isBrokenRef || internalSchemaError.hasError) && (
<Icon
title={refNode?.error! || internalSchemaError.error}
color="danger"
icon={['fas', 'exclamation-triangle']}
size="sm"
/>
)}
</VStack>

<Error schemaNode={schemaNode} />

{renderRowAddon ? <Box>{renderRowAddon({ schemaNode, nestingLevel })}</Box> : null}
</Flex>

{isCollapsible && isExpanded ? (
<ChildStack
schemaNode={schemaNode}
Expand Down
8 changes: 2 additions & 6 deletions src/components/SchemaRow/TopLevelSchemaRow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { calculateChildrenToShow, isComplexArray } from '../../tree';
import { showPathCrumbsAtom } from '../PathCrumbs/state';
import { Description } from '../shared';
import { ChildStack } from '../shared/ChildStack';
import { getInternalSchemaError } from '../shared/Validations';
import { Error } from '../shared/Error';
import { SchemaRow, SchemaRowProps } from './SchemaRow';
import { useChoices } from './useChoices';

Expand All @@ -23,8 +23,6 @@ export const TopLevelSchemaRow = ({

const nodeId = schemaNode.fragment?.['x-stoplight']?.id;

const internalSchemaError = getInternalSchemaError(schemaNode);

// regular objects are flattened at the top level
if (isRegularNode(schemaNode) && isPureObjectNode(schemaNode)) {
return (
Expand All @@ -37,9 +35,7 @@ export const TopLevelSchemaRow = ({
currentNestingLevel={nestingLevel}
parentNodeId={nodeId}
/>
{internalSchemaError.hasError && (
<Icon title={internalSchemaError.error} color="danger" icon={['fas', 'exclamation-triangle']} size="sm" />
)}
<Error schemaNode={schemaNode} />
</>
);
}
Expand Down
18 changes: 9 additions & 9 deletions src/components/__tests__/SchemaRow.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ describe('SchemaRow component', () => {

it('given no custom resolver, should render a generic error message', () => {
const wrapper = mount(<SchemaRow schemaNode={tree.children[0]!} nestingLevel={0} />);
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();
});

Expand All @@ -44,7 +44,7 @@ describe('SchemaRow component', () => {
});

const wrapper = mount(<SchemaRow schemaNode={tree.children[0]!} nestingLevel={0} />);
expect(wrapper.find(Icon).at(1)).toHaveProp('title', message);
expect(wrapper.find(Icon).at(1)).toHaveProp('aria-label', message);
wrapper.unmount();
});
});
Expand All @@ -53,15 +53,15 @@ 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,
'x-sl-error-message': 'You do not have permission to view this reference',
};
tree = buildTree(schema);
const wrapper = mount(<SchemaRow schemaNode={tree.children[0]!} nestingLevel={0} />);
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();
});

Expand All @@ -73,7 +73,7 @@ describe('SchemaRow component', () => {
};
tree = buildTree(schema);
const wrapper = mount(<SchemaRow schemaNode={tree.children[0]!} nestingLevel={0} />);
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();
});

Expand All @@ -85,7 +85,7 @@ describe('SchemaRow component', () => {
};
tree = buildTree(schema);
const wrapper = mount(<SchemaRow schemaNode={tree.children[0]!} nestingLevel={0} />);
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();
});

Expand All @@ -97,7 +97,7 @@ describe('SchemaRow component', () => {
};
tree = buildTree(schema);
const wrapper = mount(<SchemaRow schemaNode={tree.children[0]!} nestingLevel={0} />);
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();
});

Expand All @@ -109,7 +109,7 @@ describe('SchemaRow component', () => {
};
tree = buildTree(schema);
const wrapper = mount(<SchemaRow schemaNode={tree.children[0]!} nestingLevel={0} />);
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();
});

Expand All @@ -125,7 +125,7 @@ describe('SchemaRow component', () => {
};
tree = buildTree(schema);
const wrapper = mount(<SchemaRow schemaNode={tree.children[0]!} nestingLevel={0} />);
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();
});
});
Expand Down
24 changes: 12 additions & 12 deletions src/components/__tests__/TopLevelSchemaRow.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -41,14 +41,14 @@ describe('resolving permission error', () => {

tree = buildTree(schema);
const wrapper = mount(<TopLevelSchemaRow schemaNode={tree.children[0]!} />);
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',
Expand All @@ -74,25 +74,25 @@ describe('resolving permission error', () => {

tree = buildTree(schema);
const wrapper = mount(<TopLevelSchemaRow schemaNode={tree.children[0]!} />);
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',
'x-sl-internally-excluded': true,
};
tree = buildTree(schema);
const wrapper = mount(<TopLevelSchemaRow schemaNode={tree.children[0]!} />);
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',
Expand Down
43 changes: 43 additions & 0 deletions src/components/shared/Error.tsx
Original file line number Diff line number Diff line change
@@ -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<ReferenceNode | null>(() => {
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 (
<Tooltip
renderTrigger={
<Box as="span" display="inline-block" ml={1.5}>
<Icon aria-label={error} color="var(--color-danger)" icon={['fas', 'exclamation-triangle']} size="1x" />
</Box>
}
>
{error}
</Tooltip>
);
};
25 changes: 1 addition & 24 deletions src/components/shared/Validations.tsx
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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,
};
}
29 changes: 29 additions & 0 deletions src/utils/getInternalSchemaError.ts
Original file line number Diff line number Diff line change
@@ -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;
}

0 comments on commit e620432

Please sign in to comment.