Don't use AJV. Use JSON. about sdk-javascript HOT 8 CLOSED

I don't have any particular love or hate for AJV. But do you have another suggested tool for schema validation? AJV gets 31 million downloads a week, so I think it's probably pretty well liked.

from sdk-javascript.

Can you help me understand why you think that JSON schema validation makes creating CloudEvent instances difficult? I don't follow.

from sdk-javascript.

The issue is around AJV.

https://github.com/cloudevents/sdk-javascript/blob/master/lib/specs/spec_1.js#L36

which expects a file like:

https://github.com/cloudevents/sdk-javascript/blob/master/ext/spec_1.json

and uses boilerplate like this:

https://github.com/cloudevents/sdk-javascript/blob/master/lib/specs/spec_1.js#L111-L201

Rather than creating this boilerplate, I suggest we remove AJV. This will make it possible to do things like fix this issue around directly using JSON: #65

from sdk-javascript.

However, it looks to me like that's not what AJV is used for in this case. If you look at the commit history of that file, you can see that it appears to have been hand written. In this case, AJV is not being used to generate code, rather it is being used to validate input, which I think is a reasonable usage of the module since that input may be incorrect.

It's certainly reasonable to want to simplify this file, but I don't think removing data validation checks is necessarily required to do so.

from sdk-javascript.

OK, so if we want to keep the feature to validate the input, I suggest we use a different tool than AJV for the isValidAgainstSchema method. Perhaps in a validate_ce.js file rather than the main spec file. Is that fine?

from sdk-javascript.

I don't have any particular love or hate for AJV. But do you have another suggested tool for schema validation? AJV get's 31 million downloads a week, so I think it's probably pretty well liked.

I'll look into it. It might just be the way we use the library with our current format and not the validator itself.

from sdk-javascript.

@grant I think that the schema validator is actually really important and would be a lot of work to replace manually (and would not be as fast/efficient as AJV). It is sometimes valuable to have the capability to determine if a CloudEvent is actually valid per the specification and that's what this does. I can see an argument for decoupling the validator from the CloudEvent class itself and exposing it as a function. But I don't see value in removing this capability altogether. I think this issue should be closed, or perhaps you have something in mind that I'm not aware of.

from sdk-javascript.

Yeah, the usage now is a lot cleaner than it was before.
It does take 60% of the bundle size, so I was a little concerned if we're trying to slim this package down.
Maybe it could an optional dependency in the future.

Should be fine to keep until there's a better suggestion.

from sdk-javascript.