-
-
Notifications
You must be signed in to change notification settings - Fork 22
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
Great library, could use some improvements to the developer experience #71
Comments
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'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.
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.
Actually, there is a way to get a schema behind authentication. You can use a custom 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 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
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 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);
}
};
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. |
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
registerSchema
. I need to be able to check if schema is loaded before registering it.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 libraryvalidate
method, you are required to firstregisterSchema(jsonSchema, jsonSchema.$id)
. This coupled with the inability to check if a schema has been registered leads to bad developer experience.Hope this feedback is helpful!
The text was updated successfully, but these errors were encountered: