eclipsesource / jsonforms

Customizable JSON Schema-based forms with React, Angular and Vue support out of the box.
http://jsonforms.io
Other
2.15k stars 361 forks source link

Using "#" as the scope in a rule condition always evaluates to true. #2141

Open lellimecnar opened 1 year ago

lellimecnar commented 1 year ago

Describe the bug

If, in a rule condition, I use "#" as the scope, resolveData will resolve to undefined instead of the data itself, so evaluateCondition will always return true.

Expected behavior

Using "#" as the scope should evaluate condition.schema on the value of data.

Steps to reproduce the issue

  1. Have a rule in your uischema with `"#"` as the scope of the condition ```json { "type": "Control", "scope": "#/properties/someObj", "rule": { "effect": "SHOW", "condition": { "scope": "#", "schema": { "properties": { "someProp": { "const": "someValue" } } } } } } ```
  2. The control should only be visible when `someObj.someProp` equals `"someValue"`, but it always renders.

Screenshots

No response

In which browser are you experiencing the issue?

Chrome/113.0.0.0

Which Version of JSON Forms are you using?

3.0.0

Framework

React

RendererSet

Vanilla

Additional context

The only way I could get isVisible to return false is to hard-code an empty string for the path argument.

sdirix commented 1 year ago

Hi @lellimecnar,

Thanks for the report.

In general we don't just return undefined when the scope is #, see here. Can you confirm that your whole data object is not undefined in the error case? What happens if you hand over an empty object to JSON Forms?

lellimecnar commented 1 year ago

I think this an issue of ambiguity. Some functions (like resolveData) almost never seen to do what I expect. Both JSON pointers and dot-delimited paths are referred to as "path" in different places in the code, so I have to look at the source to figure out which to use. I think this issue was because I was passing the scope to isVisible instead of the path.

I saw that you closed my other issue (#2140). I think both of these can be summed up as "paths/pointers need a more robust implementation." I'd like to at least submit a PR which allows a config option to override resolution behavior. Being able to use JSON pointers, JSON paths, jsonata, etc. would allow for more advanced scopes.

In my schema, I have an array of vehicles, each with an id. In a separate array, there are items related to vehicles, each with a vehicleId. I need some way to "lookup" the related items by vehicleId to render those fields alongside the vehicle fields.

(and no, I don't have control over the JSON schema)

sdirix commented 1 year ago

In the JSON Forms code base we have two different kind of paths:

The prop for the renderers is called path and is the current dataPath. Renderers typically don't care about the schemaPath which is why the specification was dropped there. It might be nicer to call it dataPath but it's probably not worth it to break backwards compatibility for this.

If you see a generic path somewhere else in the code feel free to open a PR to clarify which kind of path it is. resolveData is not ambiguous as it specifies a dataPath as the argument.

I saw that you closed my other issue (https://github.com/eclipsesource/jsonforms/issues/2140). I think both of these can be summed up as "paths/pointers need a more robust implementation." I'd like to at least submit a PR which allows a config option to override resolution behavior. Being able to use JSON pointers, JSON paths, jsonata, etc. would allow for more advanced scopes.

The other issue (#2140) was closed because it's about a concept which is not foreseen in the JSON Forms architecture: By default we only support schemaPath in the UI Schema scope, not any kind of dataPath. This is the case because we need the schema information to determine what kind of renderer to show, the instance data does not help much there. Therefore it doesn't matter whether JSON Pointers, JSON Paths or jsonata is used, they will all resolve against the JSON Schema.

Before you open a PR I would like to recommend to describe the architecture of your suggested implementation to see whether it is something we would like to support. Note that even today we are already very flexible, the UI Schema can contain an arbitrary number of scopes (or similar properties) specifying any kind of path you would like to see. For these elements you then need to register custom renderers which can handle the customized properties.

Thinking about it: For the rule scopes it could make sense to support data paths. We reused scope, i.e. schemaPath because that's already a concept in the UI Schema. So if you would like to contribute a rule alternative in which a dataScope instead of a scope is specified, then that would be fine with me.

In my schema, I have an array of vehicles, each with an id. In a separate array, there are items related to vehicles, each with a vehicleId. I need some way to "lookup" the related items by vehicleId to render those fields alongside the vehicle fields.

This use case is not supported by default in JSON Forms. It's also not expressable in general via JSON Schema. However using custom renderers this can be accomplished so I would like to suggest going that route. If you need help with that please open a new thread in the community forum.


I tried to reproduce the issue but for me it seems to work correctly:

ShowRuleExample