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

The (underlying data schema) API #2

Open
mohsen1 opened this issue Mar 2, 2015 · 43 comments
Open

The (underlying data schema) API #2

mohsen1 opened this issue Mar 2, 2015 · 43 comments

Comments

@mohsen1
Copy link
Member

mohsen1 commented Mar 2, 2015

Based on our discussions, it would be very beneficial to make the core component dependency-free and expose the core component via different means(AngularJS directive, React component or web component)

For that, we need to agree on the API of the core component. What should core component get and what it should return?

For starters I put these input/output lists here:

Input

  • Form Schema (JSON Schema)
  • List of templates (HTML Handlebars templates)

output

  • Data object to write the result of the user input (aka ng-model). Question: should we have getter/setter for this?

events

  • change (also keypress?)
  • Form submit
  • Validation error
@kentcdodds
Copy link
Member

I would highly encourage a using a custom field config object with the ability to convert a JSON schema into this field config object. JSON schema is verbose and unnecessarily complex if you're only wanting to setup client-side forms and your server doesn't use JSON schema as well. But JSON schema support is a must I'd say as well.

I think it would be pretty cool if we could figure out a way to decouple our library from the DOM. So we wouldn't need to be opinionated about what the templates/types look like. Also, validation works so differently between the different frameworks that I think that maybe that could be pushed down into the adapters as well because I think it would be valuable for us to leverage the framework where possible. I just don't want to have to fight the framework when it comes to making these adapters...

@bvaughn
Copy link
Member

bvaughn commented Mar 2, 2015

Agreed with Kent. Custom field config will be necessary because of custom validations (which seem to be fairly common, at least among formFor users).

Decoupling validation and display also seems like a nice goal, although I feel we'd likely want to provide a couple of default templates (e.g. Bootstrap, Polymer, whatever) as folks are unlikely to want to always write these from scratch.

FWIW, I think our validation and object-watching can be totally vanilla ES6. I might be missing something obvious, but I don't think we need any framework(s) to be involved.

@davidlgj
Copy link

davidlgj commented Mar 2, 2015

I agree wholeheartedly with @kentcdodds on that we should use our own format for field configuration and not marry ourselves to JSON Schema, its just to limiting in its validation.

And +1 for vanilla ES6

@kentcdodds
Copy link
Member

Totally agree @bvaughn that we should create default templates to encourage adoption. This is the approach I've taken with formly and it's worked really well. I always encourage people to create their own templates if they want to get serious about stuff (though, as formly has recently become much more flexible this is less necessary)

Whatever we do, we should make it very very easy to get started and very very easy to extend what exists to customize it. I think the API simplicity should be paramount so this is a terrific issue to discuss.

Also, I seriously recommend that we look into my apiCheck.js library. It's still fairly young, but it's vanilla-js (ES6 authored of course) and it helps people figure out the API without having to review the docs or post an issue.

@mohsen1
Copy link
Member Author

mohsen1 commented Mar 2, 2015

If we're going to have our own format, how about extending JSON Schema itself? There are a couple of benefits to that:

  • JSON Schema has a hyper schema that validates JSON Schema spec files. If we extend that hyper schema, we can get free JSON Schema validation for the objects people pass to this component.
  • JSON Schema will work with no adapters.

@kentcdodds
Copy link
Member

I'm not certain what this would entail, but I'm mostly concerned about the API. One of the things I work really hard to accomplish with formly is an api that is so simple that you only have to look at it once and then never again. This is helped a great deal because of apiCheck, but I think a simple api with an adapter is more valuable than a complex api without one. Even if it means more work for us.

@bvaughn
Copy link
Member

bvaughn commented Mar 2, 2015

Sounds to me like @mohsen1 is a good candidate for owning the JSON schema portion of forms-js. Seems like you're the one who feels most passionate about it (and who has the most experience with it). I'm comfortable with you making the schema-related decisions.

I think we should define what our validation input looks like first. Make sure it's robust, sexy, etc. - then we can tackle a utility that consumes JSON schema and generates this validation format.

@mohsen1
Copy link
Member Author

mohsen1 commented Mar 2, 2015

Just to be clear, I hate JSON Schema too! It's weird and has almost no documentations!

But
JSON Schema is a well defined standard. JSON Schema at IETF. At first look most things look weird, but there is always a good reason why the did it that way. For example @kentcdodds brought up required in our call. Sure, it makes tons of sense to put the required as a boolean property in the property definition itself. It's cleaner and everything is together. But it won't work when you have two properties that one of them (oneOf) is required. So they had to put the required filed outside and make it an array.

I promise we will have to redo all the work people did in JSON Schema working group over the years if we decide to make our own schema.

@bvaughn
Copy link
Member

bvaughn commented Mar 2, 2015

To me, that sounds like another argument for a configuration object that allows users to provide either truthy/falsy values or custom callbacks that return truthy/falsy values or custom callbacks that return Promises. At that point, it's relatively easy to implement complex logic without the mental overhead of parsing/understanding a schema.

@kentcdodds
Copy link
Member

Also, we have the luxury of JavaScript, whereas they had the limitations of JSON. I know that there will be those who use JSON schema and they'll want to use this library, which is why we'll make an adapter for it. But I believe that the main use case (based on users of angular-formly) will be those who just want to configure their fields with JavaScript.

@mohsen1
Copy link
Member Author

mohsen1 commented Mar 2, 2015

We need a list of properties with type, title, description and other information about each property. That list has to be recursive and allow arrays and so on. Everything we need is already in JSON Schema. We can extend JSON Schema to allow a lot more than that. I don't think it's a wise decision to not use JSON Schema for that list of properties.

@mchapman
Copy link

mchapman commented Mar 2, 2015

Strongly recommend you have a look at something like mongoose.js schemas and the idea of extending them (following the principle, but probably not the detail, I adopted in my library). I have been sending the schema from the server, but if there is something like a cross between pouchdb and mongoose then that would be a good place to start - it would give the most frictionless starter apps / demos.

@bvaughn
Copy link
Member

bvaughn commented Mar 2, 2015

We need more than just static (or pre-defined-conditional) validation though. Many of formFor's users need support for async, JIT validations. That can't be achieved with something like JSON schema.

@kentcdodds
Copy link
Member

👍 for that @bvaughn. JSON schema just isn't powerful enough. We need to enable people to use JavaScript. And if we're going to do that, then let's just leverage the power of a real programming language altogether and make the api simpler while we're at it.

@mchapman
Copy link

mchapman commented Mar 2, 2015

I agree, but think that Mohsens's diagram handles that well. Use JSON for the simple cases, and base it on something that is already well understood. This can include simple validation (min, max, patterns etc as in mongoose schemas). The advantage of doing this is two-fold - less education for those that already use that form of JSON schema and something tested in real life for others. Trivial forms can then be generated without the "Customizer Object" (or probably with a null one).

@mohsen1
Copy link
Member Author

mohsen1 commented Mar 2, 2015

We can have both :)

{
 "type": "object",
 "properties": {
  "name": {
    "type": "string"
  }
 },

 "validators": [
  function(object) {
    return $.get('someurl').then(function(data){
      return data.isValid;
    });
  }
 ]
}

@bvaughn
Copy link
Member

bvaughn commented Mar 2, 2015

I'm going to speak out of my ass a little here, but I strongly suspect that a vastly larger percentage of developers are familiar/comfortable with the concept of a POJOs that contain validation rules vs those that are familiar with JSON schema. (Our group is too small to be representative, but also seems to support this anecdote.)

That being said, @mohsen1 example above doesn't look very different at all from formFor's current validation rules.

    {
      agreed: {
        required: {
          rule: true,
          message: 'You must accept the TOS'
        }
      },
      signupEmail: {
        required: true,
        pattern: /\w+@\w+\.\w+/,
        custom: function(value) {
          if (value === 'briandavidvaughn@gmail.com') {
            return $q.reject('That email is already mine!');
          }

          return $q.resolve();
        }
      },

@mchapman
Copy link

mchapman commented Mar 2, 2015

We will also need validation that applies across multiple fields - password matching, or "you must specify at least one of home or work email address", so your example would need to be extended to cater for that.

@mohsen1
Copy link
Member Author

mohsen1 commented Mar 2, 2015

I expect this component get used in API and backend development tools. Like Swagger Editor. Those tools are very heavy on JSON Schema.

@kentcdodds
Copy link
Member

Which is why we'll provide something to make it work with JSON schema as well. I believe we can get both by making something to convert JSON schema to our own schema.

@mchapman
Copy link

mchapman commented Mar 2, 2015

👍 @kentcdodds for the conversion...

I am not going to fight for basing it on a current database schema format (extensible with JS in whatever way seems appropriate) if people think I am wrong, but I would really like some assurance that people have considered the idea and rejected it. (Just 👎 to let me know it was a rubbish idea and I will stop bleating)

@mohsen1
Copy link
Member Author

mohsen1 commented Mar 2, 2015

I'm all for our own schema. It seems necessary to have our own schema. I'm just advocating for making it a superset of JSON Schema so JSON Schema works out of the box.

If you think JSON Schema structure is bad or verbose and we should use a different structure, please make a proposal. I think JSON Schema covers tons of edge cases that we are not aware of right now and anything that we come up with will eventually have to solve those issues.

Here is an example of extended JSON Schema that covers @mchapman's case for validating across multiple properties. Thanks to JSON Schema recursiveness, it's easy to extend it in multiple levels.

{
  type: "object",

  "properties": {
    "email": {
      "type": "email",
      "validators": [
        function(){
          return Math.random() > 0.4; // :D
        }
      ]
    }, 
    "password": {
      "type": "password",
      "validators": [
        function(password) {
          return password.indexOf('M') === -1; // ?!?
        }
      ]
    }
  }, 

  "validators": [
    function(obj) {
      return obj.password != obj.email;
    }
  ]
}

@bvaughn
Copy link
Member

bvaughn commented Mar 2, 2015

One thing that may be worth pointing out is that formFor's validation is opt-in, meaning that you can specify little to no validation rules and things will just work. For fields that are validated, you can specify as little as...

{
  fieldName: {
    required: true
  }
}

Or as much as shown above (custom validations per-field, custom error messages, etc.)

If we go with a super-set of JSON schema, I'd want to maintain that feature.

Edit for clarity: I'm really just asserting that we should not require all attributes to be present in our schema. A lot of people (at least those using my library) choose not to validate a significant percentage of their attributes.

@mohsen1
Copy link
Member Author

mohsen1 commented Mar 2, 2015

Yes, JSON Schema can be small too. Here are some examples:

{"type": "string"}
{
  "type": "object",
  "properties": {
    "name": {"type": "string"}
  },
  required: ["name"]
}
{
  "type": "array",
  "items": {
    "type": "string"
  }
}

@bvaughn
Copy link
Member

bvaughn commented Mar 2, 2015

Is it optional though? Will it barf if it encounters attributes in the object being validated that are not explicitly mentioned in the schema?

Edit: Answering my own question here. Seems like this behavior can be controlled via additionalProperties: true|false so I'm happy.

@mchapman
Copy link

mchapman commented Mar 2, 2015

@bvaughn how about:

    myBasicSchema = {
      agreed: {type: Boolean, required: true},
      signupEmail: {type: String, required: true, pattern: /\w+@\w+\.\w+/}
    };
    myExtraStuff = {
      agreed: {validationMessages:[{required:'You must accept the TOS'}]},
      signupEmail: {
         html5type: 'Email', 
         validation: [{custom: function(value) {
            if (value === 'briandavidvaughn@gmail.com') {
              return $q.reject('That email is already mine!');
            }
            return $q.resolve();
          }], 
          validationMessages:[{required:'You must accept the TOS'}]
      }

except that in my experience the extraStuff is generally quite a lot smaller than the basicStuff

@mohsen1
Copy link
Member Author

mohsen1 commented Mar 2, 2015

I love mongoose schema. The only problem is, it doesn't support anyOf, oneOf, allOf and it's not as easy to serialize/deserialize as JSON Schema. JSON Schema itself is JSON so it's much easier to transfer it over the wire.

@mchapman
Copy link

mchapman commented Mar 2, 2015

OK. I have just realised we are talking about the JSON Schema rather than a JSON schema. So I will find a stone and crawl under it.

@mohsen1
Copy link
Member Author

mohsen1 commented Mar 2, 2015

If everybody is onboard with extending JSON Schema, I can open other issues for various aspects of extending it(validators and so on)

@mchapman
Copy link

mchapman commented Mar 2, 2015

I am, and I think we can assume David is (since it was his choice in the first place)

@bvaughn
Copy link
Member

bvaughn commented Mar 2, 2015

Haha, no worries Mark. It's understandably a little ambiguous in a few places above.

I am not initially opposed to investigating an approach that uses an extended JSON Schema. I'd love to see some example schemas that range from basic to highly-customized to ensure that we aren't forgetting something.

@mohsen1
Copy link
Member Author

mohsen1 commented Mar 2, 2015

Please have a glance at this page. http://json-schema.org/example2.html

@bvaughn
Copy link
Member

bvaughn commented Mar 2, 2015

Sorry for being ambiguous myself, but I was suggesting we put together a couple of examples of what our extended schema would look like. The base JSON schema doesn't have built-in support for things like custom validation functions, right?

@mohsen1
Copy link
Member Author

mohsen1 commented Mar 3, 2015

@bvaughn made a proposal for validators
#5

@mchapman
Copy link

mchapman commented Mar 3, 2015

Not sure if we want a separate issue for this but...

Now we have (hopefully) sorted out the data schema API how are we to deal with cases where the same schema is used in multiple forms? According to the spreadsheet angular-schema-form and forms-angular were the only two that had a solution here (with angular-formly having a WIP against it).

A use case would be a customer schema with invoice and delivery addresses. The accounts view of the data needs to include the former, and despatch the latter.

Both the existing solutions have an implicit view (the whole schema) and options create alternative views. These views specify the fields to be included and (certainly in one case) can modify / extend the underlying schema (for instance have additional validation for the view).

I like this distinction between data schema and view schema, though I can find fault with both the existing implementations. What do others think (and should this be a separate issue?)

@bvaughn
Copy link
Member

bvaughn commented Mar 3, 2015

formFor supported this use-case.

I think it was nice for several reasons:

  • Allows view/layout flexbility
  • Enables flexibility for users to share validations if needed

I'm also curious about how we're exposing schemas to forms-js. That's kind of what I'm asking on the other issue.

@mchapman mchapman changed the title The API The (underlying data schema) API Mar 3, 2015
@bvaughn
Copy link
Member

bvaughn commented Mar 3, 2015

I've given this a little more passive thought after work, and I believe I'm feeling less optimistic about the JSON schema approach and reverting back to my initial preference to stick with a POJO for validation (and generate that POJO from a JSON schema for users that prefer to roll that way).

My main concern is this: JSON schema is big and complicated. @mohsen1 seems to view this as a positive- in that it supports a lot of things- but I fear it's actually a negative. Here's my main 2 points:

Firstly, I'm not convinced that there are "tons of edge cases that we are not aware of right now". Between the 5 of us, we have many hundreds of Github stars worth of users. I think at this point, we probably are pretty aware of even less-common use-cases for libraries like these. So I think this may not be as big of a concern as you think.

Secondly, I'm worried there is a significant amount of hidden complexity behind extending JSON Schema and bolting on our own validation rules. Since our own rules aren't part of the JSON schema spec, I fear we'll either have to (a) write our own JSON schema validator (non-trivial) or (b) write logic that converts our super-schema into a regular JSON schema (that can be run through an existing validator) and a POJO that we apply our custom validation rules to- and then we'll have to merge the results. Honestly this doesn't sound any easier to me than implementing our own validation service.

I'm sure formFor doesn't cover all of the validation cases we'll want to support with forms-js, but it does have pretty decent coverage (judging by the lack of feature requests in this area) and the validation portion is relatively small and easy to understand (in addition to being well covered by test cases). I'm confident that if I were to re-write this validation as an ES6 module today, it would be even easier. Between the 5 of us- I bet it's much easier.

Anyway, just some thoughts. I'd like to see us nail down this basic thing before we get too caught up on templating and markup-generation, etc. because I think it's fundamental and very important.

@mohsen1
Copy link
Member Author

mohsen1 commented Mar 3, 2015

I'm open to ideas. If you think we can do better than JSON Schema, let's explore it. Please open a ticket and write your proposal for the POJO that defines the forma and validation. Let's see how it goes.

An example that describes this form would be great. We could compare it to JSON Schema.

@bvaughn
Copy link
Member

bvaughn commented Mar 3, 2015

Here are some initial thoughts. :)

https://gist.github.com/bvaughn/7db78c3e8639f8613a63

Totally open for discussion regarding any of this.

@davidlgj
Copy link

davidlgj commented Mar 3, 2015

TL;DR;
I don't like the idea of extending JSON Schema. JSON Schema doesn't validate a lot of common scenarios but adds complexity, and if we extend the JSON Schema standard with validation rules we will severely limit any of benefits of it being a standard.

First off, I'm sorry @mchapman but you where wrong about me supporting that we extend JSON Schema :) No worries, but actually I'm strongly against extending JSON Schema. I didn't choose to base angular schema form on JSON Schema, we already had lots of 'em and needed a form lib for it in angular.

I believe we should define our own "form config"/POJO/"form definition" that both describes the structure and the validation of the form and offer nice transform functions from both JSON Schema, and possibly an extended version of JSON Schema.

We can support generating forms from a JSON schema and I believe it's an important use case to cover, but not all possible schemas and not as the basis of form-js.

Why? Ok, let me start by ranting a bit about JSON Schema and forms.

Let me first start by saying that I feel JSON Schema has one strong point: Its a standard and therefore it has a lot of validators in different languages. This means you can validate in the same way both on frontend and server side.

So JSON Schema is for data validation, and it does that somewhat well. But data validation is not straight up the same as form field validation. There are a lot of rules that don't make any sense for forms, at least not for most cases. And having to code "around" them complicates stuff.

So an example to clarify things. JSON Schema supports something called "patternProperties" which basically lets you write regexp to match an objects property names, could be useful but there is not really a straight forward form UI you can build for it. It all depends on the use case. The same goes for oneOf, recursive $refs, arrays with specific items at specific indexes etc.

Another problem with JSON Schemas its validation doesn't cover a couple of common scenarios, we will need to add our own validation for these:

  • Async validation checks, like checking if a user name is already taken.
  • Inter-field validation, think select box that chains into select box, like country -> state, or "if this checkbox is checked then no address is needed"

Inter field validation is really a big deal, it's hard not to end up needing it. Version 5 of JSON Schema has some things to address this, but I don't know how useful they are.
Both of these are of course fixable by extending JSON Schema, but if we do we loose the benefit of it being a standard since we can't use third party validators, so basically we loose the plus side of using schemas.

I also believe that one schema is not enough to make a form, regardless of how many extensions we give it. A common scenario is that you have one schema and like to have several different form views of it. For example you might have a "User" JSON Schema, and need one form for the logged in user to change its own address, but not username. Then you have a admin page for sys admin that needs to be able to edit all properties of the user. So in this case you would need to strip out parts of the schema, which in my opinion is a bit of a bad design decision because its very easy to introduce bugs this way. And not to mention the hassle of keeping track of what schema should validate this time on the backend.

Another problem with schemas is that they are objects, so you usually end up doing the form fields in the same order as the properties are in the schema, this is not such a good solution though it works in practice the standard says that the order of properties is undefined. And could also be a problem when consuming schemas made by third party.

I also believe that most of the more advanced validation scenarios needs specialized UI to make it really user friendly so instead of trying to cover all complex scenarios with generic solutions we should instead try to have a core that is simple and make it easy to extend by plugins/templates/field types or some such.

@mchapman
Copy link

mchapman commented Mar 3, 2015

@bvaughn That gist looks fine to me, especially the number of pets validation. Please tell my kids. Not 100% sure about the non serialisable types but you have probably thought through what that buys more than me.

@davidlgj Sorry for getting that wrong. I agree about view schema and opened a separate issue #7 as I think they are separate issues (and I suspect this one is the easiest to reach agreement on).

@mchapman
Copy link

mchapman commented Mar 3, 2015

I have started a doc, mostly to help me think about stuff, but on reflection I think collaborating there may be quicker and better than referring way back in the thread. See what you think.

@mohsen1
Copy link
Member Author

mohsen1 commented Mar 3, 2015

@davidlgj and @bvaughn I'm convinced to drop support for JSON Schema based schema. Mainly because of undefined property order in JSON. And I agree that we need a clear path from JSON Schema to our own schema.

One thing that I learned from @bvaughn's example is i's nice to have extendable list of types. The user of forms-js can add their own type by providing the template.

I'm opening another issue for discussing list of default types we want to cover.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants