stoplightio / spectral

A flexible JSON/YAML linter for creating automated style guides, with baked in support for OpenAPI v3.1, v3.0, and v2.0 as well as AsyncAPI v2.x.
https://stoplight.io/spectral
Apache License 2.0
2.35k stars 224 forks source link

Switch to APIDevTools/json-schema-ref-parser #1054

Open philsturgeon opened 4 years ago

philsturgeon commented 4 years ago

Currently we use our home grown json-ref-resolver, but we've switched most of our ecosystem over to APIDevTools/json-schema-ref-parser for it's superior bundling strategy (https://github.com/stoplightio/studio/issues/275) amongst other things.

We've got some outstanding pull requests going on, but so to avoid extra work let's wait for https://github.com/APIDevTools/json-schema-ref-parser/pull/153 to be merged, then switch Spectral over.

philsturgeon commented 4 years ago

Work was started on this a while back but not completed. Feel free to use this as a base: https://github.com/stoplightio/spectral/tree/feat/schema-ref-parser

lehphyro commented 4 years ago

Are you guys planning to finish the switch soon? The linked issues forced us to disable linting in our projects due to random errors produced by failure to resolve references properly.

philsturgeon commented 4 years ago

@lehphyro most of the examples in the linked issue (#870) have workarounds, and some of them even seem to indicate user error, so we've focused on frying bigger fish for now. Is there no workaround for your infinitely circular dependency scenario?

lehphyro commented 4 years ago

@philsturgeon It's not about only circular refs, I've created a repo that you can use to reproduce the issue: https://github.com/lehphyro/spectral-ref-issue

I usually use maven to run node tools, so you can reproduce it by cloning that repo and then running mvn clean install from the root directory, running spectral binary directly yields the same result.

This is the error you should see:

$ spectral lint src/main/resources/openapi.yaml --ruleset openapi-rules.yaml
OpenAPI 3.x detected

/repo/spectral-ref-issue/src/main/resources/examples/response.json
 1:1  error  examples-responses-are-valid  can't resolve reference schemas/statement.yaml from id #

✖ 1 problem (1 error, 0 warnings, 0 infos, 0 hints)
philsturgeon commented 4 years ago

Interesting. There may well be some sort of issue from having a model load up the main openapi file again, which is in turn trying to load up a bunch of refs and then some of them call back to the main file, and on it goes.

I'd switch this:

  documents:
    description: List of documents provided by the bank
    items:
      $ref: '../openapi.yaml#/components/schemas/AccountStatementIdentification'
    type: array

to this:

  documents:
    description: List of documents provided by the bank
    items:
      $ref: schemas/statement-id.yaml
    type: array

See if that sorts you out.

lehphyro commented 4 years ago

I cannot unfortunately, I need to have all $refs point to the components section in order to have https://openapi-generator.tech/ generate code with good class names.

lehphyro commented 4 years ago

Should I assume that this won't be done any time soon? Since I'll have to switch to another linter entirely, it'd be nice to know it before making the effort.

philsturgeon commented 3 years ago

@lehphyro Lots of work is being done on json-schema-ref-parser and several releases have been made since these discussions started, so I'm sure whatever concerns you have with it will be solved soon enough, if not already.

The plan is to wrap up the resolution logic in an adapter pattern as part of v6, and provide an adapter for both resolvers - or allow people to pass in their own entirely - therefore separating the resolution and linting logic entirely. You can keep using the old one until json-schema-ref-parser has solved whatever problems are keeping you off it.

Consolidation of efforts is the goal here, meaning two teams are working on improving one tool instead of us both playing whackamole with various edge cases. Neither are good or bad, they've just both got different problems, but json-schema-ref-parser generally has fewer of them, and is getting a lot of PRs from us to remove the rest. Soon comes the new $ref logic for $id and OAS3.1/JSON Schema 2019-09 logic (https://github.com/APIDevTools/json-schema-ref-parser/issues/145), so... no point us all doing that twice.

lehphyro commented 3 years ago

@philsturgeon thank you for the update, I'll give it a try when a working solution becomes available.

sebas2day commented 3 years ago

If you (accidentally) have a reference inside the spec to the spec itself and run spectral lint ./self-ref-spec.yaml it will run into an infinite loop and eat up your memory. Not sure if this is a known issue but it might seem related.

Any idea if switching to this new json-schema-ref-parser package would fix this as well?

philsturgeon commented 3 years ago

Yes, the new parser is much better at handling circular dependencies of all sorts. The team is currently working on v6 after several delays and changing priorities so Im eagerly waiting for this to be done!

-- Phil Sturgeon Product @ Stoplight.io

On Jun 15, 2021, at 20:16, Sebastiaan Brouwer @.***> wrote:

 If you (accidentally) have a reference inside the spec to the spec itself and run spectral lint ./self-ref-spec.yaml it will run into an infinite loop and eat up your memory. Not sure if this is a known issue but it might seem related.

Any idea if switching to this new json-schema-ref-parser package would fix this as well?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

jarrodparkes commented 2 years ago

@philsturgeon any updates on this? is there anyway to bypass circular references, use the new parser, etc in the meantime?

philsturgeon commented 2 years ago

Ping @mnaumanali94

magicmatatjahu commented 1 year ago

@philsturgeon @mnaumanali94 Hi! Any update? From what I can see in the Stoplight Studio sources a new resolver based on json-schema-ref-parser is already in use (I could be wrong, I'm looking at the minified code). I don't have any problems with the current resolver, however, what hurts me the most is that it doesn't keep the same reference (in JS, after dereferencing) between $refs, example:

type: object
properties:
  someProperty:
    type: object
    properties:
      deepProperty:
        $ref: '#/properties/anotherProperty'
  anotherProperty:
    type: string

and comparison:

$.properties.someProperty.properties.deepProperty === $.properties.anotherProperty

equals false 😢 I can "fix" that using documentInventory but it's a big hack to reassign references again.

magicmatatjahu commented 1 year ago

Sorry everyone! This is an error in my code. Sorry for the problem but question Any update? is still active 😄

smoya commented 1 year ago

Any update on the effort to do the migration? Especially considering the fact https://github.com/stoplightio/json-ref-resolver is marked as deprecated.

Thanks 🙏

ankorstore-haddowg commented 1 year ago

any update on this, having a number of issues with non-deterministic results which seems related to our usage of $refs

afmhenry commented 1 year ago

Also running into issues with this when running spectral on multiple files inside the same cli invocation.

derberg commented 1 year ago

@mnaumanali94 hey any ideas on when will that be addressed?