Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

External nested refs aren't being resolved #96

Closed
wants to merge 2 commits into from

Conversation

diegode
Copy link

@diegode diegode commented Aug 9, 2016

This test is failing.

@whitlockjc
Copy link
Member

This is currently being tracked in whitlockjc/json-refs#80 The plan is to update the json-refs version in sway when this is fixed.

@whitlockjc whitlockjc closed this Aug 9, 2016
@diegode
Copy link
Author

diegode commented Aug 10, 2016

@whitlockjc I think this is a different issue, since json-refs resolves it fine:

$ json-refs resolve test/browser/documents/2.0/nested-refs/b.yaml 
{
  "swagger": "2.0",
  "info": {
    "title": "test",
    "version": "0.0.0"
  },
  "paths": {},
  "definitions": {
    "x": {
      "type": "object",
      "properties": {
        "children": {
          "type": "array",
          "items": {
            "type": "object",
            "properties": {
              "name": {
                "type": "string"
              }
            }
          }
        }
      }
    }
  }
}
$ json-refs --version
2.1.6

@whitlockjc
Copy link
Member

Yes, json-refs@2.1.6 resolves it fine but swagger-tools isn't using that version. That release fixes the issue but has a huge performance issue that I'm working on now. As soon as json-refs@2.1.7 comes out, or whatever its next version is, I'll update swagger-tools accordingly.

@diegode
Copy link
Author

diegode commented Aug 11, 2016

@whitlockjc thanks, and I'm sorry to bother you again: I just tried with json-refs@2.1.5 and it resolved it fine, just like 2.1.6 did. So I think that maybe this issue is unrelated to json-refs. Can you confirm?

@whitlockjc
Copy link
Member

json-refs is the resolving library for sway and swagger-tools. It is the only thing that is used for resolving references. What version of json-refs is installed in your swagger-tools? npm ls | grep swagger-tools

@diegode
Copy link
Author

diegode commented Aug 11, 2016

@whitlockjc I'm just using the json-refs git repo:

$ git checkout v2.1.5
HEAD is now at 9a84142... v2.1.5
$ npm install
(...)
$ ./bin/json-refs --version
2.1.5
$ ./bin/json-refs resolve ../sway/test/browser/documents/2.0/nested-refs/b.yaml 
{
  "swagger": "2.0",
  "info": {
    "title": "test",
    "version": "0.0.0"
  },
  "paths": {},
  "definitions": {
    "x": {
      "type": "object",
      "properties": {
        "children": {
          "type": "array",
          "items": {
            "type": "object",
            "properties": {
              "name": {
                "type": "string"
              }
            }
          }
        }
      }
    }
  }
}

@whitlockjc
Copy link
Member

What I'm trying to say is that you reported an error with sway that has since been fixed in json-refs but that version of json-refs is not the one installed into your sway (likely). So to figure out if I'm right, wherever your sway-based project is, run this: npm ls | grep json-refs and tell me the version. (Sorry about the previous example referencing swagger-tools, that likely muddied the waters.)

@diegode
Copy link
Author

diegode commented Aug 11, 2016

I'm sorry @whitlockjc: the version of json-refs used in my project that depends on sway is 2.1.6, and it's failing there. Now that I've read whitlockjc/json-refs#80, I understand that the issue happens when using "-I relative", and that makes sense. I agree with the solution proposed here: whitlockjc/json-refs#80 (comment)
We can continue the discussion over there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants