json-refs v2.0.0 Planning

[whitlockjc]

I think it's time to plan for json-refs v2.0.0. Long story short, json-refs started life really as a utility for swagger-tools. As people have began to use it, we've tacked on new features, fixed bugs and a number of other things...things that were not originally planned. The project has evolved and now we must take a step back and create a release with these features planned in from the beginning. My plan is to start working on this now and update you with the changes that happen. Feel free to request new features/ideas/options/... in this thread.

Initial plans:

• Collapse APIs for checking reference types into one API that returns the reference type
• Generate reference metadata when finding references (This is an efficiency thing)
• Make json-refs more efficient
• Refactor options so that the ones passed downstream to path-loader are obvious
• Support all types of references (Right now, we support the hash-based references but the spec actually says you can omit the # and that needs to be handled to.)
• Support id-based references (Issue Support URI Resolution Scope Alteration compatible with JSON Schema #3)

I'm copying all who have filed bug/feature requests and PRs as I think your opinion is invaluable.

@brettz9
@HyperBrain
@ippeiukai
@IvanGoncharov
@mjkaye
@mohsen1
@NadavK
@parky128
@Pharylon
@robotnic
@sacarro
@schrag

[sacarro]

Let me know how I can help. I would be happy to participate/contribute.

[brettz9]

What do you think of supporting a non-standard but pre-compile feature (which leverages the reserved nature of the tilde) to allow for run-time substitutions? One deficiency I've found with JSON references is in handling locales where the exact item to reference is dynamic. Maybe something like

jsonRefs.resolveRefs({$ref: '...'}, {substitutions: {var1: 'value1', '=var2': 'value2'});

which could work on

{"$ref": "#/some/path/~var1~/~=var2~"}

..and whereby the "=" would add escaping such that, within the "=var2" variable value, any "/" would become "1" and any "" would become "~0", whereas "var1" would not escape them and thus allow arbitrary additions to the path.

While this does rely on some dynamic properties (by necessity), it still does support the declarative nature of JSON references and produces valid output JSON. And perhaps another method could apply substitutions to create valid JSON references without resolving them.

My particular use case is in supporting a work around for the current lack of support for the i18n proposal made at https://github.com/json-schema/json-schema/wiki/multilingual-meta-data-%28v5-proposal%29#user-content-concerns whereby a JSON reference might be structured like

{
    "localization-strings": {
        "en": {
            "example": {
                "title": "Example schema",
                "description": "Example schema description"
            }
        },
        "de": {
            "example": {}
        }
    },
    "type": "object",
    "$ref": "#/localization-strings/~=lang~/example"
}

[whitlockjc]

I think we should collapse the isFilePointer and isRemotePointer. Basically, isFilePointer is really identifying relative pointers. I'm still thinking of what API(s) if any we need to identify the type of pointer and what impact that has on the options passed into relevant APIs.

What do you think of having a filter function as an option instead of having configurations for each type of reference? We still might have some API for getting this information, like get[Ref|Ptr]Details or something like that.

[brettz9]

I think it is relevant to keep having an API for conveniently distinguishing relative and absolute pointers, both as a utility and as config passed to methods like resolveRefs though a filter function sounds very good in addition.

What did you think of my suggestion (or was the filter idea meant as an alternative)? I think having some pseudo-standard convention ought to be helpful for making variable substitutions before resolving the refs, and I think it would make a nice utility to be available out of the box...

(Btw, it seems like prepareRequest is only available in the browser files, but not in index.js as I would think it should be also.)

[whitlockjc]

I do plan on having a way to distinguish between different pointer types but having a function for each kind doesn't make sense to me. So instead of having a function for local, remote, relative, ..., we'll likely end up with a single function that returns the type so you could do if (JsonRefs.getRefType(ref) === 'local') { or some way to get all reference details in one API like if (JsonRefs.getRefDetails(ref).type === 'local') {. Thoughts?

[whitlockjc]

As for your idea, it sounds extremely specific and doing this now within your application could be done with a simple preprocessor of the document. I'm hoping that my rewrite of JsonRefs#findRefs will provide you with enough information to do this yourself. Heck...maybe I could throw some lifecycle bits into the API but I'd need to think about that a little more.

[brettz9]

I think that should be fine re: getRefType or getRefDetails.

As far as my use case, I think variable substitution ought to be a pretty common scenario. And given the lack of i18n currently in JSONSchema, it really would help to have a modular (and hopefully increasingly recognizable) way of embedding locale or other such info where a portion is inevitably going to need to be dynamic (but still readily interpretable in static form). I was hoping json-refs could be the (unfailingly accurate) pre-processor :)

Regarding consideraton of the API for variable substitution, although JSON Reference has no formal way of expressing such custom variables, I chose ~...~ because the only reserved characters were ~ and /, and ~ made more sense, but needed to be delimited on both sides (unlike the built-in escapes which are currently limited to ~0 and ~1). (Maybe to avoid future conflicts, there should always be a prefix after the first tilde, like "var:...", or ideally, imo, something could get adopted by the JSON refs spec.)

[whitlockjc]

I'm not completely against it. Let me think about it a little more.

[whitlockjc]

I've been working on [email protected] locally and I was wondering what your thoughts were on me pushing my changes so we can have others give it feedback. That would mean that master would become unstable, as I started fresh and have been adding APIs back iteratively. Thoughts?

[brettz9]

Maybe you could just tag your latest stable version. No big deal either way to me personally though...

[whitlockjc]

Already done. I tag every release and there were no unreleased changes.

[whitlockjc]

Hello all. master is now officially for 2.0.0 development. Please review the commit log, the API documentation and let me know what you think. The major thing missing from this code drop is that resolveRefs is missing. Here are my plans:

• resolveRefs: I want to make this like findRefs in that it only works against the local file, as in no remote resolution at all. It will be a synchronous function.
• resolveAllRefs: I want this to work like what the old version of resolveRefs did. It will always be asynchronous.

I would also like to use the same concept for findRefs in that we could add a findAllRefs that would recursively find all references even in relative/remote references.

The idea for these changes is to have a clear "local only" option and with consistent naming and then to have a "full fledged" option that will by default resolve all references, again with consistent naming. Of course, findAllRefs and resolveAllRefs will operate with an options object like the current findRefs where you can provider an options.filter to choose which types of references you're interested in finding/resolving.

Thoughts?

[theganyo]

Your intent could be perhaps be made crystal clear by naming the local functions resolveLocalRefs and findLocalRefs.

[whitlockjc]

I started down that path and can't remember why I removed it. Maybe that's what I'll do when we have the remote-capable versions available. I'm all ears for other feedback as well.

[whitlockjc]

Do you think it's important to keep callback support for the asynchronous APIs? Promise-wrapped callbacks can be problematic in that errors thrown in them can never get out of the Promise chain as a runtime error without weirdness. I don't mind keeping them but are any of you on this thread using the callback-based versions of the APIs.

[whitlockjc]

I think I'll remove callback support for a simpler API unless someone needs it.

[brettz9]

Haven't yet found time to check the API, but could the callbacks also be required to complete execution before executing the then (like Promise.all)?

[whitlockjc]

resolveRefs currently has an optional third argument to allow for callback-based usage. I'm just talking about removing that argument.

[brettz9]

Ah--fine by me...

[sacarro]

Wouldn't there be a need for callback support for remote refs? Or are you just talking the find/resolveLocalRefs?

[whitlockjc]

In 1.x, resolveRefs always returns a Promise but there is support for callbacks, which also still returns a Promise. I'm proposing we always work with Promises and do away with the callback-based variant. Wrapping a callback in a Promise is straight forward but can be misleading when the user expects errors thrown in the callback to propagate out but that is impossible with promises. So to avoid confusion, I'd much rather just say that this is a Promise-based API.

[sacarro]

Oh sorry. I missed that. Then yes I would say remove them for clarity.

[whitlockjc]

Do any of you have a few minutes to join Gitter for a quick chat?

[whitlockjc]

I think 2.0.0 is ready for review. Below is the high-level change log:

• Added new functions
  • #findRefsAt: Just like #findRefs but it takes a string location as its first argument instead of an array or object. This version will retrieve the remote document at location and then find JSON References within it.
  • #getRefDetails: Returns JSON Reference details for an object (Handles invalid JSON Reference definitions as well)
  • #resolveRefsAt: Just like #resolveRefs but it takes a string location as its first argument instead of an array or object. This version will retrieve the remote document at location and then resolve the JSON References within it.
• Removed functions
  • #isFilePointer as that is now available via #getRefDetails and JSON Reference type is now included in all results that retrieve reference information
  • #isRemotePointer same as above
  • #resolveLocalRefs
• Renamed functions
  • #isJsonPointer -> #isPtr
  • #isJsonReference -> #isRef
  • #pathFromPointer -> #pathFromPtr
  • #pathToPointer -> #pathToPtr
• Updated functions
  • #findRefs now takes an options object that allows you to do various things like filtering references based on type (options.filter), only finding references for a sub portion of the document (options.subDocPath), ...
  • #findRefs now returns detailed information for each reference
  • #isPtr (Formally #isJsonPointer) now takes the JSON Reference definition object instead of the string portion of the JSON Reference definition. It also validates the $ref portion of the JSON Reference definition.
  • #pathToPtr (Formally #pathToPointer) now takes a second argument, a boolean, that tells whether to create a hash-based version of the JSON Pointer (default) or a hash-less version
  • #pathFromPtr (Formally #pathFromPointer) now works with hash-less JSON Pointers
  • #resolveRefs no longer has callback support
  • #resolveRefs has had its options argument changed:
    • options.depth is removed
    • options.loaderOptions is now used to pass options to PathLoader#load
    • options.location is now options.relativeBase to be more self-explanatory
    • options.resolveLocalRefs is removed in favor of options.filter
    • options.resolveRemoteRefs is removed in favor of options.filter
    • options.resolveFileRefs is removed in favor of options.filter
    • options.prepareRequest is removed in favor of options.loaderOptions
    • options.processContent is removed in favor of options.loaderOptions
  • #resolveRefs returns detailed information for each of the references

I will be adding examples to the source code so that the API documentation is updated accordingly. Until then, feel free to look at the unit tests to see how the APIs are used.

If I've not heard any objections shortly, I'll likely cut a 2.0.0 release so I can move on.

[whitlockjc]

I have updated the API documentation linked above to include examples for all "complex" APIs, including examples of the new/updated options.

[brettz9]

FWIW, I do plan to take a look, Jeremy, but busy with some other projects ATM...

[sacarro]

Same with me. Will get to it post holiday break. Thanks for all the hard work.

[whitlockjc]

No rush. I appreciate your feedback.

[brettz9]

Besides the PR and issues just reported, it all looks very good to me. Nice job! And thanks for addressing the desire for the *At retrieval methods.

[whitlockjc]

Thanks a lot. If I don't hear from anyone else, once I've addressed your issues/PRs, I'll likely cut [email protected] and we can then use it to find bugs and necessary APIs we missed.

[whitlockjc]

[email protected] has release: https://github.com/whitlockjc/json-refs/blob/master/RELEASE_NOTES.md#v200-2016-01-06

[whitlockjc]

Thanks for all of your feedback and help. Please review the release notes to see how your existing [email protected] code will need to change for [email protected].

[brettz9]

Congratulations on the release! Appears nice and robust...

[whitlockjc]

It's not perfect. I'm sure I'll end up with a few patch releases today as I run into consistency issues now that I'm upgrading sway to use it. Bear with me.