jsonforms icon indicating copy to clipboard operation
jsonforms copied to clipboard

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

Open lellimecnar opened this issue 2 years ago • 3 comments

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

    {
        "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.

lellimecnar avatar Jun 01 '23 22:06 lellimecnar

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?

sdirix avatar Jun 05 '23 09:06 sdirix

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)

lellimecnar avatar Jun 10 '23 01:06 lellimecnar

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

  • The schemaPath which is in the form of a JSON Pointer and resolves to a subschema of the root JSON Schema, e.g. #/properties/personalInformation/properties/name.
  • The dataPath which points to the corresponding property in the actual data object in dot notation, e.g. personalInformation.name. This data path is usually derived from the schemaPath including the current scope, e.g. the array index entry

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

  • When the whole data is undefined the rule evaluates to true because this use case is not covered by JSON Schema. This is consistent with popular validators like AJV.
  • When the someProp is undefined the rules still evaluates to true as this is valid JSON Schema. If you want to evaluate to false when the someProp is undefined you need to add a required constraint
  • When the someProp is set to some different value than someValue the rule evaluates to false
  • When the someProp is set to someValue it evalutes to true.

sdirix avatar Jun 12 '23 14:06 sdirix