No support for remote refs

[automatthew]

JSCK doesn't pass the official refRemote test suite and this one test in ref.js.

[automatthew]

I'd like to keep this all synchronous for the time being, but am willing to discuss.

[andrewjanssen]

How about an "async" option on validate()? Defaults to false.

[automatthew]

We need to resolve the remote refs at construction time, in case any other schemas refer to schemas within the remotes. Unless I've missed something.

[andrewjanssen]

... in order to limit any schema-resolving recursion? Are there other problems that refs-referring-to-schemas would create?

[automatthew]

Yeah, all schema reference resolution has to happen at "compile" time. I guess we can just bite the bullet and make instantiation be async. I'd want both an optional callback and some sort of EventEmitter like interface.

[andrewjanssen]

I might be missing something, but I think we can put all the schema resolution in the validate method. The validate method has access to the schema's schema, so fetching it won't be a problem. And it's only during validation that we can determine what the refs and their schema's even are. Furthermore, I think you'd agree that it's more natural a) to make network requests with... and b) to supply callbacks to... a verb method like "validate" than to a constructor. If the validator caches retrieved schemas, there's no performance problem on validator reuse. Thoughts?

[automatthew]

JIT compilation IOW

[automatthew]

One consideration is being able to fail at construction time rather than at first use.

[automatthew]

Which consideration we can handle by having a method that fetches refs and does callbacks && event emission.

[automatthew]

Another possibility is to make JSCK async by default, but provide a method for synchronous usage which throws an error when it encounters a remote ref.

It should be possible to allow users to prefetch a remote schema document and include it in the construction call. If the top level "id" URL is correct, this might even work now?

[andrewjanssen]

One consideration is being able to fail at construction time rather than at first use.

Agree. Failing on construction is a better pattern.

Which consideration we can handle by having a method that fetches refs and does callbacks && event emission.

Excellent. Love it. Would this method be required and cause verify to fail if it wasn't called first, or should it be called by verify as needed?

async by default

Could be good. I could even like an async-only design, as that would force all calling code into the same calling pattern. Depends on your design philosophy; do you favor having many ways to do something, or having only one way to do something?

[automatthew]

Many ways, when compelling use cases can be established.

One of my common uses of JSON Schema (and thus JSCK, lately) is for validating service configuration files. These are simple enough schemas that I don't foresee ever needing remote refs. Synchronous construction and validation in this case helps mitigate the complexity of setting up a server in node.js.

[andrewjanssen]

Sounds good. I think I have enough specification to make this issue happen. :) Here I go!

[automatthew]

Bump the minor version in package.json for your branch, please.

[andrewjanssen]

Will do.

[mafintosh]

What if we created a inlineRemoteRefs module that was async and just inlined remoteRef definitions in your schema? That could be used by all schema engines and would mean that we wouldn't have to support a weird async interface

var inlineRemoteRefs = require('...')
inlineRemoteRefs(schema, function(err, sch) {
  if (err) throw err
  // sch has all remoteRefs definitions inlined
  var validate = validator(sch)
  ...
})

[automatthew]

What if we created a inlineRemoteRefs module that was async and just inlined remoteRef definitions

That's probably the best way to add it non-invasively.

[tauren]

👎 for async loading at construction time. Certainly don't do it at runtime

I suppose it depends on the use case, but it seems that adding support to automatically and recursively make http requests from this library may unnecessarily add complexity. I'm not just talking about requiring an async interface, but also the code in the library will grow and potentially be more fragile.

For my needs, I know in advance all of the schemas needed, so my application can simply prefetch them and provide them to jsck. This allows jsck to stay light and simple with fewer potential bugs and performance problems.

However, if an application doesn't know the schemas it will need and must load them dynamically at run time, perhaps a feature like this would be beneficial. I just hope that it doesn't bloat the library.

👍 for @mafintosh proposal. This would keep the async code in a separate module and wouldn't bloat the core jsck codebase.