fix: allow $defs and fix circular pointers issue with $ref in root

[KurtGokhan]

Fixes #382, #370, #356

I think all of the issues above are caused by the same root cause, that $ref in root object isn't correctly resolved.

Changes in this PR:

• Removed the short-circuit in Pointer.resolve. As I see, it was there to prevent a circular reference. But it causes incorrect behavior. The root of the problem that should be fixed is that Pointer doesn't have a mechanism to detect circular (self-referencing) refs.
  • There is no test case for failing circular refs. But it's a configuration error and cannot be dereferenced anyway. I think it's an advanced use case that doesn't need to be handled.
  • The change in Pointer caused one test to break, which is circular-external-direct. At first it seems like a legit test-case but it's actually not. Changed the test case to be more meaningful.
• Added handling for $defs in addition to definitions
• Used Vitest snapshots for tests. Let me know if there is a reason to keep using the utils other tests are using.
• Ran prettier format

[shahmir-oscilar]

Hey-hey! Thanks for this. I was facing the same issue in my code and I ended up using https://www.npmjs.com/package/patch-package to incorporate your changes and they worked out great!

[jonluca]

I'm not sure why some of the tests were removed/changed or why the null dereferencing logic was removed. Can you explain that?

Edit: I read your description 🤦‍♂️

I'll take a look at the behavior changes in the next few days, but otherwise looks good

[jonluca]

Can you expand on the test case not being valid?

If it's actually invalid we should remove the test case altogether

[KurtGokhan]

The test case was like this:

# root.yaml
$ref: ./child.yaml#/foo

# child.yaml
foo:
  $ref: ./root.yaml#/foo

And after I fixed the original issue, this was throwing an error like "property foo can't be found". The error is right because if you break it down:

root == child.foo
child.foo == root.foo

therefore:

root = root.foo

But root.foo doesn't exist.

This test was passing before because since root.yaml has $ref in it's root it was running into the incorrect behavior that was caused by the line I deleted. But the test itself was wrong because root = root.foo and since root.foo doesn't exist it should error. For this test, I added root.foo so the test case should be valid now.

What I mean when I said "There is no test case for failing circular refs"

This fix surfaced one more issue that isn't handled. Imagine a case like:

root == child.foo
child.foo == root

therefore:

root == root

In this repo I guess this is not called circular ref. It's another kind of circular ref I call "self-referencing" ref. I am not sure how this should be handled, or even if it could be handled. But the error could be graceful at least. Right now this case causes infinite recursion. So I guess there can be a mechanism to at least prevent that. That's why I was saying the Pointer utility doesn't have a way to detect circular refs.

[jonluca]

That makes sense. This looks good. I think the null logic that was removed comes from https://github.com/APIDevTools/json-schema-ref-parser/pull/374 - as long as the null test cases still pass there, though, I think this is ok? @erunion you worked on that feature, do you have thoughts?

[coveralls]

Pull Request Test Coverage Report for Build 14691060465

Details

• 12 of 12 (100.0%) changed or added relevant lines in 3 files are covered.
• 2 unchanged lines in 1 file lost coverage.
• Overall coverage decreased (-0.3%) to 92.605%

---

Files with Coverage Reduction	New Missed Lines	%
lib/util/url.ts	2	81.56%

Totals
Change from base Build 14540335093:	-0.3%
Covered Lines:	1654
Relevant Lines:	1754

---

💛 - Coveralls

[erunion]

Sorry for the delay on getting to this @jonluca and @KurtGokhan but I think the change makes sense!

[github-actions]

🎉 This PR is included in version 12.0.2 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[KurtGokhan]

Thanks all. Feel free to ping me if there are any issues related to this.

[github-actions]

🎉 This PR is included in version 15.0.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀