Skip to content

Conversation

@mallachari
Copy link

Fixes #71

Changes:

  • checks node type before inferring
  • adds tests

Screenshot from 2019-08-28 16-29-00

@mallachari mallachari requested a review from P0lip August 28, 2019 14:33
Copy link
Contributor

@P0lip P0lip left a comment

Choose a reason for hiding this comment

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

Looking okay. I'd just extend inferType fn.

export function inferType(node: JSONSchema4): JSONSchema4TypeName | undefined {
  if ('properties' in node) {
    return SchemaKind.Object;
  }

  if ('items' in node) {
    return SchemaKind.Array;
  }

  if ('type' in node) {
    return node.type;
  }

  return;
}

import { JSONSchema4, JSONSchema4TypeName } from 'json-schema';
import { inferType } from './inferType';

export function getNodeType(node: JSONSchema4): JSONSchema4TypeName | JSONSchema4TypeName[] | undefined {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's just extend inferType function.

Copy link
Author

@mallachari mallachari Aug 28, 2019

Choose a reason for hiding this comment

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

That was actually my first intention but thought it might not be precise enough as it is used separately in few places in the code already.
I'll move it there. But if so shouldn't type be checked before properties and items?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, it could be moved to the top.

import { Property, Types } from '../shared';

describe('Property component', () => {
it('should render Types with propper type and subtype', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@mallachari mallachari requested a review from P0lip August 28, 2019 18:33
Copy link
Contributor

@P0lip P0lip left a comment

Choose a reason for hiding this comment

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

Good job.

@mallachari mallachari merged commit 4bcd27c into master Aug 29, 2019
@mallachari mallachari deleted the fix/array-subtype branch August 29, 2019 09:36
@stoplight-bot
Copy link
Collaborator

🎉 This PR is included in version 2.6.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

JSE > Array fields do not display type in "Read" view

4 participants