Limenius / liform-react

Generate forms from JSON Schema to use with React (& redux-form)
https://limenius.github.io/liform-react/
MIT License
174 stars 40 forks source link

Cannot set the error for a type of array #38

Open shuowpro opened 6 years ago

shuowpro commented 6 years ago

Problem:

I tried with a schema like this

var schema = {
  "type": "array",
  "items": {
    "type": "object",
    "properties": {
      "foo": { "type": "string" },
      "bar": { "type": "number", "maximum": 3 }
    }
  }
};

This is an array schema, I cannot get the error when I want to valid the properties inside the object of the array such as foo or bar. I don't know whether that is a bug. I try to validate that with ajv, and the ajv reported 2 errors with dataPath: [0].foo and [0].bar, the first character of the dataPath is not ., so that I think that might the reason why the buildSyncValidate will not transform the dataPath into the format like /0/foo, so that I cannot validate the field in a object in the array.

I think the if statement of detect whether the first character of dataPath is . or not is redundant. This will be repaired by remove that if statement.

br0wn commented 6 years ago

I've encountered the same error. From what I've managed to see it is the bug in the deepmerge library.

I have data structure like follows:

{
    list: [{ foo: 'val', bar: 'val' }, ...]
}

The bug happens in following code:

// buildSyncValidation.js
    var errors = ajvErrors.map(function (error) {
      return setError(error, schema);
    });
    // We need at least two elements
    errors.push({});
    errors.push({});
    return _deepmerge2.default.all(errors);

The ajvErrors.map will return something like this

[
   { list: [ {foo: 'some error} ] },
   { list: [ {bar: 'some error} ] },
]

_deepmerge2.default.all(errors) then returns following

{ list: [  {foo: 'some error}, {bar: 'some error} }

which is not correct. ~I believe this bug was fixed in version 2 of the deepmerge library. Maybe only updating that package would fix this.~ Disregard this, just saw "deepmerge": "^2.0.1", in package.json.

shuowpro commented 6 years ago

@br0wn Hello, have you found any solution for that? I found a temporary solution for this. I can read the schema and build a field level validation manually. But I don't think this is a good solution for now.

br0wn commented 6 years ago

@leuction Unfortunately no :( I disabled the default sync validation provided by the library as I don't need it at the moment. If you have time and will, you can fix buildSyncValidation.js and provide a PR :) Until it is merged you can use your fork.