Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/elements/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
"@stoplight/http-spec": "^3.1.1",
"@stoplight/json": "^3.10.0",
"@stoplight/json-schema-ref-parser": "^9.0.5",
"@stoplight/json-schema-viewer": "^3.0.0",
"@stoplight/json-schema-viewer": "^4.0.0-beta.7",
"@stoplight/markdown": "^2.10.0",
"@stoplight/markdown-viewer": "^4.3.3",
"@stoplight/mosaic": "1.0.0-beta.33",
Expand Down
13 changes: 8 additions & 5 deletions packages/elements/src/components/Docs/HttpOperation/Body.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { Select } from '@stoplight/mosaic';
import { JsonSchemaViewer } from '@stoplight/json-schema-viewer';
import { Box, Select } from '@stoplight/mosaic';
import { IHttpOperationRequestBody } from '@stoplight/types';
import { JSONSchema4 } from 'json-schema';
import * as React from 'react';

import { isJSONSchema } from '../../../utils/guards';
import { MarkdownViewer } from '../../MarkdownViewer';
import { SchemaViewer } from '../../SchemaViewer';
import { SubSectionPanel } from '../Sections';
import { getExamplesObject } from './utils';

export interface BodyProps {
body: IHttpOperationRequestBody;
Expand All @@ -25,7 +25,6 @@ export const Body = ({ body: { contents = [], description }, onChange }: BodyPro
if (contents.length === 0 && !description) return null;

const schema = contents[chosenContent]?.schema;
const examples = getExamplesObject(contents[chosenContent]?.examples || []);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens to these now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above, examples are now being rendered by the Response Examples component on the right.


return (
<SubSectionPanel
Expand All @@ -42,7 +41,11 @@ export const Body = ({ body: { contents = [], description }, onChange }: BodyPro
>
{description && <MarkdownViewer className="mb-6" markdown={description} />}

{isJSONSchema(schema) && <SchemaViewer className="mt-6" schema={schema} examples={examples} viewMode="write" />}
{isJSONSchema(schema) && (
<Box ml={-9}>
<JsonSchemaViewer schema={schema as JSONSchema4} viewMode="write" />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are we casting here? 🤔

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, sometimes we use our SchemaViewer wrapper, sometimes we use JSV directly. What is the logic behind that?

Copy link
Contributor Author

@marcelltoth marcelltoth Mar 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me answer the 2nd question first:

SchemaViewer is a wrapper that renders both the schema and the corresponding examples (if any) in a tab-panel. By the new design we don't need this behavior, we simply want to render the schema. If there are any examples those will be displayed on the side inside the Response Examples panel.

Now the cast: To be honest, I don't know. I simply copied the rendering of JsonSchemaViewer from SchemaViewer to here. That had the cast, this needs the cast as well, as JSV only advertises JSONSchema4 support. Now on why, and how to resolve this cast, we should discuss with @stoplightio/void-crew , thanks for raising that. But I think this should happen separately from this as this is the same thing we've been doing since forever.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case I would think about renaming SchemaViewer somehow. It seems it's purpose is more specific than its name suggests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking about that, but didn't want to expand the scope too much. If you think that's fine, I'll do it now!

</Box>
)}
</SubSectionPanel>
);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,6 @@ import { HttpOperation as HttpOperationWithoutPersistence } from './index';

const HttpOperation = withPersistenceBoundary(HttpOperationWithoutPersistence);

jest.mock('@stoplight/json-schema-viewer', () => ({
__esModule: true,
PropertyTypeColors: {},
JsonSchemaViewer: () => <div>This is JsonSchemaViewer</div>,
}));

describe('HttpOperation', () => {
describe('Header', () => {
it('should display "Deprecated" badge for deprecated http operation', () => {
Expand Down Expand Up @@ -272,7 +266,18 @@ describe('HttpOperation', () => {
method: 'get',
request: {
body: {
contents: [{ mediaType: 'application/json', schema: {} }, { mediaType: 'application/xml' }],
contents: [
{
mediaType: 'application/json',
schema: {
type: 'object' as const,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the point of casting 'object' as const?

Copy link
Contributor Author

@marcelltoth marcelltoth Mar 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise the entire thingy is not a valid IHttpOperation object, because JSONSchema4 inside IHttpOperation dictates that type: 'object' | 'string" | ..... You know - unless you put the as const - string literals inside objects get widened to string, and that would give me an error when passing as a prop. You can try it if you wish.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You make a very good point though. It would be much nicer to read if I simply annotated the type of the data variable instead. Let me see if that is feasible.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure it's possible! Updated 😉

properties: {
some_property: { type: 'string' as const },
},
},
},
{ mediaType: 'application/xml' },
],
},
},
responses: [
Expand Down Expand Up @@ -342,7 +347,7 @@ describe('HttpOperation', () => {
const body = screen.getByRole('heading', { name: 'Body' });
userEvent.click(body);

expect(await screen.findByText('This is JsonSchemaViewer')).toBeInTheDocument();
expect(await screen.findByText('some_property')).toBeInTheDocument();
});

it('request body selection in Docs should update TryIt', async () => {
Expand Down Expand Up @@ -376,7 +381,18 @@ describe('HttpOperation', () => {
{
code: '200',
description: 'Hello world!',
contents: [{ mediaType: 'application/json', schema: {} }, { mediaType: 'application/xml' }],
contents: [
{
mediaType: 'application/json',
schema: {
type: 'object' as const,
properties: {
some_property: { type: 'string' as const },
},
},
},
{ mediaType: 'application/xml' },
],
},
],
};
Expand Down Expand Up @@ -431,7 +447,14 @@ describe('HttpOperation', () => {
const body = screen.getByRole('heading', { name: 'Body' });
userEvent.click(body);

expect(await screen.findByText('This is JsonSchemaViewer')).toBeInTheDocument();
const property = await screen.findByText('some_property');
expect(property).toBeInTheDocument();

const select = screen.getByLabelText('Choose Response Body Content Type');

userEvent.selectOptions(select, 'application/xml');

expect(screen.queryByText('some_property')).not.toBeInTheDocument();
});
});
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { PropertyTypeColors } from '@stoplight/json-schema-viewer';
import { VStack } from '@stoplight/mosaic';
import { Dictionary, HttpParamStyles, IHttpParam, Primitive } from '@stoplight/types';
import { Tag } from '@stoplight/ui-kit';
Expand Down Expand Up @@ -50,7 +49,7 @@ export const Parameters: React.FunctionComponent<ParametersProps> = ({ parameter

return (
<VStack spacing={4} divider>
{sortBy(parameters, ['required', 'name']).map((parameter, index) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we remove sorting by index here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did I? I believe that was simply an unused argument so I simply removed it.

{sortBy(parameters, ['required', 'name']).map(parameter => {
const resolvedSchema =
parameter.schema?.$ref && resolveRef
? resolveRef({ pointer: parameter.schema.$ref, source: null }, null, {})
Expand Down Expand Up @@ -107,7 +106,7 @@ export const Parameter: React.FunctionComponent<IParameterProps> = ({ parameter,
<div className="HttpOperation__Parameters">
<div className="flex items-center">
<div className="font-medium font-mono">{parameter.name}</div>
<div className={cn('ml-2 text-sm', PropertyTypeColors[type])}>{format ? `${type}<${format}>` : type}</div>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the minimalistic unicolor look 😎

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, was that supposed to be changed too? I know it matches jsv types and I like it this way. Just wondering if that's the planned thing.
Also we could make that text muted so it looks the same as in jsv.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is all grayscale, see here: https://www.figma.com/file/IkNc59AU2JN5Kc1qbd2AiB/Elements?node-id=157%3A0

So it was planned for in the design. It wasn't planned to be done in this issue (the issues don't quite cover the design changes fully, actually), but as JSV removed the colors it also removed the PropertyTypeColors export so I had to do this otherwise we crash.

re muting: good idea, thanks, I'll fix that!

<div className={'ml-2 text-sm'}>{format ? `${type}<${format}>` : type}</div>
{parameterType !== 'path' && (
<div
className={cn('ml-2 text-sm', {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
import { JsonSchemaViewer } from '@stoplight/json-schema-viewer';
import { Box, Select, Tab, TabList, TabPanel, Tabs, Text } from '@stoplight/mosaic';
import { IHttpOperationResponse } from '@stoplight/types';
import { JSONSchema4 } from 'json-schema';
import { sortBy, uniqBy } from 'lodash';
import * as React from 'react';

import { useInlineRefResolver } from '../../../context/InlineRefResolver';
import { MarkdownViewer } from '../../MarkdownViewer';
import { SchemaViewer } from '../../SchemaViewer';
import { SectionTitle, SubSectionPanel } from '../Sections';
import { Parameters } from './Parameters';
import { getExamplesObject } from './utils';

export const HttpCodeColor = {
1: 'gray',
Expand Down Expand Up @@ -73,10 +74,10 @@ export const Response = ({
onMediaTypeChange,
}: ResponseProps) => {
const [chosenContent, setChosenContent] = React.useState(0);
const refResolver = useInlineRefResolver();

const responseContent = contents[chosenContent];
const schema = responseContent?.schema;
const examples = getExamplesObject(responseContent?.examples || []);

React.useEffect(() => {
responseContent && onMediaTypeChange(responseContent.mediaType);
Expand Down Expand Up @@ -104,7 +105,11 @@ export const Response = ({
/>
}
>
{schema && <SchemaViewer schema={schema} examples={examples} viewMode="read" forceShowTabs />}
{schema && (
<Box ml={-9}>
<JsonSchemaViewer schema={schema as JSONSchema4} resolveRef={refResolver} viewMode="read" />
</Box>
)}
</SubSectionPanel>
)}
</Box>
Expand Down
12 changes: 0 additions & 12 deletions packages/elements/src/components/Docs/HttpOperation/utils.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,5 @@
import { INodeExample, INodeExternalExample } from '@stoplight/types';
import { isObject } from 'lodash';

export function getExamplesObject(examples: Array<INodeExample | INodeExternalExample>) {
return examples.reduce((collection, item) => {
const value = 'externalValue' in item ? item.externalValue : item.value;
if (value) {
collection[item.key] = value;
}

return collection;
}, {});
}

export function getExamplesFromSchema(data: unknown) {
// `examples` are available in JSON Schema v6 and v7. For v4 we can use `x-examples`.
// `example` is not supported by any version but we can use `x-example` extension.
Expand Down
2 changes: 1 addition & 1 deletion packages/elements/src/components/Docs/Model/Model.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const ModelComponent: React.FC<ModelProps> = ({ data, className, headless }) =>
<div className={cn('Model MarkdownViewer', className)}>
{!headless && data.title !== void 0 && <h1 className={Classes.HEADING}>{data.title}</h1>}

<SchemaViewer schema={data} description={data.description} examples={getExamplesFromSchema(data)} maxRows={50} />
<SchemaViewer schema={data} description={data.description} examples={getExamplesFromSchema(data)} />
</div>
);
};
Expand Down
6 changes: 0 additions & 6 deletions packages/elements/src/components/SchemaViewer/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,22 +21,19 @@ export interface ISchemaViewerProps {
title?: string;
description?: string;
errors?: string[];
maxRows?: number;
examples?: Dictionary<string>;
className?: string;
forceShowTabs?: boolean;
viewMode?: ViewMode;
}

const JSV_MAX_ROWS = 20;
export const SchemaViewer = ({
className,
title,
description,
schema,
examples,
errors,
maxRows = JSV_MAX_ROWS,
viewMode,
forceShowTabs,
}: ISchemaViewerProps) => {
Expand All @@ -52,13 +49,10 @@ export const SchemaViewer = ({
{description && <MarkdownViewer markdown={description} />}

<JsonSchemaViewer
mergeAllOf
resolveRef={resolveRef}
className={jsvClassName}
schema={schema as JSONSchema4}
maxRows={maxRows}
viewMode={viewMode}
shouldResolveEagerly
/>
</>
);
Expand Down
2 changes: 1 addition & 1 deletion packages/elements/src/context/InlineRefResolver.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { resolveInlineRef } from '@stoplight/json';
import { SchemaTreeRefDereferenceFn } from '@stoplight/json-schema-viewer';
import type { SchemaTreeRefDereferenceFn } from '@stoplight/json-schema-tree';
import { isPlainObject } from 'lodash';
import * as React from 'react';
import { useContext } from 'react';
Expand Down
Loading