fix: use uri-js to resovle uri's

[sondrele]

What issue does this pull request resolve?
This is a PR to resolve #423

In the issue you mentioned that it should maybe be part of the JSON-schema-test-suite, but juding from the last comment on the issue, it might not be necessary.

What changes did you make?
Replaced usage of the node url package with uri-js for resolving url's and schema id's.

Is there anything that requires more attention while reviewing?
This changes a pretty central part of url resolving, but it seems like it should be covered by the tests.

Let me know if you'd like to do some other changes, if it need more tests, or implement it some other way.

[epoberezkin]

I'll have a look a bit later

[epoberezkin]

@sondrele I'm a bit reluctant to switch from internal url to uri-js for several reasons:

1. the use case when URIs are used in $ref is not common.
2. it may have some differences in behaviour compared to url that is difficult to identify/test.
3. the bundle becomes 4kb bigger.

I think a better approach could be to allow passing uri-js (or any other library) as an option and specify in docs what methods it should support. What do you think?

[sondrele]

@epoberezkin I understand your concerns, both about size, and about a possibly different behavior (although that could maybe be solved with a version bump). It's a little unfortunate that not all urn's are supported, but I guess it's fine as long as it's documented as a limitation.

As a curiosity (and side-note), have you considered looking into replacing the legacy url.parse() and url.resolve() functions from the node-url package, with the whatwg-url API instead? That should help decrease the bundle size even further (and maybe offer improved, native performance, for all I know), if url's are going to be the main concern for Ajv.

An option to configure the id resolving would work for my case above, and it might also allow for some other use-cases where opaque identifiers are all that's needed. What's your stance on the API?

[epoberezkin]

@sondrele

have you considered looking into replacing the legacy functions from the node-url package, with the whatwg-url API instead?

That would make it more difficult to allow supporting all URIs, no?

What's your stance on the API?

I thought it would be just an option "uri" that should be an object compatible with the current "legacy" url package API (i.e. supply methods parse and resolve, where parse returns an object with properties (at least) protocol, href, host, path). If it is not supplied, "url" will be used.

[sondrele]

That would make it more difficult to allow supporting all URIs, no?

Probably, but I thought I'd just throw it in as a different issue, since URL id's are going to be the main concern for Ajv. :)
The "legacy" part comes from this issue and comment on the node.js issue tracker. They seam to talk about deprecation of url.parse(), but not for the foreseeable future, judging from the comments.

I'll get around looking into a uri option when time allows :)

[sirugh]

Using ajv for schema compilation in the browser is proving difficult because of the use of the node url module. We're currently wrestling with rollup/webpack trying to figure out a way around it but I personally would love to see this issue/PR handled. This library's tag-line is "The fastest JSON Schema validator for Node.js and browser" but it can't run natively in the browser, at least not when used as a dependency of our own bundle.

[epoberezkin]

@sirugh your comment is unrelated to the problem this PR addresses (support for full URI spec in refs rather than just URLs). Please submit a specific example that doesn’t work in the browser for you together with how you load ajv. At the very least I expect it to work ok with the provided bundle, so maybe your issue can be addressed with the changes in how you build it?

[sirugh]

You're right. I'm just at wits end here trying to figure it out. I'll try to come up with a reasonably simple use case that displays the error I'm talking about.