Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Schema composition ('allOf') is always rendered first irrespective of where it is defined in the schema #169

Open
pspot2 opened this issue Oct 27, 2022 · 4 comments
Open

Schema composition ('allOf') is always rendered first irrespective of where it is defined in the schema #169

pspot2 opened this issue Oct 27, 2022 · 4 comments
Labels
enhancement New feature or request

Comments

@pspot2
Copy link

pspot2 commented Oct 27, 2022

Hi,

thanks for this wonderful tool. This is the best open-source schema documentation generator I've stumbled upon so far. It does many things right and the Markdown output and the templating layer are fantastic. However, as the schema size scales, the tool reveals several issues that need improvement. I hope I will be able to contribute myself, but first the issues need to be raised and confirmed that this is not something that "works-as-designed".

Issue: items of composed schemas (allOf in my case) are displayed first no matter where they are really listed in the (sub)schema.

Consider the following schema:

Click to expand 
{
    "$id": "file:///home/eub-user/schema_test1.json",
    "$schema": "https://json-schema.org/draft/2020-12/schema",
    "title": "root",
    "description": "Root-level schema",
    "type": "object",
    "required": [
        "test_property01",
        "test_property02",
        "test_property03"
    ],
    "properties": {
        "test_property01": {
            "description": "Test 01",
            "type": "string",
            "pattern": "^[a-z]{2,}[-_a-z0-9]+$"
        },
        "test_property02": {
            "description": "Test 02",
            "type": "integer"
        },
        "test_property03": {
            "description": "Test 03",
            "enum": [
                "myvalue1",
                "myvalue2"
            ]
        }
    },
    "allOf": [
        {
            "if": {
                "properties": {
                    "test_property03": {
                        "const": "myvalue1"
                    }
                },
                "required": [
                    "test_property03"
                ]
            },
            "then": {
                "description": "TBD",
                "required": [
                    "test_propertyA"
                ],
                "properties": {
                    "test_propertyA": {
                        "description": "Test A",
                        "type": "string"
                    }
                }
            }
        },
        {
            "if": {
                "properties": {
                    "test_property03": {
                        "const": "myvalue2"
                    }
                },
                "required": [
                    "test_property03"
                ]
            },
            "then": {
                "description": "TBD",
                "required": [
                    "test_propertyB"
                ],
                "properties": {
                    "test_propertyB": {
                        "description": "Test B",
                        "type": "string"
                    }
                }
            }
        }
    ]
}

There are 3 simple properties and an if-then condition that adds a fourth property based on the value of the third property. Simple properties are defined before the branches and this is how the documentation is expected to be rendered: the first 3 properties are rendered first and the branching afterwards:

Click to expand

root

Title: root
Type combining
Additional properties Any type: allowed

Description: Root-level schema

Property Pattern Type Deprecated Definition Title/Description
+ test_property01 No string No - Test 01
+ test_property02 No integer No - Test 02
+ test_property03 No enum (of string) No - Test 03
All of(Requirement)
item 0
item 1

Required Property root > test_property01
Type string

Description: Test 01

Restrictions
Must match regular expression ^[a-z]{2,}[-_a-z0-9]+$ Test

Required Property root > test_property02
Type integer

Description: Test 02

Required Property root > test_property03
Type enum (of string)

Description: Test 03

Must be one of:

  • "myvalue1"
  • "myvalue2"

Property root > allOf > item 0
Type object
Additional properties Any type: allowed

If (test_property03 = "myvalue1")
Type object
Additional properties Any type: allowed

Description: TBD

Property Pattern Type Deprecated Definition Title/Description
+ test_propertyA No string No - Test A

Property root > allOf > item 0 > then > test_propertyA
Type string

Description: Test A

Property root > allOf > item 1
Type object
Additional properties Any type: allowed

If (test_property03 = "myvalue2")
Type object
Additional properties Any type: allowed

Description: TBD

Property Pattern Type Deprecated Definition Title/Description
+ test_propertyB No string No - Test B

Property root > allOf > item 1 > then > test_propertyB
Type string

Description: Test B

Generated using json-schema-for-humans on 2022-10-27 at 14:32:59 +0000

Actual result: the branching subschemas are rendered first, followed by the simple 3 properties:

Click to expand

root

Title: root
Type combining
Additional properties Any type: allowed

Description: Root-level schema

Property Pattern Type Deprecated Definition Title/Description
+ test_property01 No string No - Test 01
+ test_property02 No integer No - Test 02
+ test_property03 No enum (of string) No - Test 03
All of(Requirement)
item 0
item 1

Property root > allOf > item 0
Type object
Additional properties Any type: allowed

If (test_property03 = "myvalue1")
Type object
Additional properties Any type: allowed

Description: TBD

Property Pattern Type Deprecated Definition Title/Description
+ test_propertyA No string No - Test A

Property root > allOf > item 0 > then > test_propertyA
Type string

Description: Test A

Property root > allOf > item 1
Type object
Additional properties Any type: allowed

If (test_property03 = "myvalue2")
Type object
Additional properties Any type: allowed

Description: TBD

Property Pattern Type Deprecated Definition Title/Description
+ test_propertyB No string No - Test B

Property root > allOf > item 1 > then > test_propertyB
Type string

Description: Test B

Required Property root > test_property01
Type string

Description: Test 01

Restrictions
Must match regular expression ^[a-z]{2,}[-_a-z0-9]+$ Test

Required Property root > test_property02
Type integer

Description: Test 02

Required Property root > test_property03
Type enum (of string)

Description: Test 03

Must be one of:

  • "myvalue1"
  • "myvalue2"

Generated using json-schema-for-humans on 2022-10-27 at 14:32:59 +0000

This becomes especially counterintuitive and difficult to read when the branching subtree goes several levels deep (and potentially references other schemas). In this case one would see the 3 root-level simple properties at the very bottom of the (potentially large) document which doesn't make any sense.
@pspot2 pspot2 mentioned this issue Oct 27, 2022
Open
@dblanchette dblanchette added the enhancement New feature or request label Nov 9, 2022
@dblanchette
Copy link
Collaborator

Thanks for all the reports.

Displaying allOf and the properties (as long as other type of blocks) in the order they appear would be difficult with the way the code is currently built. I have an idea to change that, but I'm not sure when I'll have/take time to work on something more than short/easy bugfixes.

An easy change would be to display properties first, I think it would be better than the current implementation, since properties are certain, while conditional blocks are not.

Do you think this would be a positive?

@pspot2
Copy link
Author

pspot2 commented Nov 9, 2022

Do you think this would be a positive?

Yes, definitely. Even this initial fix would have a major impact on readability.

@mbronk
Copy link

mbronk commented Jul 12, 2024

An easy change would be to display properties first (...) Do you think this would be a positive?

@dblanchette - sorry for resurrecting such an old issue... I concur this simple change would immensely help readability.
Is there any chance for it getting introduced (perhaps a config option even?) in the foreseeable future?

@Jellyfrog
Copy link

was this ever implemented? think we're hitting the same issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@Jellyfrog @mbronk @dblanchette @pspot2