Great library, could use some improvements to the developer experience

[antonio-ivanovski]

Hello, great library you have build supporting great amount of JSON Schema spec. I was considering switching from Ajv to this library in order to get away from their schema-no-backward compatibility (eg. 2020-12 cannot be used together with 2019-09 or draft-7). Testing some schemas on the playground, this library passed those mixed-schema-version checks and gave me confidence that this might be it.

But I have some gripes that make this library unusable for me

• No way to check if schema is registered. There is just the method of registerSchema. I need to be able to check if schema is loaded before registering it.
• No way to get schema that is behind authentication or served via non-standard media type. I can use to some extent addMediaTypePlugin, but working with dynamic set of of schemas, you are destined to stumble on schemas that don't have the required type for the library
• No way to pass the JSON schema directly to the validate method, you are required to first registerSchema(jsonSchema, jsonSchema.$id). This coupled with the inability to check if a schema has been registered leads to bad developer experience.
• Single validator instance per application. Having the ability to separate maybe 2 instance of validator makes sense. Considering that there could be clash of instances between packages. Also maybe I want to pass the validate instance outside of my package and others to use it.

Hope this feedback is helpful!

[jdesrosiers]

Thanks for your feedback. Different people can have different ideas about what's essential, so it's valuable to hear different perspectives. Some of these things I'm aware of and some I'm hearing for the first time.

I was considering switching from Ajv to this library in order to get away from their schema-no-backward compatibility (eg. 2020-12 cannot be used together with 2019-09 or draft-7).

I'm glad to hear this feature is useful to you. I don't think many implementations in any language get this right and I'm proud that mine does.

No way to check if schema is registered. There is just the method of registerSchema. I need to be able to check if schema is loaded before registering it.

This is not a need I've ever come across, nor has anyone else ever asked for it, but it's very easy to add. I'll do that soon. I'm curious what you're doing that you would need this for. I can provide a better API in the future if I understand the use-case.

No way to get schema that is behind authentication or served via non-standard media type. I can use to some extent addMediaTypePlugin, but working with dynamic set of of schemas, you are destined to stumble on schemas that don't have the required type for the library

Actually, there is a way to get a schema behind authentication. You can use a custom UriSchemePlugin to override how the request is made. Something like this,

import { addUriSchemePlugin, acceptableMediaTypes } from "@hyperjump/browser";

addUriSchemePlugin("https", {
  retrieve: async (uri) => {
    const response = await fetch(uri, {
      headers: {
        Authentication: "token goes here",
        Accept: acceptableMediaTypes()
      }
    });

    if (response.status !== 200) {
      throw Error(`${response.status}: Failed to retrieve ${uri}`);
    }

    return response;
  }
})

Ideally there would be an easier way to handle the special case of setting a header without having to provide a completely custom UriSchemePlugin, but I didn't think of an approach I liked, so this is the only way for now. This is definitely something I'd like to improve in v2.0.

As for non-standard media types, you can use custom MediaTypePlugins to allow whatever you like. But, it sounds like what you want is a wildcard MediaTypePlugin that would allow you to add a MediaTypePlugin for */* that serves as a default handler. That would allow you to support unknown media types. I'd advise against doing that. Assuming a response is JSON that says it's something else is considered unsafe and could be a security vulnerability. However, I'm willing to add support for wildcards because I think supporting something like application/*+json would be reasonable. That should be pretty easy to add. I'll do that soon.

No way to pass the JSON schema directly to the validate method, you are required to first registerSchema(jsonSchema, jsonSchema.$id). This coupled with the inability to check if a schema has been registered leads to bad developer experience.

I deliberately chose not to provide this function. I'm aware it's confusing for people coming from ajv, but I think it promotes good practice. Ephemeral or anonymous schemas don't come up much in the real world. When they're used that way, it almost always means the developer isn't managing their schemas effectively. Schemas used by an application are generally known ahead of time and should be registered when the application starts. Then when they're needed, you don't pull in the whole schema, you just use it's identifier (URI). When schemas are discovered dynamically, you don't register them at all, you retrieve them from their authority (usually a web server). Either way, you're always using a URI to refer to a schema.

I'll point out that the second argument of the registerSchema function is optional. It will automatically look for jsonSchema.$id. Passing it as the second argument is redundant. You only need it if the schema is anonymous (doesn't have $id) or you need a consistent URI to reference the schema by whether it has $id or not. For example, you can create the function you want.

export const validate = async (schema, instance, outputFormat) => {
  const uri = "https://example.com/ephemeral-schema";
  try {
    JsonSchema.registerSchema(schema, uri);
    return JsonSchema.validate(uri, instance, outputFormat);
  } finally {
    JsonSchema.unregisterSchema(uri);
  }
};

Single validator instance per application. Having the ability to separate maybe 2 instance of validator makes sense. Considering that there could be clash of instances between packages. Also maybe I want to pass the validate instance outside of my package and others to use it.

I agree 100%. There are things about this implementation that I think could be better, but this is the only thing I consider a big mistake. The main objective for v2.0 is to correct that design flaw.

[hysmio]

Single validator instance per application. Having the ability to separate maybe 2 instance of validator makes sense. Considering that there could be clash of instances between packages. Also maybe I want to pass the validate instance outside of my package and others to use it.

I agree 100%. There are things about this implementation that I think could be better, but this is the only thing I consider a big mistake. The main objective for v2.0 is to correct that design flaw.

Would love to have this instanced support! I'd love to support if I can, is there any plans/proposals for this v2.0 design? Otherwise, happy to put one together.

[jdesrosiers]

Thanks for the offer. I have been working on this, but my attention keeps getting pulled to other things. I'll create a discussion with my vision for 2.0 and what I've done so far. We can continue the discussion there.

[jdesrosiers]

@hysmio https://github.com/hyperjump-io/json-schema/discussions/80

[michaelfaith]

I deliberately chose not to provide this function. I'm aware it's confusing for people coming from ajv, but I think it promotes good practice. Ephemeral or anonymous schemas don't come up much in the real world...

Just wanted to point out that this makes some pretty big assumptions about how people are using json schemas. For instance, ESLint's rule options are built around the json schema spec. Rule authors define the schema for their rules as part of the rule meta data. And ESLint, as part of parsing a user's eslint.config has to validate that the options that a user has defined in their config, conforms to whatever schema the rule author of that plugin has defined. So, it's completely dymanic, and there are no consistent URIs. https://eslint.org/docs/latest/extend/custom-rules#options-schemas Currently ESLint uses ajv to do the validation. So, just calling out that, maybe there are use cases that you haven't considered when making that design choice?

[jdesrosiers]

@michaelfaith Thanks for sharing your perspective. I'm sure there are exceptions. In those cases, you can always make a temporary identifier like I demonstrated in the comment you're quoting from. (Remember that the URI doesn't need to be hosted anywhere. It's just a string in the shape of a URI.) You have to write a little more code or use that helper function I shared. I'd rather not include that helper function in the API because I expect it will be naively used in cases where it shouldn't. I'd rather people need to write a little more code in exceptional use cases than compromise the promotion of good schema management.

Specifically in the ESLint case, I'd ague that it's not ideal to have a schema embedded in the config in that way. When schemas are their own document, there a lot of development and automation tools you can take advantage of like schema validation, autocomplete, syntax highlighting, linting, etc. You loose all of those benefits when you bury the schema in code. The trade-offs are similar to constructing SQL statements in strings embedded in your code. It can be error prone and difficult to detect errors before they're blowing up your application.

I would suggest that instead of an embedded schema, the config could take a path to a separate schema document or just have a well-known file name it looks for automatically. That's something this library handles well and you don't need any fake identifiers. Of course, changing the way the config works probably isn't an option or at least would be very slow to change. And maybe the approach I suggest doesn't work for some reason that I'm not aware of because I don't know that system in detail. I still want to nudge in the direction of this kind of approach being the first choice and embedding only being used if a thorough analysis of the trade-offs reveals that it's a better choice.