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.

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

Invalid output from oneOf element/wrong field bindings #1253

Closed
johnmorrell opened this issue Feb 7, 2019 · 17 comments
Closed

Invalid output from oneOf element/wrong field bindings #1253

johnmorrell opened this issue Feb 7, 2019 · 17 comments

Comments

@johnmorrell
Copy link

Describe the bug
Invalid JSON output from oneOf elements, where the output value is nested a level deeper than expected.

To Reproduce

Schema:

{
   "type":"object",
   "properties":{
      "vaule":{
         "oneOf":[
            {
              "title":"String",
              "type":"string"
            },
            {
              "title":"Number",
              "type":"number"
            }
         ]
      }
   }
}

UISchema:

{
   "type":"Control",
   "label":"Value",
   "scope":"#/properties/vaule"
}

Enter string and inspect data as JSON, to get:

{
  "vaule": {
    "vaule": [
      "abcdefg"
    ]
  }
}

Expected behavior

Example output as JSON:

{
    "vaule": "abcdefg"
}

Browser (please complete the following information):

  • Browser: Safari
  • Version: 12.0.3

Used Setup (please complete the following information):

  • Framework: React
  • RendererSet: Material

Additional context
JSON Forms v2.2.0

@edgarmueller
Copy link
Contributor

Thanks for the report! We experienced this issue ourselves in one of our projects and started working on a fix. I hope we'll be able to release a fix in the upcoming days, probably in the beginning of the next week.

@edgarmueller edgarmueller added this to the 2.3.0 milestone Feb 13, 2019
@edgarmueller
Copy link
Contributor

@johnmorrell We've released a new 2.2.1-alpha.0 release which should address this issue. Let me know if it works.

@johnmorrell
Copy link
Author

Thank @edgarmueller, I've take a look at 2.2.1-alpha.0 and the data structure now looks good. The titles are big improvement too, but I did notice a case where the first title would be shown as ONEOF-0 rather than the title value. Simplified examples below:

This works as expected

{
   "oneOf":[
      {
         "title":"Option 1",
         "type":"object",
         "properties":{
            "option1":{
               "oneOf":[
                  {
                     "title":"Option 1A",
                     "type":"string"
                  },
                  {
                     "title":"Option 1B",
                     "type":"string"
                  }
               ]
            }
         }
      },
      {
         "title":"Option 2",
         "type":"object",
         "properties":{
            "option2":{
               "type":"number"
            }
         }
      }
   ]
}

This results in first title being ONEOF-0

{
   "oneOf":[
      {
         "title":"Option 1",
         "$ref":"#/definitions/moreOptions"
      },
      {
         "title":"Option 2",
         "type":"object",
         "properties":{
            "option2":{
               "type":"number"
            }
         }
      }
   ],
   "definitions":{
      "moreOptions":{
         "type":"object",
         "properties":{
            "option1":{
               "oneOf":[
                  {
                     "title":"Option 1A",
                     "type":"string"
                  },
                  {
                     "title":"Option 1B",
                     "type":"string"
                  }
               ]
            }
         }
      }
   }
}

@edgarmueller
Copy link
Contributor

edgarmueller commented Feb 14, 2019

The problem in this case is that the title property is a sibling of the $ref and hence is ignored (as outlined in https://json-schema.org/latest/json-schema-core.html#rfc.section.8.3). Move it into the moreOptions definition and it should work fine.

@johnmorrell
Copy link
Author

Ah yes, of course. Thank you that works fine.

I am also investigating an issue where I can't add an element to an array. It is something that seems to be working in v2.2.0 but not in this alpha. It is nested a couple of levels down but will try and provide a clearer example, assuming it is not an issue with my schema.

@edgarmueller
Copy link
Contributor

We did a bit of refactoring internally related to ref resolution, so that might be the case. Any feedback/help is appreciated.

@johnmorrell
Copy link
Author

johnmorrell commented Feb 14, 2019

There looks like there is an issue, I am seeing this is two different places that work ok on v2.2.0. In one case when adding an element I get a blank screen with message "An unexpected error has occurred.". The error in the console is:

[Violation] Forced reflow while executing JavaScript took 65ms
commons.662dd5e0a7b60d014b53.js:39 TypeError: Cannot read property 'map' of undefined
    at t.MaterialEnumField (_app.js:16)
    at hi (commons.662dd5e0a7b60d014b53.js:39)
    at Gi (commons.662dd5e0a7b60d014b53.js:39)
    at Ji (commons.662dd5e0a7b60d014b53.js:39)
    at Ia (commons.662dd5e0a7b60d014b53.js:39)
    at Da (commons.662dd5e0a7b60d014b53.js:39)
    at Ma (commons.662dd5e0a7b60d014b53.js:39)
    at Or (commons.662dd5e0a7b60d014b53.js:39)
_i @ commons.662dd5e0a7b60d014b53.js:39
i.function.i.componentDidCatch.r.callback @ commons.662dd5e0a7b60d014b53.js:39
fo @ commons.662dd5e0a7b60d014b53.js:39
lo @ commons.662dd5e0a7b60d014b53.js:39
Fa @ commons.662dd5e0a7b60d014b53.js:39
Ia @ commons.662dd5e0a7b60d014b53.js:39
Da @ commons.662dd5e0a7b60d014b53.js:39
Ma @ commons.662dd5e0a7b60d014b53.js:39
Or @ commons.662dd5e0a7b60d014b53.js:39
commons.662dd5e0a7b60d014b53.js:39 TypeError: Cannot read property 'map' of undefined
    at t.MaterialEnumField (_app.js:16)
    at hi (commons.662dd5e0a7b60d014b53.js:39)
    at Gi (commons.662dd5e0a7b60d014b53.js:39)
    at Ji (commons.662dd5e0a7b60d014b53.js:39)
    at Ia (commons.662dd5e0a7b60d014b53.js:39)
    at Da (commons.662dd5e0a7b60d014b53.js:39)
    at Ma (commons.662dd5e0a7b60d014b53.js:39)
    at Or (commons.662dd5e0a7b60d014b53.js:39)
_i @ commons.662dd5e0a7b60d014b53.js:39
i.function.i.componentDidCatch.r.callback @ commons.662dd5e0a7b60d014b53.js:39
fo @ commons.662dd5e0a7b60d014b53.js:39
lo @ commons.662dd5e0a7b60d014b53.js:39
Fa @ commons.662dd5e0a7b60d014b53.js:39
Ia @ commons.662dd5e0a7b60d014b53.js:39
Da @ commons.662dd5e0a7b60d014b53.js:39
Ma @ commons.662dd5e0a7b60d014b53.js:39
Or @ commons.662dd5e0a7b60d014b53.js:39
main-41bbfff1b13a50b42a48.js:1 TypeError: Cannot read property 'map' of undefined
    at t.MaterialEnumField (_app.js:16)
    at hi (commons.662dd5e0a7b60d014b53.js:39)
    at Gi (commons.662dd5e0a7b60d014b53.js:39)
    at Ji (commons.662dd5e0a7b60d014b53.js:39)
    at Ia (commons.662dd5e0a7b60d014b53.js:39)
    at Da (commons.662dd5e0a7b60d014b53.js:39)
    at Ma (commons.662dd5e0a7b60d014b53.js:39)
    at Or (commons.662dd5e0a7b60d014b53.js:39)
(anonymous) @ main-41bbfff1b13a50b42a48.js:1
w @ commons.662dd5e0a7b60d014b53.js:23
(anonymous) @ commons.662dd5e0a7b60d014b53.js:23
e.(anonymous function) @ commons.662dd5e0a7b60d014b53.js:23
o @ commons.662dd5e0a7b60d014b53.js:1
u @ commons.662dd5e0a7b60d014b53.js:1
(anonymous) @ commons.662dd5e0a7b60d014b53.js:1
t @ commons.662dd5e0a7b60d014b53.js:1
(anonymous) @ commons.662dd5e0a7b60d014b53.js:1
J @ main-41bbfff1b13a50b42a48.js:1
B @ main-41bbfff1b13a50b42a48.js:1
(anonymous) @ main-41bbfff1b13a50b42a48.js:1
w @ commons.662dd5e0a7b60d014b53.js:23
(anonymous) @ commons.662dd5e0a7b60d014b53.js:23
e.(anonymous function) @ commons.662dd5e0a7b60d014b53.js:23
o @ commons.662dd5e0a7b60d014b53.js:1
u @ commons.662dd5e0a7b60d014b53.js:1
(anonymous) @ commons.662dd5e0a7b60d014b53.js:1
t @ commons.662dd5e0a7b60d014b53.js:1
(anonymous) @ commons.662dd5e0a7b60d014b53.js:1
(anonymous) @ main-41bbfff1b13a50b42a48.js:1
value @ main-41bbfff1b13a50b42a48.js:1
i.function.i.componentDidCatch.r.callback @ commons.662dd5e0a7b60d014b53.js:39
fo @ commons.662dd5e0a7b60d014b53.js:39
lo @ commons.662dd5e0a7b60d014b53.js:39
Fa @ commons.662dd5e0a7b60d014b53.js:39
Ia @ commons.662dd5e0a7b60d014b53.js:39
Da @ commons.662dd5e0a7b60d014b53.js:39
Ma @ commons.662dd5e0a7b60d014b53.js:39
Or @ commons.662dd5e0a7b60d014b53.js:39

In the other, clicking the add button does nothing and the console error is:

[Violation] Forced reflow while executing JavaScript took 32ms
commons.662dd5e0a7b60d014b53.js:6 Uncaught TypeError: e.push is not a function
    at Object.updater (commons.662dd5e0a7b60d014b53.js:6)
    at t.coreReducer (commons.662dd5e0a7b60d014b53.js:56)
    at commons.662dd5e0a7b60d014b53.js:1
    at commons.662dd5e0a7b60d014b53.js:1
    at m (commons.662dd5e0a7b60d014b53.js:1)
    at commons.662dd5e0a7b60d014b53.js:6
    at Object.<anonymous> (commons.662dd5e0a7b60d014b53.js:39)
    at p (commons.662dd5e0a7b60d014b53.js:39)
    at commons.662dd5e0a7b60d014b53.js:39
    at P (commons.662dd5e0a7b60d014b53.js:39)
(anonymous) @ commons.662dd5e0a7b60d014b53.js:6
t.coreReducer @ commons.662dd5e0a7b60d014b53.js:56
(anonymous) @ commons.662dd5e0a7b60d014b53.js:1
(anonymous) @ commons.662dd5e0a7b60d014b53.js:1
m @ commons.662dd5e0a7b60d014b53.js:1
(anonymous) @ commons.662dd5e0a7b60d014b53.js:6
(anonymous) @ commons.662dd5e0a7b60d014b53.js:39
p @ commons.662dd5e0a7b60d014b53.js:39
(anonymous) @ commons.662dd5e0a7b60d014b53.js:39
P @ commons.662dd5e0a7b60d014b53.js:39
T @ commons.662dd5e0a7b60d014b53.js:39
O @ commons.662dd5e0a7b60d014b53.js:39
A @ commons.662dd5e0a7b60d014b53.js:39
xr @ commons.662dd5e0a7b60d014b53.js:39
La @ commons.662dd5e0a7b60d014b53.js:39
Ne @ commons.662dd5e0a7b60d014b53.js:39
Cr @ commons.662dd5e0a7b60d014b53.js:39
Ma @ commons.662dd5e0a7b60d014b53.js:39
Or @ commons.662dd5e0a7b60d014b53.js:39

I appreciate this probably doesn't help much, I will try and provide an example schema to reproduce.

@johnmorrell
Copy link
Author

I think this captures the issue, in v2.2.0 it is ok apart from the data structure issue and in the alpha it errors with 'No applicable field found.':

{
   "type":"object",
   "properties":{
      "oneOrMoreThings":{
         "oneOf":[
            {
               "$ref":"#/definitions/thing"
            },
            {
               "$ref":"#/definitions/thingArray"
            }
         ]
      }
   },
   "definitions":{
      "thing":{
         "title":"Thing",
         "type":"string"
      },
      "thingArray":{
         "title":"Things",
         "type":"array",
         "items":{
            "$ref":"#/definitions/thing"
         }
      }
   }
}

@edgarmueller
Copy link
Contributor

Thanks for the repro! I've had a look and it seems as if this is actually an error in the schemaSubPathMatches tester. We'll provide a fix tomorrow.

@edgarmueller
Copy link
Contributor

edgarmueller commented Feb 15, 2019

The tester has been fixed with v2.2.1-apha.2. I've tested it with your examples and it seems to do the trick, let me know if you experience any issues.

@johnmorrell
Copy link
Author

I've just tried alpha.2 and I see the same two issues I did originally in my Schema, but the example above does now work. I'll try again for a simplified repro with this version and see what that looks like.

@johnmorrell
Copy link
Author

A slight variation on the previous example, but with a oneOf as an object. This example behaves as one of the issues I see in my schema, where the array shows but pressing add does nothing, erroring to console.

{
   "type":"object",
   "properties":{
      "thingOrThings":{
         "oneOf":[
            {
               "title":"Thing",
               "type":"object",
               "properties":{
                  "thing":{
                     "$ref":"#/definitions/thing"
                  }
               }
            },
            {
               "$ref":"#/definitions/thingArray"
            }
         ]
      }
   },
   "definitions":{
      "thing":{
         "title":"Thing",
         "type":"string"
      },
      "thingArray":{
         "title":"Things",
         "type":"array",
         "items":{
            "$ref":"#/definitions/thing"
         }
      }
   }
}

@edgarmueller
Copy link
Contributor

edgarmueller commented Feb 15, 2019

Thanks for the example, I'll try to have a look later today.

@johnmorrell
Copy link
Author

@edgarmueller thanks for the latest release. I have tested against alpha.3 and one of the two issues is fixed, unfortunately there seems to be one remaining, where I see the "An unexpected error has occurred." when adding to an array. This actually isn't within a oneOf, but is something that was working in v2.2.0 so probably related.

It can be reproduced with:

{
   "type":"object",
   "properties":{
      "things":{
         "type":"array",
         "items":{
            "type":"object",
            "properties":{
               "thing":{
                  "type":"string",
                  "enum":[
                     "thing"
                  ]
               }
            }
         }
      }
   }
}

@edgarmueller edgarmueller changed the title Invalid output from oneOf element Invalid output from oneOf element/wrong field bindings Feb 20, 2019
@edgarmueller
Copy link
Contributor

edgarmueller commented Feb 20, 2019

@johnmorrell Thanks again for the example. Unfortunately it seems as our recent refactoring broke a couple of things which we didn't have tests for. We have added those now, so I hope it's fixed with 2.2.1-alpha.4. Let me now if you are still experiencing any issues.

@johnmorrell
Copy link
Author

@edgarmueller alpha.4 looks good!!

Thank you for all you help on this. We've been reviewing JSON Schema form libraries over the past 6 months and non have been able to support our schema. Mostly due to the heavy use of oneOf and general size and complexity ~2000 lines! This is the first time I have seen it fully supported.

The original and follow up issues all look resolved, so feel free to close the issue. At some point we would love to have #1210 allowing us to use our schemas in their original form rather than combining to a single file, but it is great to be where we are with this.

@edgarmueller
Copy link
Contributor

Great, glad it works! Thanks for all the examples!

@edgarmueller edgarmueller modified the milestones: 2.3.0, 2.2.1 Feb 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants