flitbit / json-ptr

A complete implementation of JSON Pointer (RFC 6901) for nodejs and modern browsers.
MIT License
93 stars 28 forks source link

Support relative json pointers #19

Closed forivall closed 3 years ago

forivall commented 4 years ago

I discovered that these exist when i was looking through the ajv code.

Spec: https://tools.ietf.org/id/draft-handrews-relative-json-pointer-00.html

chrishalbert commented 4 years ago

👍 @flitbit I encountered the need while working with the json schema spec too. I have some bandwidth if you would like any assistance. I can do the docs, tests, or implementation. Thanks for your work!

flitbit commented 4 years ago

API Thoughts (for feedback)

Relative pointers relate to a relative point in the object graph in relation to a fixed point. I think the fixed point can be any ole JsonPointer. So to work with relatives, I'm inclined to a semantic like this:

// ...
const fixed = new JsonPointer('/some/pointer');
const relative = fixed.relative('1/other/value');

const hasRelative = relative.has(target);
const relativeValue = relative.get(target);
// etc. etc.

This implies a new class RelativePointer which is fixed/prepared for the two locations in the object graph. It also enables compiling/preparing the RelativePointer to perform well when reused.

forivall commented 4 years ago

Makes sense to me.

Perhaps there could also be a pointer.resolve() similar to path.resolve(), etc. and other similar static helper methods.

chrishalbert commented 4 years ago

@forivall Can you elaborate with a code sample?

flitbit commented 4 years ago

@forivall if I understand your .resolve() suggestion, it would be a new method on JsonPointer such that:

import { JsonPointer } from 'json-ptr';
import { ok } from 'assert';

const data = {
  foo: ['bar', 'baz'],
  highly: {
    nested: {
      objects: true
    }
  }
};
const p0 = new JsonPointer('/foo/1');
const p1 = p0.resolve('1/0');
assert.ok(p1.pointer === '/foo/0');
assert.ok(p1.get(data) === 'bar');

I like it.

forivall commented 4 years ago

ok, actually digging into the current API, i think it's simplest to just update the JsonPointer.concat method instead adding a new method.

The resolve() function i was thinking of would be a static helper method. actually, it's better to just call it join() (like path.join(), since we don't want to confuse it with JsonReference.resolve()

so, for example:

import { JsonPointer } from 'json-ptr';
import { ok } from 'assert';

const data = {
  foo: ['bar', 'baz'],
  highly: {
    nested: {
      objects: true
    }
  }
};
const p0 = new JsonPointer('/foo/1');
const p1 = p0.concat('1/0');
assert.ok(p1.pointer === '/foo/0');
assert.ok(p1.get(data) === 'bar');

const p2 = JsonPointer.join('/foo/1', '1/0');
assert.ok(p2.pointer === p1.pointer);
forivall commented 4 years ago

yeah, it makes sense for it to be join and not resolve since in the path analogy, there's no "current directory" that we can resolve from.

benatkin commented 4 years ago

This is something I could use right now. The proposed API looks good, except a pointer passed in in my use case, and perhaps many others, need not be a relative pointer, so perhaps it would be good to change the name to something more general. I suggest ref or rel (which is short for relative but used in places where it doesn't strictly mean relative, like <link rel> when referencing a stylesheet).

benatkin commented 4 years ago

I have an easy workaround - I just use a regex to check if it starts with a number, and if it does split it into the number and the rest of the pointer. I get the node a number of levels up from my point of reference, and pass that node into new JsonPointer, and use the rest of the pointer to get a value.

chrishalbert commented 4 years ago

Building off of @benatkin 's thoughts on using rel or ref, I'd like to suggest a new method on the instance and static method named .relative() This makes it clear that it's implementing the Relative JSON Pointer spec. Modifying concat() means that the method would be supporting both the JSON Pointer and Relative JSON Pointer spec, which conflicts the SRP and may misguide users into thinking that their pointers abide to RFC 6901, when its actually a Relative JSON Pointer. Additionally, this could result in unexpected behavior for existing codebases, like falsely resulting in valid resolutions to existing code, which would break backwards compatibility.