-
Notifications
You must be signed in to change notification settings - Fork 391
Mixed support #2409
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
base: master
Are you sure you want to change the base?
Mixed support #2409
Changes from all commits
f8c2caf
c078710
ece92ac
bd28667
a62efb4
4bc6b73
4716a43
807546f
25300db
81c81db
3a4f2b9
6b5dca8
2f96aef
cc44091
d460e6c
1f24b21
d716953
498cbf1
72e17d0
89bfb0b
ef24610
492e5fd
434a1f9
532fa9f
22760af
8a1b523
1d51ee8
770531a
6ead876
b7460ab
27c11fe
318950e
3a6f240
e3c8b3c
40dc34f
a7fa97b
9111316
0a6047f
1c146bd
fd7efb4
0e1577b
6b863d9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -592,11 +592,12 @@ export const mapStateToControlProps = ( | |
const required = | ||
controlElement.scope !== undefined && | ||
isRequired(ownProps.schema, controlElement.scope, rootSchema); | ||
const resolvedSchema = Resolve.schema( | ||
ownProps.schema || rootSchema, | ||
controlElement.scope, | ||
rootSchema | ||
); | ||
const resolvedSchema = | ||
Resolve.schema( | ||
ownProps.schema || rootSchema, | ||
controlElement.scope, | ||
rootSchema | ||
) ?? ownProps.schema; // if resolve fails then rely that the ownProps.schema if exist | ||
const errors = getErrorAt(path, resolvedSchema)(state); | ||
|
||
const description = | ||
|
@@ -871,7 +872,21 @@ export const mapStateToArrayControlProps = ( | |
const { path, schema, uischema, label, ...props } = | ||
mapStateToControlWithDetailProps(state, ownProps); | ||
|
||
const resolvedSchema = Resolve.schema(schema, 'items', props.rootSchema); | ||
let resolvedSchema = Resolve.schema(schema, 'items', props.rootSchema); | ||
if ((resolvedSchema as any) === true) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't like the cast to |
||
// help the testers to determine the mixed control | ||
resolvedSchema = { | ||
type: [ | ||
'array', | ||
'boolean', | ||
'integer', | ||
'null', | ||
'number', | ||
'object', | ||
'string', | ||
], | ||
}; | ||
} | ||
Comment on lines
+876
to
+889
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't like this. Up until now, the I'm all for being able to handle |
||
const childErrors = getSubErrorsAt(path, resolvedSchema)(state); | ||
|
||
return { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -123,6 +123,14 @@ const resolveSchemaWithSegments = ( | |
pathSegments: string[], | ||
rootSchema: JsonSchema | ||
): JsonSchema => { | ||
if ( | ||
typeof schema === 'boolean' && | ||
(!pathSegments || pathSegments.length === 0) | ||
) { | ||
// add the case where the schema can be true value for example: items: true or additionalProperties: false | ||
return schema; | ||
} | ||
Comment on lines
+126
to
+132
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the use case of this? Who hands over segments to resolve if the schema is just a boolean already anyway? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that is to resolve the case where the value of the last path segment is equals to true. For example items: true and you want to bind that items then you need to resolve the jsonschema which can be boolean but in the code we ignore that this can be ever the case. Also the next check for isEmpty if we pass true/false values which are valid JSON schema will return an undefined when resolved instead of boolean There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The case is already handled via
If having a boolean I guess the typing does not properly reflect that the |
||
|
||
if (isEmpty(schema)) { | ||
return undefined; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
/* | ||
The MIT License | ||
|
||
Copyright (c) 2017-2019 EclipseSource Munich | ||
https://github.com/eclipsesource/jsonforms | ||
|
||
Permission is hereby granted, free of charge, to any person obtaining a copy | ||
of this software and associated documentation files (the "Software"), to deal | ||
in the Software without restriction, including without limitation the rights | ||
to use, copy, modify, merge, publish, distribute, sublicense, and/or sell | ||
copies of the Software, and to permit persons to whom the Software is | ||
furnished to do so, subject to the following conditions: | ||
|
||
The above copyright notice and this permission notice shall be included in | ||
all copies or substantial portions of the Software. | ||
|
||
THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR | ||
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, | ||
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE | ||
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER | ||
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, | ||
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN | ||
THE SOFTWARE. | ||
*/ | ||
import { registerExamples } from '../register'; | ||
|
||
export const schema = { | ||
type: ['array', 'boolean', 'integer', 'null', 'number', 'object', 'string'], | ||
additionalProperties: true, | ||
items: { | ||
type: ['array', 'boolean', 'integer', 'null', 'number', 'object', 'string'], | ||
}, | ||
}; | ||
|
||
export const uischema = { | ||
type: 'Control', | ||
scope: '#/', | ||
}; | ||
|
||
const data = undefined as any; | ||
|
||
registerExamples([ | ||
{ | ||
name: 'json-editor', | ||
label: 'JSON Editor', | ||
data, | ||
schema, | ||
uischema, | ||
}, | ||
]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel this is plastering over bugs. If a schema can't be resolved then this means the handed over
schema
,uischema
combination was wrong. It's better to fail early than falling back to the unresolved schema.