-
-
Notifications
You must be signed in to change notification settings - Fork 97
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
Reference resolver in v2 #761
Comments
TBH, I didn't know it was deprecated. In fact, it seems we are not the only one. Besides that, there is a reason we use that resolver. That's the ref resolver that Spectral uses and requires when instantiating it. See https://github.com/stoplightio/spectral/blob/66f769001f045eb841f4fa35821a83ce0b661074/packages/core/src/types/spectral.ts#L5-L7. It is anything but ideal, but we had to use it. In fact, it has cons such as circular references not being handled which have been fixed with a workaround @magicmatatjahu did. There is an effort by Stoplight team for using I think the benefit on using Spectral is way far (better) than having to use this deprecated ref resolver. So i will say we should just deal with it and either 🙏 some day Stoplight finally does the migration or either we do it in their code base. cc @magicmatatjahu Just in case, check my comment if I'm missing something or I said something wrong, as you were the one who took care of that part ❤️ |
sorry but is why do we need it in dependencies? just quickly checking, I see that we have https://github.com/asyncapi/parser-js/blob/next-major/src/resolver.ts but it is not clear why we actually need a custom resolvers to pass to Spectral |
Allowing custom resolvers is needed since tools like the generator use it in current implementation. See #593 |
no no, don't get me wrong, I love the idea of custom resolvers and this is my main reason why I do not like stoplight resolver and that we do not use the resolver we used in v1 -> asyncapi/spec#930 |
We have the option of creating a PR to Spectral with the needed migration for using the |
yup, that should be ok. The issue I linked is something we fix after spec 3.0 release, so still few months ahead. I wanted to make sure we are not going away from the reference resolver. btw, looking at the custom resolver issue, circular refs issue, the duck taping we had to do. Also the fact that we need to maintain AsyncAPI rules on our own.....does it mean that officially spectral folks like to write https://www.asyncapi.com/blog/creating-consistency-announcing-asyncapi-spectral-together but in fact there is not much help from their side to fix outstanding issues? |
This issue has been automatically marked as stale because it has not had recent activity 😴 It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation. There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model. Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here. Thank you for your patience ❤️ |
still a big problem |
Yes indeed, very big problem |
I don't have the full context here, but I'm able to run import path from "node:path";
import { readFile } from "node:fs/promises";
import fs from "node:fs";
import spectralCore from "@stoplight/spectral-core";
import { bundleAndLoadRuleset } from "@stoplight/spectral-ruleset-bundler/with-loader";
import Parsers from "@stoplight/spectral-parsers";
import yaml from "js-yaml";
import { $RefParser } from "@apidevtools/json-schema-ref-parser";
import { JSONSchema } from "../types.js";
const { Spectral, Document } = spectralCore;
const runSpectralOnFile = async (filePath: string) => {
const spectral = new Spectral();
const rulesetFilepath = path.join(process.cwd(), ".spectral.yaml");
spectral.setRuleset(
await bundleAndLoadRuleset(rulesetFilepath, { fs, fetch }),
);
const file = await readFile(filePath, "utf8");
const yamlFile = yaml.load(file) as JSONSchema;
const yamlBundle = await $RefParser.dereference(
`${path.dirname(filePath)}/`,
yamlFile,
{},
);
return spectral.run(
new Document(JSON.stringify(yamlBundle), Parsers.Json, filePath),
);
}; |
@kennethaasan yep, but you're not able to do that in asyncapi parser-js library (AFAICT) which uses spectral internally. |
With spectral there are rules that are applied before and after dereferencing. That is why parser currently only uses spectral, cause the input needs to contain references to correctly apply rules. |
This issue has been automatically marked as stale because it has not had recent activity 😴 It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation. There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model. Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here. Thank you for your patience ❤️ |
In Parser v1 we use
@apidevtools/json-schema-ref-parser
and now I see that v2 uses@stoplight/json-ref-resolver
which is marked as deprecated and points to@apidevtools/json-schema-ref-parser
😄any particular reason for that?
cc @magicmatatjahu @smoya
The text was updated successfully, but these errors were encountered: